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

Qemu: snapshot: Don't forbid snapshot if autodestroy is registered! #6114

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

bskjois
Copy link
Contributor

@bskjois bskjois commented Jan 6, 2025

Signed-off-by: Kowshik Jois B S kowsjois@linux.ibm.com

Description: On v5.8.0 and later versions of libvirt, it is allowed to take the snapshot of guests created with 'autodestroy' option since semantically VIR_DOMAIN_START_AUTODESTROY doesn't really clash with snapshot operations as the VM stays on the same host and thus bound to the same connection.

Referrece commit: https://gitlab.com/libvirt/libvirt/-/commit/045a8e197c1e14faa1c32bd637033838269094a2

Signed-off-by: Kowshik Jois B S <kowsjois@linux.ibm.com>

Description: On v5.8.0 and later versions of lobvirt, it is allowed to take the snapshot of guests created with 'autodestroy' since semantically VIR_DOMAIN_START_AUTODESTROY doesn't really clash with snapshot operations as the VM stays on the same host and thus bound to the same connection.

Referrece commit: https://gitlab.com/libvirt/libvirt/-/commit/045a8e197c1e14faa1c32bd637033838269094a2
@bskjois
Copy link
Contributor Author

bskjois commented Jan 7, 2025

Logs before applying the patch:

(2/3) type_specific.io-github-autotest-libvirt.virsh.snapshot_create_as.negative_tests.autodestroy_domain: STARTED
(2/3) type_specific.io-github-autotest-libvirt.virsh.snapshot_create_as.negative_tests.autodestroy_domain: FAIL: Run successfully with wrong command! (9.44 s)

Logs after adding the proposed changes:

(2/3) type_specific.io-github-autotest-libvirt.virsh.snapshot_create_as.negative_tests.autodestroy_domain: STARTED
(2/3) type_specific.io-github-autotest-libvirt.virsh.snapshot_create_as.negative_tests.autodestroy_domain: PASS (9.77 s)

@bskjois
Copy link
Contributor Author

bskjois commented Jan 9, 2025

@chloerh @chunfuwen @cliping @dzhengfy @pipo @xiaodwan @yalzhang @Yingshun
May I request one of you to please take a look at this PR and give your valuable review comments. Thanks in Advance!

Copy link
Contributor

@yalzhang yalzhang left a comment

Choose a reason for hiding this comment

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

Fair enough. Run offline with the patch pass. One small concern: the case name includes 'negative_tests,' but it's now a positive test. However, this isn't an major issue.

@Yingshun Yingshun merged commit 2d34785 into autotest:master Jan 14, 2025
5 checks passed
@bskjois
Copy link
Contributor Author

bskjois commented Jan 15, 2025

@yalzhang @Yingshun Thank you so much for quick review and merger of this PR.
As per the comments received, I will add the code to change this test case to a positive scenario soon.

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