Fixing maxvcpu mismatch with topology issue, making it dynamic!

[Anushree-Mathur]

Maxvcpu is hardcoded for the specific testcase in virsh_emulatorpin.py. When smp value, vcpu_cores, vcpu_threads, vcpu_sockets is specified at the time of running the testcase then the hardcoded value will fail. Adding the code to get this value dynamically based on the smp value provided in the test suite config file.
1 example is that hardcoded value is 7 in virsh_emulatorpin.cfg but in the testsuite config smp value given is 32 not topology will be affected in this case my check will help.

Signed-off-by: Anushree-Mathur anushree.mathur@linux.ibm.com

Summary by CodeRabbit

• Tests
  • Enhanced vCPU configuration handling to properly compute maximum vCPUs from available CPU topology information when processing emulator pin attributes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The change modifies the run() function in the test file to add conditional logic for computing vCPU attributes. When vcpu_attrs contains a cpuset parameter, the code now checks if CPU topology information is available through vmxml.get_cpu_topology(). If topology data exists, it calculates the maximum vCPU count by multiplying sockets, cores, and threads values, then assigns this computed value to vcpu_attrs['vcpu'] before applying the attributes to the domain.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• The CPU topology calculation logic and its necessity within the test context
• Verification that the sockets \* cores \* threads formula is the appropriate way to compute max vCPUs
• Edge cases when get_cpu_topology() returns None or incomplete data

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title refers to fixing a maxvcpu mismatch with topology and making it dynamic, which aligns with the main change: computing maxvcpu dynamically from CPU topology instead of using hardcoded values.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

libvirt/tests/src/virsh_cmd/domain/virsh_emulatorpin.py (1)

271-281: Dynamic max_vcpu calculation looks correct; consider caching topology and tightening error handling

The new logic to derive max_vcpu from sockets * cores * threads when vmxml.get_cpu_topology() is present is consistent with the intent to match maxvcpu to the runtime smp/topology and should fix the mismatch you described for cases like smp=32.

Two small, non-blocking improvements you might consider:

• Avoid calling vmxml.get_cpu_topology() twice and cache the result, both for clarity and to ensure you’re working with a single, well-defined snapshot:

topology = vmxml.get_cpu_topology()
if topology:
    max_vcpu = int(topology['sockets']) * int(topology['cores']) * int(topology['threads'])
    vcpu_attrs['vcpu'] = max_vcpu

• For robustness, think about how you want to behave if topology is present but missing any of sockets/cores/threads or contains unexpected values. Right now that would raise a generic KeyError/ValueError and fail the test without a clear message; you might prefer either a more explicit test.fail() or to log and skip updating vcpu_attrs['vcpu'] in such a case.

Also, this dynamic update only runs when vcpu_attrs['cpuset'] == "x,y". Please double‑check that all scenarios where smp / topology can vary and previously hit the hard‑coded maxvcpu=7 do go through this same cpuset placeholder path; otherwise, some configs might still rely on static values in the .cfg.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a302a6f and a697845.

📒 Files selected for processing (1)

• libvirt/tests/src/virsh_cmd/domain/virsh_emulatorpin.py (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Python 3.11
• GitHub Check: Python 3.8
• GitHub Check: Python 3.12
• GitHub Check: Python 3.9

[Anushree-Mathur]

1 scenario example:
The guest is taking the maxvcpus as "7" and in config smp value passed was "32". After applying the patch if values are mentioned in the testsuite config file then only topology will be true, so it will take maxvcpu value as 32 in guest.

Before applying the patch:

14:11:58 WARNING : Overriding user setting and enabling kvm bootstrap as guest tests are requested
14:11:59 INFO    : Check for environment
14:12:01 INFO    : Creating temporary mux dir
14:12:01 INFO    :
14:12:01 INFO    : Running Guest Tests Suite cpu_emulator
14:12:01 INFO    : Running: /usr/local/bin/avocado run --vt-type libvirt --vt-config /home/Anu/tests/data/avocado-vt/backends/libvirt/cfg/cpu_emulator.cfg                 --force-job-id 6ea6343c9baf5f302a94366b8972ac037ddbe24f                 --job-results-dir /home/Anu/tests/results  --vt-only-filter                                             "virtio_scsi virtio_net qcow2 Fedora.43.ppc64le"
JOB ID     : 6ea6343c9baf5f302a94366b8972ac037ddbe24f
JOB LOG    : /home/Anu/tests/results/job-2025-11-23T14.12-6ea6343/job.log
 (1/3) io-github-autotest-qemu.unattended_install.import.import.default_install.aio_native: STARTED
 (1/3) io-github-autotest-qemu.unattended_install.import.import.default_install.aio_native: PASS (53.87 s)
 (2/3) type_specific.io-github-autotest-libvirt.virsh.emulatorpin.positive_testing.set_emulatorpin_parameter.running_guest.cpulist.emulatorpin_options.current.single.start_with_cpuset_config: STARTED
 (2/3) type_specific.io-github-autotest-libvirt.virsh.emulatorpin.positive_testing.set_emulatorpin_parameter.running_guest.cpulist.emulatorpin_options.current.single.start_with_cpuset_config: ERROR: Failed to define avocado-vt-vm1 for reason:\nerror: Failed to define domain from /var/tmp/xml_utils_temp_mdextk0i.xml\nerror: unsupported configuration: CPU topology doesn't match maximum vcpu count\n (19.78 s)
 (3/3) io-github-autotest-libvirt.remove_guest.without_disk: STARTED
 (3/3) io-github-autotest-libvirt.remove_guest.without_disk: PASS (4.37 s)

After applying the patch:

05:04:53 WARNING : Overriding user setting and enabling kvm bootstrap as guest tests are requested
05:04:53 INFO    : Check for environment
05:04:55 INFO    : Creating temporary mux dir
05:04:55 INFO    :
05:04:55 INFO    : Running Guest Tests Suite cpu_emulator
05:04:55 INFO    : Running: /usr/local/bin/avocado run --vt-type libvirt --vt-config /home/Anu/tests/data/avocado-vt/backends/libvirt/cfg/cpu_emulator.cfg                 --force-job-id 9a9cf9e29a41f530f7ec05af53fa4a482092012b                 --job-results-dir /home/Anu/tests/results  --vt-only-filter                                             "virtio_scsi virtio_net qcow2 Fedora.43.ppc64le"
JOB ID     : 9a9cf9e29a41f530f7ec05af53fa4a482092012b
JOB LOG    : /home/Anu/tests/results/job-2025-11-22T05.04-9a9cf9e/job.log
 (1/3) io-github-autotest-qemu.unattended_install.import.import.default_install.aio_native: STARTED
 (1/3) io-github-autotest-qemu.unattended_install.import.import.default_install.aio_native: PASS (40.11 s)
 (2/3) type_specific.io-github-autotest-libvirt.virsh.emulatorpin.positive_testing.set_emulatorpin_parameter.running_guest.cpulist.emulatorpin_options.current.single.start_with_cpuset_config: STARTED
 (2/3) type_specific.io-github-autotest-libvirt.virsh.emulatorpin.positive_testing.set_emulatorpin_parameter.running_guest.cpulist.emulatorpin_options.current.single.start_with_cpuset_config: PASS (23.78 s)
 (3/3) io-github-autotest-libvirt.remove_guest.without_disk: STARTED
 (3/3) io-github-autotest-libvirt.remove_guest.without_disk: PASS (4.30 s)