CLOUDSTACK-5863: revert volume snapshot for KVM/QCOW2 #732
Conversation
|
cloudstack-pull-rats #376 SUCCESS |
|
cloudstack-pull-analysis #309 SUCCESS |
|
LGTM to me. We should however stay as far away as possible from invoking all kinds of scripts. Implementing this for RBD is also a lot easier since you laid some groundwork. The java RBD bindings should be able to do this. |
|
LGTM to me too. Tested on a master test deployment (Ubuntu 14.04 / KVM / NFS) |
| @@ -91,7 +102,6 @@ public void execute() { | |||
| boolean result = _snapshotService.revertSnapshot(getId()); | |||
| if (result) { | |||
| SuccessResponse response = new SuccessResponse(getCommandName()); | |||
| response.setResponseName(getCommandName()); | |||
There was a problem hiding this comment.
this might cause API response issues.
|
In general LGTM, please see if you could fix the response class and object of the API (pl see the comments). |
|
Can you add some tests please? |
|
cloudstack-pull-rats #432 SUCCESS |
|
LGTM |
|
cloudstack-pull-analysis #365 FAILURE |
|
any other comment about this PR ? |
|
No, seems good. We can merge this. |
…-master Guys, can you review it? things need to be discussed: (1) this supports KVM/QCOW2 only. Anyone want to implement for other Hypervisor/format ? (2) The original data volume (on primary storage) will be removed. (3) The script uses the default timeout in libvirtComputingResource. Do we need to add one in global configuration (like copy.volume.wait or backup.snapshot.wait, create.volume.from.snapshot.wait) (4) In scripts/storage/qcow2/managesnapshot.sh, I use "qemu-img convert -f qcow2 -O qcow2" to copy the snapshot from secondary to primary (hence there is no base image file), instead of "cp -f", this is because convert is faster than cp in my testing. * pr/732: CLOUDSTACK-5863: revert volume snapshot for KVM/QCOW2 Signed-off-by: Wei Zhou <w.zhou@tech.leaseweb.com>
|
@ustcweizhou this caused a new coverity issue. Can you check? |
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Guys, can you review it? things need to be discussed:
(1) this supports KVM/QCOW2 only. Anyone want to implement for other Hypervisor/format ?
(2) The original data volume (on primary storage) will be removed.
(3) The script uses the default timeout in libvirtComputingResource. Do we need to add one in global configuration (like copy.volume.wait or backup.snapshot.wait, create.volume.from.snapshot.wait)
(4) In scripts/storage/qcow2/managesnapshot.sh, I use "qemu-img convert -f qcow2 -O qcow2" to copy the snapshot from secondary to primary (hence there is no base image file), instead of "cp -f", this is because convert is faster than cp in my testing.