Skip to content

libvirt_mem: Add vhost configuration for SLES #6886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sneh-3 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

libvirt_mem: Add vhost configuration for SLES #6886

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions libvirt/tests/src/libvirt_mem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,56 @@ def restore_hugepages(page_size=4):
config.restore()
utils_libvirtd.libvirtd_restart()

def setup_vhost_max_mem_regions():
"""
Configure vhost max_mem_regions for SLES systems.
SLES has lower default values compared to other distros.
This is required for memory hotplug tests with max_slots.
Must be called when no VM is using vhost modules.
This function applies runtime configuration only.
For persistent configuration across reboots, manually create:
/etc/modprobe.d/vhost.conf with content: options vhost max_mem_regions=512
"""
try:
# Check if running on SLES
distro_check = process.run("cat /etc/os-release | grep -i sles",
shell=True, ignore_status=True)
if distro_check.exit_status == 0:
logging.info("SLES detected, configuring vhost max_mem_regions=512")
vm_check = process.run("virsh list --state-running | grep -v 'Id\\|^$\\|^-'",
shell=True, ignore_status=True)
if vm_check.exit_status == 0 and vm_check.stdout_text.strip():
Comment on lines +135 to +137

@coderabbitai coderabbitai Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard command can misclassify virsh failures as “safe to proceed.”

At Line 135, the pipeline status reflects grep, not virsh. If virsh list fails, this branch can still fall through and proceed with module reload. Please split command execution so virsh failure is handled explicitly before parsing output.

Suggested fix 
-                vm_check = process.run("virsh list --state-running | grep -v 'Id\\|^$\\|^-'",
-                                      shell=True, ignore_status=True)
-                if vm_check.exit_status == 0 and vm_check.stdout_text.strip():
+                vm_check = process.run("virsh list --state-running --name",
+                                       shell=True, ignore_status=True)
+                if vm_check.exit_status != 0:
+                    logging.warning("Unable to determine running VMs; skipping vhost configuration")
+                    return
+                running_vms = [name for name in vm_check.stdout_text.splitlines() if name.strip()]
+                if running_vms:
                     logging.warning("Running VMs detected, skipping vhost configuration for safety:")
-                    logging.warning("%s", vm_check.stdout_text.strip())
+                    logging.warning("%s", "\n".join(running_vms))
                     return
🧰 Tools
🪛 Ruff (0.15.15)

[error] 135-135: Function call with shell=True parameter identified, security issue

