Fix libvirtd start issue #2519
Fix libvirtd start issue #2519
Conversation
WalkthroughBoth files had SR-IOV (Single Root I/O Virtualization) related checks and error handling removed. The pcicheck.php script now only performs PCI device change detection, while the qemu script removes the SR-IOV branch from its PCI checking logic. Both changes simplify their respective control flows. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-11-14T15:18:55.567ZApplied to files:
📚 Learning: 2025-12-28T15:54:58.673ZApplied to files:
✏️ Tip: You can disable this entire section by setting 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. Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
emhttp/plugins/dynamix.vm.manager/scripts/pcicheck.php (2)
17-19: Avoid corrupting “yes/no” output with CLI$_SERVER['DOCUMENT_ROOT']noticesWhen run via CLI,
$_SERVER['DOCUMENT_ROOT']is commonly unset; if notices are displayed, they’ll pollute stdout and can break any consumer expecting exactlyyes/no.Proposed fix
-$docroot ??= ($_SERVER['DOCUMENT_ROOT'] ?: '/usr/local/emhttp'); +$docroot ??= (($_SERVER['DOCUMENT_ROOT'] ?? '') !== '' ? $_SERVER['DOCUMENT_ROOT'] : '/usr/local/emhttp'); require_once "$docroot/webGui/include/Helpers.php";
23-35: Make output parsing robust: drop== true, add newline, and guard$argvProposed fix
-$pci_addresses = []; -foreach ($argv as $arg) { +$pci_addresses = []; +foreach (($argv ?? []) as $arg) { if (preg_match('/"host"\s*:\s*"([^"]+)"/', $arg, $matches)) { $pci_addresses[] = $matches[1]; } } foreach($pci_addresses as $pciid) { if (isset($pci_device_changes[$pciid])) { $pcierror = true; + break; } } -echo $pcierror == true ? "yes" : "no"; +echo $pcierror ? "yes\n" : "no\n";
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix.vm.manager/scripts/pcicheck.phpsbin/qemu
💤 Files with no reviewable changes (1)
- sbin/qemu
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: SimonFair
Repo: unraid/webgui PR: 2443
File: emhttp/plugins/dynamix/include/SriovHelpers.php:16-16
Timestamp: 2025-11-14T15:18:55.567Z
Learning: In the file emhttp/plugins/dynamix/include/SriovHelpers.php, the global variable $allowedPCIClass is defined and is used in emhttp/plugins/dynamix/include/SysDevs.php which includes SriovHelpers.php. This variable should not be flagged as unused.
📚 Learning: 2025-11-14T15:18:55.567Z
Learnt from: SimonFair
Repo: unraid/webgui PR: 2443
File: emhttp/plugins/dynamix/include/SriovHelpers.php:16-16
Timestamp: 2025-11-14T15:18:55.567Z
Learning: In the file emhttp/plugins/dynamix/include/SriovHelpers.php, the global variable $allowedPCIClass is defined and is used in emhttp/plugins/dynamix/include/SysDevs.php which includes SriovHelpers.php. This variable should not be flagged as unused.
Applied to files:
emhttp/plugins/dynamix.vm.manager/scripts/pcicheck.php
📚 Learning: 2025-12-28T15:54:58.673Z
Learnt from: mgutt
Repo: unraid/webgui PR: 2496
File: emhttp/plugins/dynamix/Browse.page:901-906
Timestamp: 2025-12-28T15:54:58.673Z
Learning: In the unraid/webgui codebase, CSRF validation is centralized in the global auto_prepend_file (local_prepend.php) which runs before every PHP request. Do not add per-script CSRF checks in individual files like Browse.page or Control.php. If a script relies on global validation, ensure it does not duplicate CSRF logic; otherwise extend the central preface to cover the needed checks.
Applied to files:
emhttp/plugins/dynamix.vm.manager/scripts/pcicheck.php
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.