Skip to content

Migrate cpu module from avocado to autils#105

Open
harvey0100 wants to merge 1 commit intoavocado-framework:mainfrom
harvey0100:cpu
Open

Migrate cpu module from avocado to autils#105
harvey0100 wants to merge 1 commit intoavocado-framework:mainfrom
harvey0100:cpu

Conversation

@harvey0100
Copy link
Copy Markdown
Contributor

  • Add autils/system/cpu.py (line-for-line from avocado)
  • Add unit tests with architecture-specific cpuinfo fixtures
  • Add metadata/system/cpu.yml
  • Add cpu to docs under System section

@harvey0100 harvey0100 requested a review from richtja April 2, 2026 11:33
@harvey0100 harvey0100 self-assigned this Apr 2, 2026
@mr-avocado mr-avocado Bot moved this to Review Requested in Default project Apr 2, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new autils.system.cpu module for querying and managing CPU information across various architectures, including x86_64, powerpc, s390, and aarch64. It provides functions for architecture detection, vendor identification, online/offline status management, and NUMA topology. Feedback focuses on improving the robustness of parsing /proc/cpuinfo and /proc/<pid>/task/<tid>/stat files, as well as optimizing the collection of NUMA node CPU assignments.

Comment thread autils/system/cpu.py
proc_cpuinfo = genio.read_file("/proc/cpuinfo")
for line in proc_cpuinfo.splitlines():
if "revision" in line:
rev = line.split(" ")[3].strip()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation of get_revision uses a fixed index after splitting by a single space, which is highly fragile. In many /proc/cpuinfo implementations (especially on non-x86 architectures like POWER), the number of spaces between the key and the value can vary, leading to an empty string or an IndexError. It is safer to split by the colon character and then extract the first word of the value.

Suggested change
rev = line.split(" ")[3].strip()
rev = line.split(":", 1)[1].strip().split()[0]

Comment thread autils/system/cpu.py
Comment on lines +615 to +617
with open(proc_stat_file) as proc_stat: # pylint: disable=W1514
cpus.add(proc_stat.read().split(" ")[processor_id_index])
except IOError:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Parsing /proc/<pid>/task/<tid>/stat by splitting with spaces and using a fixed negative index is unreliable. The process name (the second field in stat) is enclosed in parentheses and can contain spaces, which shifts the index of all subsequent fields. A more robust approach is to find the last closing parenthesis to correctly identify the end of the process name field before splitting the remaining fields.

            with open(proc_stat_file) as proc_stat:  # pylint: disable=W1514
                data = proc_stat.read()
                # The comm field (2nd) is in parens and can have spaces.
                # Find the last ')' to safely split the fields following it.
                fields = data[data.rfind(")") + 2 :].split()
                # The 'processor' field is the 39th field in the file.
                # Since we split after the 2nd field, the index is 39 - 3 = 36.
                cpus.add(fields[36])
        except (IOError, IndexError):

Comment thread autils/system/cpu.py
Comment on lines +655 to +657
def _deprecated(newfunc, oldfuncname):
"""Print a deprecation warning and return the new function.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The check if pinned_cpus is not None: is redundant because pinned_cpus is initialized as an empty list just before the loop. Furthermore, updating the numa_nodes_with_cpus dictionary inside the inner loop is inefficient; it should be updated once per NUMA node after all its CPUs have been collected.

            pinned_cpus.append(cpu)
        numa_nodes_with_cpus[node] = sorted(pinned_cpus)

- Add autils/system/cpu.py (line-for-line from avocado)
- Add unit tests with architecture-specific cpuinfo fixtures
- Add metadata/system/cpu.yml
- Add cpu to docs under System section

Assisted-By: Cursor-Claude-4-Sonnet
Signed-off-by: Harvey Lynden <hlynden@redhat.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (81428c5) to head (3bdf5ff).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #105   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review Requested

Development

Successfully merging this pull request may close these issues.

1 participant