(S604)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libvirt/tests/src/libvirt_mem.py` around lines 135 - 137, The current
pipeline in the vm_check assignment combines virsh and grep in a single command,
which means the exit_status will reflect grep's result rather than virsh's
failure. If virsh list fails but grep runs against empty output without error,
the condition will incorrectly assess the situation as safe to proceed. Split
the command execution into two separate steps: first run virsh list
--state-running independently and check its exit status, then only proceed to
parse the output with grep if virsh succeeds. This ensures virsh failures are
explicitly caught before attempting to process output.

logging.warning("Running VMs detected, skipping vhost configuration for safety:")
logging.warning("%s", vm_check.stdout_text.strip())
return

logging.info("No running VMs detected, proceeding with vhost configuration")
process.run("modprobe -r vhost_net vhost_scsi vhost_vsock vhost",
shell=True, ignore_status=True)
result = process.run("modprobe vhost max_mem_regions=512",
shell=True, ignore_status=True)

if result.exit_status == 0:
logging.info("Successfully configured vhost max_mem_regions=512 at runtime")
verify_cmd = "cat /sys/module/vhost/parameters/max_mem_regions"
verify_result = process.run(verify_cmd, shell=True, ignore_status=True)
if verify_result.exit_status == 0:
current_value = verify_result.stdout_text.strip()
logging.info("Verified vhost max_mem_regions is set to: %s", current_value)
if current_value != "512":
logging.warning("Expected 512 but got %s", current_value)
else:
logging.debug("Could not verify vhost max_mem_regions setting")
else:
logging.warning("Failed to configure vhost at runtime: %s",
result.stderr_text)
else:
logging.debug("Non-SLES system detected, skipping vhost configuration")

except Exception as e:
logging.warning("Error setting up vhost max_mem_regions: %s", str(e))
Comment on lines +148 to +166

@coderabbitai coderabbitai Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Setup precondition failures are downgraded to warnings, so the test can continue in invalid state.

At Line 155-156 and Line 165-166, failed verification and unexpected exceptions only log warnings. For SLES-targeted setup, this should fail/cancel the test path instead of silently continuing.

Suggested fix 
-                        if current_value != "512":
-                            logging.warning("Expected 512 but got %s", current_value)
+                        if current_value != "512":
+                            test.cancel("vhost max_mem_regions verification failed: expected 512, got %s"
+                                        % current_value)
...
-                else:
-                    logging.warning("Failed to configure vhost at runtime: %s",
-                                  result.stderr_text)
+                else:
+                    test.cancel("Failed to configure vhost at runtime: %s" % result.stderr_text)
...
-        except Exception as e:
-            logging.warning("Error setting up vhost max_mem_regions: %s", str(e))
+        except Exception as e:
+            test.cancel("Error setting up vhost max_mem_regions: %s" % str(e))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if result.exit_status == 0:
logging.info("Successfully configured vhost max_mem_regions=512 at runtime")
verify_cmd = "cat /sys/module/vhost/parameters/max_mem_regions"
verify_result = process.run(verify_cmd, shell=True, ignore_status=True)
if verify_result.exit_status == 0:
current_value = verify_result.stdout_text.strip()
logging.info("Verified vhost max_mem_regions is set to: %s", current_value)
if current_value != "512":
logging.warning("Expected 512 but got %s", current_value)
else:
logging.debug("Could not verify vhost max_mem_regions setting")
else:
logging.warning("Failed to configure vhost at runtime: %s",
result.stderr_text)
else:
logging.debug("Non-SLES system detected, skipping vhost configuration")
except Exception as e:
logging.warning("Error setting up vhost max_mem_regions: %s", str(e))
if result.exit_status == 0:
logging.info("Successfully configured vhost max_mem_regions=512 at runtime")
verify_cmd = "cat /sys/module/vhost/parameters/max_mem_regions"
verify_result = process.run(verify_cmd, shell=True, ignore_status=True)
if verify_result.exit_status == 0:
current_value = verify_result.stdout_text.strip()
logging.info("Verified vhost max_mem_regions is set to: %s", current_value)
if current_value != "512":
test.cancel("vhost max_mem_regions verification failed: expected 512, got %s"
% current_value)
else:
logging.debug("Could not verify vhost max_mem_regions setting")
else:
test.cancel("Failed to configure vhost at runtime: %s" % result.stderr_text)
else:
logging.debug("Non-SLES system detected, skipping vhost configuration")
except Exception as e:
test.cancel("Error setting up vhost max_mem_regions: %s" % str(e))
🧰 Tools
🪛 Ruff (0.15.15)

[error] 151-151: Function call with shell=True parameter identified, security issue

(S604)

[warning] 165-165: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libvirt/tests/src/libvirt_mem.py` around lines 148 - 166, The verification
failure for vhost max_mem_regions configuration and the exception handler are
downgrading setup errors to warnings, allowing the test to continue in an
invalid state. When verification confirms the value is not set to "512" (around
line 155-156), raise an exception instead of just logging a warning. In the
exception handler (around line 165-166), re-raise the caught exception instead
of logging a warning and continuing, so that SLES-targeted setup failures
properly fail or cancel the test path rather than silently proceeding.



def check_qemu_cmd(max_mem_rt, tg_size):
"""
Check qemu command line options.
Expand Down Expand Up @@ -528,6 +578,8 @@ def modify_domain_xml():
# Destroy domain first
if vm.is_alive():
vm.destroy(gracefully=False)
# Configure vhost max_mem_regions for SLES (VM is now stopped)
setup_vhost_max_mem_regions()
modify_domain_xml()
numa_info = utils_misc.NumaInfo()
logging.debug(numa_info.get_all_node_meminfo())
Expand Down