Skip to content

fix(PCI and VM Usage) fixes for PCI speed and VM usage cpu reporting.#2636

Draft
SimonFair wants to merge 5 commits into
unraid:masterfrom
SimonFair:ZFS-reserved-name-check-
Draft

fix(PCI and VM Usage) fixes for PCI speed and VM usage cpu reporting.#2636
SimonFair wants to merge 5 commits into
unraid:masterfrom
SimonFair:ZFS-reserved-name-check-

Conversation

@SimonFair
Copy link
Copy Markdown
Contributor

@SimonFair SimonFair commented May 13, 2026

For Intel gpus look up the pci tree to get speed for Intel ARC gpus which lspci reports as x1
Refine CPU calculation for VM usage.
Consistent VM disk and network rate reporting (no unintended scaling), including adjusted RX rate metric.
Remove VM reboot message as managed by boot parameters now.

Summary by CodeRabbit

  • Bug Fixes
    • Improved VM CPU usage reporting with robust guest/host timing and clamped percentages.
    • Consistent VM disk and network rate reporting (removed unintended interval scaling), plus an adjusted RX rate metric.
    • More reliable PCIe link speed/width reporting by preferring better upstream link data for certain devices.
    • Removed redundant reboot-required check in VM settings.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d0722b3a-f58d-4cf4-bc95-dbdbc40ea5fd

📥 Commits

Reviewing files that changed from the base of the PR and between 58b0a87 and 2936797.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix.vm.manager/include/libvirt_helpers.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • emhttp/plugins/dynamix.vm.manager/include/libvirt_helpers.php

Walkthrough

This PR refactors VM usage sampling to use timestamps and absolute-counter deltas (host and optional guest vCPU times), makes disk/network rates interval-aware and computes an adjusted RX rate, adds PCIe link parsing and ancestor-fallback logic for more accurate endpoint reporting (special-casing Intel GPUs/misreports), removes a redundant VM-settings reboot AJAX call, and fixes a libvirt error-check path. UI publishing/display of rates now uses raw collected rates (no timer scaling).

Changes

VM usage sampling and deltas

Layer / File(s) Summary
Sampling, CPU/disk/net delta computation
emhttp/plugins/dynamix.vm.manager/include/libvirt_helpers.php
Implements microtime-based sampling in get_vm_usage_stats(), computes host and aggregated guest CPU deltas/percentages from absolute counters with clamping, makes disk/network rate deltas interval-aware and guarded by prior samples, adds rxrateadj using accepted-packet ratio, and persists new guest/network fields into the per-VM vmusagestats cache.

PCIe Link Detection Enhancement

Layer / File(s) Summary
PCIe link parsing and ancestry helpers
emhttp/plugins/dynamix/include/Helpers.php
Adds parsePciSpeedValue(), parsePciWidthValue(), readPciLinkValuesFromPath(), and getBestPciePortAncestorLink() to parse GT/s and lane-width text, read sysfs link metrics, and traverse/score PCI ancestors.
getPciLinkInfo device-specific fallback logic
emhttp/plugins/dynamix/include/Helpers.php
getPciLinkInfo() now reads endpoint values first, detects Intel GPUs or suspicious low endpoint metrics, fetches upstream pcieport metrics via the new helpers, compares quality scores, and conditionally replaces endpoint current_*/max_* (including raw width fields) with upstream values when upstream is better.

UI publish/display consistency

Layer / File(s) Summary
Remove reboot AJAX call
emhttp/plugins/dynamix.vm.manager/VMSettings.page
Deleted the AJAX VMajax.php?action:'reboot' request that toggled a reboot-required notice; flow now continues to btrfs scrub cookie handling.
Use raw collected rates for display
emhttp/plugins/dynamix.vm.manager/nchan/vm_usage, emhttp/plugins/dynamix/nchan/vm_dashusage
Stop dividing rdrate/wrrate/rxrate/txrate by $timer when rendering/publishing so UI values match collected metric units.

Libvirt error handling

Layer / File(s) Summary
Strict-false handling for libvirt stats
emhttp/plugins/dynamix.vm.manager/include/libvirt.php
domain_get_all_domain_stats() checks for === false from libvirt_connect_get_all_domain_stats() and returns _set_last_error() on that explicit failure path.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🐰
I count the ticks, I trace each thread,
Guest cycles summed and network read,
Upstream links I chase and pick,
Ajax trimmed, reports now click,
Metrics sing — a tidy hop ahead.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title mentions 'PCI speed' and 'VM usage cpu reporting' which align with substantive changes across multiple files (PCI link detection, CPU stats computation, network metrics), making it specific and relevant to the main objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2026.05.15.0838
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

  1. Go to Plugins → Install Plugin
  2. Copy and paste this URL:
https://preview.dl.unraid.net/pr-plugins/pr-2636/webgui-pr-2636.plg
  1. Click Install

Alternative: Direct Download

⚠️ Important Notes:

  • Testing only: This plugin is for testing PR changes
  • Backup included: Original files are automatically backed up
  • Easy removal: Files are restored when plugin is removed
  • Conflicts: Remove this plugin before installing production updates
  • Post-merge behavior: This preview stays available after merge until preview storage expires or it is manually cleaned up

📝 Modified Files:

Click to expand file list
emhttp/plugins/dynamix.vm.manager/VMSettings.page
emhttp/plugins/dynamix.vm.manager/include/libvirt.php
emhttp/plugins/dynamix.vm.manager/include/libvirt_helpers.php
emhttp/plugins/dynamix.vm.manager/nchan/vm_usage
emhttp/plugins/dynamix/include/Helpers.php
emhttp/plugins/dynamix/nchan/vm_dashusage

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2636, or run:

plugin remove webgui-pr-2636

🤖 This comment is automatically generated and will be updated with each new push to this PR.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@emhttp/plugins/dynamix.vm.manager/include/libvirt_helpers.php`:
- Around line 2804-2806: The current logic sets $currentmem to 0 when
balloon.current is missing, causing meminuse to be clamped to 0; change the
fallback so $currentmem uses balloon.maximum if balloon.current is not provided,
and if neither limit exists, allow RSS to be used directly. Concretely, update
the computation around $currentmem/$maximummem/$meminuse in libvirt_helpers.php
so $currentmem = (int)($data['balloon.current'] ?? $data['balloon.maximum'] ??
0) (or equivalent), then compute $meminuse from min($rss, $currentmem) but if
$currentmem is 0 use $rss directly; reference the variables $currentmem,
$maximummem, $meminuse and the $data['balloon.rss'] / $data['balloon.maximum']
entries when making the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4244794b-bf26-4be3-a664-0a40e1aa8907

📥 Commits

Reviewing files that changed from the base of the PR and between 577c5ab and 58b0a87.

📒 Files selected for processing (4)
  • emhttp/plugins/dynamix.vm.manager/include/libvirt.php
  • emhttp/plugins/dynamix.vm.manager/include/libvirt_helpers.php
  • emhttp/plugins/dynamix.vm.manager/nchan/vm_usage
  • emhttp/plugins/dynamix/nchan/vm_dashusage
✅ Files skipped from review due to trivial changes (1)
  • emhttp/plugins/dynamix.vm.manager/nchan/vm_usage

Comment thread emhttp/plugins/dynamix.vm.manager/include/libvirt_helpers.php Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant