You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/04/15 19:58:32 UTC
[GitHub] [cloudstack] ustcweizhou opened a new pull request #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
ustcweizhou opened a new pull request #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033
## Description
<!--- Describe your changes in detail -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
To void qcow2 image corruption, we'd better suspend vm when delete a vmsnapshot, and resume it when vmsnapshot is removed.
Fixes: #3193
related to: #3194 #4029
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland merged pull request #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614291722
Packaging result: ✔centos7 ✔debian. JID-1171
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] ustcweizhou opened a new pull request #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
ustcweizhou opened a new pull request #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033
## Description
<!--- Describe your changes in detail -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
To void qcow2 image corruption, we'd better suspend vm when delete a vmsnapshot, and resume it when vmsnapshot is removed.
Fixes: #3193
related to: #3194 #4029
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614300461
Good boy., ape....
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] luhaijiao commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
luhaijiao commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614380354
Would expect to have it in 4.13 RC2 given it's potential but serious impact.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614285141
@andrijapanicsb a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614297041
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614643613
2 x LGTMs/Approvals, manual testing done fine, regression tests are fine.
Ready for merge @DaanHoogland
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614552069
Not sure ai understand your question @DaanHoogland?
In general, we should first check if VM is running, and then suspend/delete/resume.
Not sure this would work for a stopped VM? (about to test the PR now)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] rhtyd closed pull request #4033: kvm: suspend/resume
in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614281398
@weizhouapache being Wei Zhou, as usual... :) thanks!
Shall we try to squeeze this in master only (4.13 is already out for voting) - or shall we craft another 4.13 RC2 due to this? (I don't see a problem there) - this seems to be a serious enough issue/fix to warrant RC2? (and simple enough change to NOT ask for a serious full-blown retesting of everything) ?
/cc @DaanHoogland @rhtyd
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4033: kvm: suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#discussion_r409416457
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java
##########
@@ -58,6 +59,9 @@ public Answer execute(final DeleteVMSnapshotCommand cmd, final LibvirtComputingR
snapshot = dm.snapshotLookupByName(cmd.getTarget().getSnapshotName());
+ s_logger.debug("Suspending domain " + vmName);
+ dm.suspend(); // suspend the vm to avoid image corruption
Review comment:
should we do this conditionally and remember whether we did, to then only start it up again in the finally clause if it wasn't suspended in the first place?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on a change in pull request
#4033: kvm: suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on a change in pull request #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#discussion_r409500511
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java
##########
@@ -58,6 +59,9 @@ public Answer execute(final DeleteVMSnapshotCommand cmd, final LibvirtComputingR
snapshot = dm.snapshotLookupByName(cmd.getTarget().getSnapshotName());
+ s_logger.debug("Suspending domain " + vmName);
+ dm.suspend(); // suspend the vm to avoid image corruption
Review comment:
Will have to leave this for another time - otherwise, the "manually suspended" VM does indeed get's unsuspended - just managed to test it. WHY would someone in the first place even touch the VM manually, then expect ACS to handle it all?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614297507
@andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4033:
kvm: suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#discussion_r409493080
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java
##########
@@ -58,6 +59,9 @@ public Answer execute(final DeleteVMSnapshotCommand cmd, final LibvirtComputingR
snapshot = dm.snapshotLookupByName(cmd.getTarget().getSnapshotName());
+ s_logger.debug("Suspending domain " + vmName);
+ dm.suspend(); // suspend the vm to avoid image corruption
Review comment:
@weizhouapache can you wrap this in a state check?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614284778
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614433722
<b>Trillian test result (tid-1420)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 27668 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4033-t1420-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 76 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_vpc_privategw_static_routes | `Failure` | 174.10 | test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 176.12 | test_privategw_acl.py
test_04_rvpc_privategw_static_routes | `Failure` | 237.66 | test_privategw_acl.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] rhtyd commented on issue #4033: kvm: suspend/resume in
deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614603862
@andrijapanicsb @DaanHoogland this also LGTM, smaller than the other two. The code to suspend is okay, but a check may be preferable.
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614612096
Packaging result: ✔centos7 ✔debian. JID-1173
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4033: kvm:
suspend/resume in deleting vm snapshot on kvm
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4033: kvm: suspend/resume in deleting vm snapshot on kvm
URL: https://github.com/apache/cloudstack/pull/4033#issuecomment-614604117
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services