Virtual disk: add 2 new datastore cases #6183
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
meinaLi
force-pushed
the
datastore_start
branch
from
80fd297 to
76447c7
Compare
|
chunfuwen
reviewed
Feb 21, 2025
| if with_hotplug: | ||
| if not vm.is_alive(): | ||
| vm.start() | ||
| _, new_image_path = disk_obj.prepare_disk_obj(disk_type, disk_dict, new_image_path="", extra=data_file_option) |
There was a problem hiding this comment.
line 51 to line 55 ,please put them into one small method to make code clean
There was a problem hiding this comment.
Move to hotplug_disk().
chunfuwen
reviewed
Feb 21, 2025
| if not vm.is_alive(): | ||
| vm.start() | ||
| vm_session = vm.wait_for_login() | ||
| test.log.info("TEST_STEP: Check the disk dumpxml.") |
There was a problem hiding this comment.
line 61 to line 68 ,please put them into one method e.g. validate_attached_datastore()
There was a problem hiding this comment.
Move to check_result().
meinaLi
force-pushed
the
datastore_start
branch
2 times, most recently
from
9995ab1 to
9ce8594
Compare
fangge1212
reviewed
Mar 6, 2025
| Start guest with datastore and hotplug disk with datastore. | ||
| """ | ||
| def prepare_vm(): | ||
| """ |
There was a problem hiding this comment.
I think it would be more appropriate to say " prepare_disk" here
fangge1212
reviewed
Mar 6, 2025
| disk_xml = virsh.dumpxml(vm_name, "--xpath //disk", debug=True).stdout_text | ||
| if "dataStore" not in disk_xml: | ||
| test.fail("Can't get the datastore element automatically!") | ||
| test.log.info("TEST_STEP: Do rear/write for disk.") |
There was a problem hiding this comment.
A typo here: s/rear/read
fangge1212
reviewed
Mar 6, 2025
| if with_hotplug: | ||
| test.log.info("TEST_STEP: Hotplug disk with datastore.") | ||
| new_image_path, disk_dev = hotplug_disk(data_file_option) | ||
| else: |
There was a problem hiding this comment.
Better to also log the TEST_STEP here, right?
fangge1212
reviewed
Mar 6, 2025
| test.log.info("TEST_STEP: Check the disk dumpxml.") | ||
| check_result() | ||
| if with_hotplug: | ||
| virsh.detach_device(vm_name, disk_dev.xml, debug=True, ignore_status=False, |
There was a problem hiding this comment.
Better to log the TEST_STEP here for hotunplug
fangge1212
reviewed
Mar 6, 2025
| finally: | ||
| vmxml_backup.sync() | ||
| disk_obj.cleanup_disk_preparation(disk_type) | ||
| if with_block: |
There was a problem hiding this comment.
Isn't cleanup_disk_preparation enough to clean up the disk? Why do we need the following lines?
Automate two cases: VIRT-303298 [virtual disks][dataStore] Hotplug/unplug disks with data files VIRT-303297 [virtual_disks][dataStore] Start guest with disks that have data files Signed-off-by: Meina Li <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Automate two cases:
VIRT-303298 [virtual disks][dataStore] Hotplug/unplug disks with data files
VIRT-303297 [virtual_disks][dataStore] Start guest with disks that have data files