Skip to content

Inc_backup: fix a size check failure #6660

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

Merged
merged 1 commit into from
Nov 30, 2025
Merged

Inc_backup: fix a size check failure #6660

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
34 changes: 23 additions & 11 deletions libvirt/tests/src/incremental_backup/incremental_backup_pull_mode_info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,30 @@ def check_scratch_info(scratch_device):
if scratch_device in three_domstats_output.stdout_text:
test.fail("The scratch device is still existed which is not expected!")

def check_domblk_info(target_min, target_max):
def check_domblk_info(dd_count, dd_seek):
"""
Check the domblk info.
Check the domain block info using domblkinfo command.

:params target_min: the min value of the block allocation
:params target_max: the max value of the block allocation
This function verifies that the block allocation of the target disk
matches the expected allocation based on the data written to the disk.

:param dd_count: The count parameter used in dd command
:param dd_seek: The seek parameter used in dd command
:raises: test.fail if block allocation is not within expected range
"""
test.log.debug("TEST STEP: check the domblk info")
domblk_output = virsh.domblkinfo(vm_name, target_disk, debug=True)
block_allocation = 0
for line in domblk_output.stdout_text.splitlines():
if "Allocation" in line:
block_allocation = int(line.split(":")[-1].strip())
block_allocation = float(line.split(":")[-1].strip())
if "Capacity" in line:
disk_capacity_bytes = float(line.split(":")[-1].strip())
capacity_MB = disk_capacity_bytes / 1024 / 1024
write_end_MB = int(dd_seek) + int(dd_count)
expected_MB = min(write_end_MB, capacity_MB)
target_min = expected_MB * 0.8
target_max = expected_MB * 1.2
block_allocation = block_allocation / 1024 / 1024
Comment on lines 141 to +151
Copy link

@coderabbitai coderabbitai bot Nov 27, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential NameError if domblkinfo output is missing expected fields.

If the domblkinfo output lacks "Allocation" or "Capacity" lines, block_allocation or disk_capacity_bytes will be unbound, causing a NameError at line 146 or 151. Consider initializing these variables before the loop or adding a guard.

         test.log.debug("TEST STEP: check the domblk info")
         domblk_output = virsh.domblkinfo(vm_name, target_disk, debug=True)
+        block_allocation = None
+        disk_capacity_bytes = None
         for line in domblk_output.stdout_text.splitlines():
             if "Allocation" in line:
                 block_allocation = float(line.split(":")[-1].strip())
             if "Capacity" in line:
                 disk_capacity_bytes = float(line.split(":")[-1].strip())
+        if block_allocation is None or disk_capacity_bytes is None:
+            test.fail("Failed to parse Allocation or Capacity from domblkinfo output")
         capacity_MB = disk_capacity_bytes / 1024 / 1024

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In libvirt/tests/src/incremental_backup/incremental_backup_pull_mode_info.py
around lines 141 to 151, the parsing loop assumes "Allocation" and "Capacity"
are always present which can leave block_allocation or disk_capacity_bytes
undefined; initialize block_allocation and disk_capacity_bytes to None (or
sensible defaults) before the loop and after the loop check for None and either
raise a clear exception or handle the missing values (e.g., skip the test, set
defaults, or fail with a message) so you avoid a NameError and provide a
deterministic failure mode.

if not (target_min <= block_allocation <= target_max):
test.fail("The block allocation %s is not expected! It's not between %s and %s."
% (block_allocation, target_min, target_max))
Expand Down Expand Up @@ -169,7 +180,7 @@ def check_domblk_info(target_min, target_max):
test.log.debug("TEST STEP2: write datas to the guest disk.")
write_datas(dd_seek)
if domblkinfo_check:
check_domblk_info(target_min, target_max)
check_domblk_info(dd_count, dd_seek)
test.log.debug("TEST STEP3: prepare the backup xml.")
backup_options, scratch_device = prepare_backup_xml()
test.log.debug("TEST STEP4: start the backup job.")
Expand All @@ -180,11 +191,12 @@ def check_domblk_info(target_min, target_max):
if not domblkinfo_check:
check_scratch_info(scratch_device)
else:
check_domblk_info(target_min, target_max)
write_datas(dd_seek="300")
check_domblk_info(target_min * 2, target_max * 2)
check_domblk_info(dd_count, dd_seek)
dd_seek = 300
write_datas(dd_seek)
check_domblk_info(dd_count, dd_seek)
virsh.domjobabort(vm_name, debug=True, ignore_status=False)
check_domblk_info(target_min * 2, target_max * 2)
check_domblk_info(dd_count, dd_seek)

finally:
if vm.is_alive():
Expand Down