-
Notifications
You must be signed in to change notification settings - Fork 184
Add new cases and fix failures of v2v options job #6772
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.
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
base: master
Are you sure you want to change the base?
Add new cases and fix failures of v2v options job #6772
Conversation
WalkthroughAdds a new incompatible_cmd test for Windows guests in ESX test config and corresponding checkpoint handling in the ESX test runner to generate and run a small bash script via --run. Updates KVM test configs with two new variants (windows_temp, disk_subfolder) and adjusts variant inclusion/exclusion. Changes v2v options configs to alter qcow2 input paths and introduce memsize variants (including mem_alloc and wbox-like sections) with specific --memsize values. Removes the automatic verbose fallback (-v -x) from allocation option handling, leaving only quiet mode handling. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
@xiaodwan pls help review, thanks! (1/8) type_specific.io-github-autotest-libvirt.specific_kvm.positive_test.windows.default.windows_temp.esx.esx_80.output_mode.local.it_vddk: PASS (739.09 s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@v2v/tests/cfg/specific_kvm.cfg`:
- Around line 321-329: The disk_subfolder test variant is missing the boottype
parameter; update the disk_subfolder variant in the config (the block labeled
"disk_subfolder") to include boottype = 2 so it matches the windows_temp variant
and other Windows test variants, ensuring the entry reads with boottype = 2
alongside only esx_80, version_required, and main_vm.
In `@v2v/tests/src/function_test_esx.py`:
- Around line 1048-1056: The block under the if
'virt_customize_incompatible_cmd' in checkpoint has inconsistent indentation and
mis-indented script content; update the indentation to match the surrounding
blocks (use 4-space indentation for the if body and dedent the triple-quoted
script content so the "#!/bin/bash" and "dnf -y install libvirt" lines are not
prefixed by extra spaces), ensure the file write to script_path (from
data_dir.get_tmp_dir()) and the v2v_params['v2v_opts'] += " --run %s" %
script_path line are indented consistently with the other conditional blocks.
| - windows_temp: | ||
| only esx_80 | ||
| boottype = 2 | ||
| version_required = "[virt-v2v-2.7.1-17,)" | ||
| main_vm = VM_NAME_GUEST_WITH_INCORRECT_FOLDER_NAME_TEMP_V2V_EXAMPLE | ||
| - disk_subfolder: | ||
| only esx_80 | ||
| version_required = "[nbdkit-1.44.1-3,)" | ||
| main_vm = VM_NAME_GUEST_WITH_DISK_IN_SUBFOLDER_V2V_EXAMPLE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure and find the specific file
git ls-files | grep -E "(specific_kvm|v2v)" | head -20Repository: autotest/tp-libvirt
Length of output: 724
🏁 Script executed:
# Read the specific file to see the context around lines 321-329
wc -l v2v/tests/cfg/specific_kvm.cfgRepository: autotest/tp-libvirt
Length of output: 97
🏁 Script executed:
# Get the context around lines 321-329 to understand the full scope
sed -n '315,370p' v2v/tests/cfg/specific_kvm.cfgRepository: autotest/tp-libvirt
Length of output: 2463
🏁 Script executed:
# Search for other boottype usages in the file to understand the pattern
rg "boottype" v2v/tests/cfg/specific_kvm.cfg -B 2 -A 2Repository: autotest/tp-libvirt
Length of output: 2479
🏁 Script executed:
# Search for version_required patterns to understand versioning conventions
rg "version_required" v2v/tests/cfg/specific_kvm.cfg | head -20Repository: autotest/tp-libvirt
Length of output: 380
🏁 Script executed:
# Check if there are any test variants without boottype that are used in Windows context
rg "^ *- [a-z_]+:" v2v/tests/cfg/specific_kvm.cfg -A 5 | grep -E "^\s*- |boottype|only esx_80" | head -50Repository: autotest/tp-libvirt
Length of output: 1325
🏁 Script executed:
# Search for any documentation or comments about boottype requirements
rg -B 3 "boottype.*=" v2v/tests/cfg/specific_kvm.cfg | grep -E "^[^:]*:.*#|boottype" | head -30Repository: autotest/tp-libvirt
Length of output: 428
🏁 Script executed:
# Check if disk_subfolder variant is tested with any special handling
rg "disk_subfolder" v2v/tests/cfg/specific_kvm.cfg -B 2 -A 2Repository: autotest/tp-libvirt
Length of output: 730
Add boottype = 2 to the disk_subfolder variant.
The windows_temp variant correctly includes boottype = 2, and all similar test variants throughout this file consistently use this parameter. The disk_subfolder variant, which is used in the Windows test context alongside windows_temp, is missing this required parameter.
Fix needed:
- disk_subfolder:
only esx_80
boottype = 2
version_required = "[nbdkit-1.44.1-3,)"
main_vm = VM_NAME_GUEST_WITH_DISK_IN_SUBFOLDER_V2V_EXAMPLE
🤖 Prompt for AI Agents
In `@v2v/tests/cfg/specific_kvm.cfg` around lines 321 - 329, The disk_subfolder
test variant is missing the boottype parameter; update the disk_subfolder
variant in the config (the block labeled "disk_subfolder") to include boottype =
2 so it matches the windows_temp variant and other Windows test variants,
ensuring the entry reads with boottype = 2 alongside only esx_80,
version_required, and main_vm.
1. Add case to check unexpecter temp folder name in windows 2. Add case to check incompatiable option run in windows 3. Add case to secureboot info in guest xml 4. Add case to check disk in subfolder 5. Add case to test memsize option 6. Fix failures of v2v option job Signed-off-by: Super User <[email protected]>
ab4b9b9 to
ac56597
Compare
|
HI @mxie91 , I suggest you submit one PR only for one test case. It's hard to review them together. |
|
@xiaodwan Because these test cases are simple, I automated them all together. I'll create a separate PR for each test case next time,. Thank you for your suggestion. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.