[weizhouapache]

Description

The disk stats of a volume are matched on the last part of the path of its disk. For local storage, whose disks are in /var/lib/libvirt/images, the agent reported 'libvirt' instead of the path of the volume. The stats of the volume were never found, so its read and write counters stayed at 0.

Use the last element of the path of the disk, which is the same as before for the other storage types. Also skip, instead of stopping at, a disk which does not match a volume, so the stats of the other disks of the Instance are still collected.

Likewise, keep checking the other volumes of a storage pool after one with a format for which the volume stats are not implemented, instead of stopping at the first one.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• Build/CI
• Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

[weizhouapache]

I tested this on KVM with the agent patched on one host, using a VM that has data disks on every storage type I could set up. This was on a 4.23.0 build with the same change; I have not run it on a 4.22 build.

Cause

The disk stats of a volume are matched on getDiskPathFromDiskDef(), which returned the 4th element of the path. That is the volume path for /mnt/<pool>/<uuid> (NFS, SharedMountPoint) and for RBD, but for local storage (/var/lib/libvirt/images/<uuid>) it is libvirt. The volume is never found, so its counters stay at 0. Also, in VmDiskStatsTask the first disk that does not match a volume ends the for loop (break), so the other disks of the VM can be skipped too.

Now the last element of the path is used (same value as before for the other pool types), and an unmatched disk is skipped (continue). The second change, in VolumeStatsTask, only makes the "stats not implemented for this format" warning appear for every such volume and not only for the first.

Test: before and after the fix

One VM on one KVM host, a root disk plus these data disks, 600 s of a fixed-rate fio load (about 140 read + 60 write requests/s) on each data disk, vm.disk.stats.interval = 300. Values are listVolumesMetrics after the run, compared with virsh domblkstat on the host.

Volume	Storage	diskioread	diskiowrite	"Disk IOPS"	libvirt rd_req + wr_req	Without the fix
nfsdisk	NFS	84,129	36,000	120,129	120,129	same
cephdisk	RBD	84,128	36,000	120,128	120,128	same
localdisk	local	168,214	72,000	240,214	240,214	0 / 0 / 0

(The local volume was collected after the fix, so its totals include the second run of the same load. Before the fix, it stayed at 0 with libvirt showing the same load on it as on the NFS and RBD disks.)

Test: different amounts on each disk, 5 data disks

Then I wrote a different number of 4 KiB writes to each disk (fio, direct I/O) and compared the values 5 minutes later:

Volume	Storage	Writes added	diskiowrite	diskkbswrite	libvirt wr_req	Match
nfsdisk	NFS	4,000	86,240	344,960	86,240	yes
cephdisk	RBD	8,000	90,240	360,960	90,240	yes
localdisk	local	12,000	94,240	376,960	94,240	yes
smpdisk1	SharedMountPoint (NFS share)	16,000	62,240	248,960	62,240	yes
smpdisk2	SharedMountPoint (NFS share)	20,000	66,240	264,960	66,240	yes

diskioread, diskkbsread, diskiopstotal and diskkbswrite also match libvirt on all five disks (diskkbswrite = writes × 4 KiB).

Unit tests

• LibvirtComputingResourceTest: 316 tests pass, including the new testGetDiskPathFromDiskDefForLocalStorage (/var/lib/libvirt/images/filepath → filepath); the existing NFS, RBD and unsupported-path tests still pass.
• StatsCollectorTest passes.

Not tested

• Linstor, StorPool and CLVM disks: I did not check the path format of their libvirt disks.
• A root disk on local storage, and the case where an unmatched disk comes first in the list.
• VMware and XenServer (the change only touches the KVM path parsing and the loop control).

Related to #14302 (separate from the "Disk IOPS" display issue discussed there).

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.07%. Comparing base (a7697fe) to head (7d05893).

Files with missing lines	Patch %	Lines
...src/main/java/com/cloud/server/StatsCollector.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.22   #14454   +/-   ##
=========================================
  Coverage     18.06%   18.07%           
- Complexity    16356    16361    +5     
=========================================
  Files          5937     5937           
  Lines        537015   537015           
  Branches      65830    65830           
=========================================
+ Hits          97026    97053   +27     
+ Misses       428913   428879   -34     
- Partials      11076    11083    +7     

Flag	Coverage Δ
uitests	4.04% <ø> (ø)
unittests	19.14% <33.33%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.