-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Storage-based Snapshots for KVM VMs #3724
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
Conversation
|
@blueorangutan package |
|
@blueorangutan package |
|
@andrijapanicsb a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-533 |
|
@blueorangutan test |
|
@andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-694)
|
|
@slavkap @andrijapanicsb |
|
Thank you @weizhouapache that you will spend time on this! I am available if you have any comments or questions. |
|
@blueorangutan test |
|
@andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@slavkap - nice stuff as already commented!
thx |
wido
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments about executing commands from inside Java
...c/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
Outdated
Show resolved
Hide resolved
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
Outdated
Show resolved
Hide resolved
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
Outdated
Show resolved
Hide resolved
|
moving to 4.15, I'm afraid we won't be able to test in time. |
@andrijapanicsb as far as I remember, vm is paused not only when we create snapshot but also delete a snapshot. I think corrupted volume issue should be fixed. |
|
Hello @andrijapanicsb, sorry for the late response! Bellow you can find answers of your questions
We'll document the functionality, but could you please tell me what should be included? Also I will update the global setting with:
Ceph is marked in DB as RAW format, but this doesn't affect snapshot and revert of Ceph's volumes, because we are using its implementation for this. For each primary storage we are using appropriate plugin with take/revert snapshot implementation. We've completed our tests including on busy VMs with different primary storages. They've been successfully reverted and it works as expected.
Unfortunately we'll need time to implement qemu-agent and qemu-monitor commands functionality in libvirt java api, and don't know when it will be accepted. |
|
@blueorangutan package |
|
@nvazquez a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2921 |
|
@blueorangutan test |
|
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-3656)
|
|
I tested this PR today with the Linstor plugin and creating/reverting and deleting works with Linstor. |
|
Thanks @rp- can you please approve on the Files changed tab -> Review changes -> Submit review? |
rp-
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with Linstor primary storage:
create/revert and delete worked.
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
Outdated
Show resolved
Hide resolved
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
Outdated
Show resolved
Hide resolved
| public static final ConfigKey<Boolean> BackupSnapshotAfterTakingSnapshot = new ConfigKey<Boolean>(Boolean.class, "snapshot.backup.to.secondary", "Snapshots", "true", | ||
| "Indicates whether to always backup primary storage snapshot to secondary storage. Keeping snapshots only on Primary storage is applicable for KVM + Ceph only.", false, ConfigKey.Scope.Global, null); | ||
|
|
||
| public static final ConfigKey<Boolean> VMsnapshotKVM = new ConfigKey<>(Boolean.class, "kvm.vmstoragesnapshot.enabled", "Snapshots", "false", "For live snapshot of virtual machine instance on KVM hypervisor without memory. Requieres qemu version 1.6+ (on NFS or Local file system) and qemu-guest-agent installed on guest VM", true, ConfigKey.Scope.Global, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it be renamed to something like 'VMStorageSnapshotKVM'? Could the scope be reduced to zone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nvazquez, I will rename it, but why limit this to a Zone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slavkap I was thinking it would make sense for admins to enable/disable the feature for a zone and not for all of them at the same time - just a suggestion
nvazquez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @slavkap - code LGTM
|
Hi @wido @GabrielBrascher @svenvogel would it be possible for you to test this PR on Ceph or Solidfire? |
|
@blueorangutan package |
|
@nvazquez a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3027 |
|
@blueorangutan test |
|
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-3805)
|
|
Merging based on approvals and tests results |
|
Fantastic, this is finally merged! Thank for your work and patience @slavkap - do raise a documentation PR to as required - https://github.com/apache/cloudstack-documentation |
|
yes, @rohityadavcloud 🎉 ❤️ |
Documentation fo pull request - apache/cloudstack#3724
* [WIP] Storage-based VM snapshots on KVM Documentation fo pull request - apache/cloudstack#3724 * Removed information for QCOW2 support * Fix reference * Fix section
* [WIP] Storage-based VM snapshots on KVM Documentation fo pull request - apache/cloudstack#3724 * Removed information for QCOW2 support * Fix reference * Fix section
Description
Cloudstack, with KVM as a hypervisor, provides live VM snapshots only with volumes whose image format is QCOW.
This is a limitation for storage providers with disks in RAW format.
link to ML: http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201911.mbox/%3cCAA6FghF7eaY-A3XGN5zSwKVvp7zrcuNzv5nFAQKaF+et3zH-ag@mail.gmail.com%3e#archives
With this feature
Types of changes
Documentation
apache/cloudstack-documentation#153
Prerequisites
guest agent enabled - used to freeze/thaw the VM
How Has This Been Tested?
Environment:
Enable the global configuration setting -
kvm.vmstoragesnapshot.enabledTake snapshot:
Revert snapshot