Skip to content
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

Test the "--managed-save" option #6131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yalzhang
Copy link
Contributor

@yalzhang yalzhang commented Jan 15, 2025

When the VM have a managedsave file, it can not be undefined without the "--managed-save" option. This option can remove the managedsave file and then undefine the VM. (automate VIRT-297771)

@yalzhang
Copy link
Contributor Author

yalzhang commented Jan 15, 2025

test results:
(1/1) type_specific.io-github-autotest-libvirt.virsh.managedsave_undefine: PASS (8.31 s)

Copy link
Contributor

@bskjois bskjois left a comment

Choose a reason for hiding this comment

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

Except 2 typos, rest looks good to me!


def vm_undefine_check(vm_name):
"""
Check if vm can be undefined with manage-save option
Copy link
Contributor

Choose a reason for hiding this comment

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

it should be managed-save in the above line. Can be ignored as it is in commented block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! updated.

ignore_status=True)
LOG.debug("%s", ret1)
if ret1.exit_status:
test.fail("Guest can't be undefine with "
Copy link
Contributor

Choose a reason for hiding this comment

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

undefined suits better than undefine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! updated.

When the VM have a managedsave file, it can not be undefined without the
"--managed-save" option. This option can remove the managedsave file and
then undefine the VM.

Signed-off-by: Yalan Zhang <[email protected]>
@yalzhang yalzhang force-pushed the with_option_managed_save branch from 29f76f0 to 1772117 Compare January 16, 2025 01:44
test.fail("Can't find managed save image")
LOG.info("Step2: Undefine the VM without --managed-save option:")
ret = virsh.undefine(vm_name, options='--nvram', ignore_status=True)
LOG.debug("%s", ret)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using 'debug=True' in virsh.undefine() instead of logging out it by 'LOG.debug("%s", ret)'? What do you think?

Comment on lines +29 to +31
if not ret.exit_status:
test.fail("Guest shouldn't be undefined"
"while domain managed save image exists!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you like to try the result checking wrapper function - libvirt.check_exit_status?

Comment on lines +33 to +38
ret1 = virsh.undefine(vm_name, options="--managed-save --nvram",
ignore_status=True)
LOG.debug("%s", ret1)
if ret1.exit_status:
test.fail("Guest can't be undefined with "
"managed-save option!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

test.fail("Guest shouldn't be undefined"
"while domain managed save image exists!")
LOG.info("Step3: Undefine the VM with --managed-save option:")
ret1 = virsh.undefine(vm_name, options="--managed-save --nvram",
Copy link
Contributor

Choose a reason for hiding this comment

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

May I ask if '--nvram' option is required? Same question to the first virsh undefine attempt(the failed one) as well.

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.

3 participants