KVM disk-only based snapshot of volumes instead of taking VM's full snapshot and extracting disks #5297

GutoVeronezi · 2021-08-09T16:27:57Z

Description

This PR solves issue #5124.

The main focus of it is to change how volume snapshots are taken in KVM. However, this proposal of changing take snapshot workflow in KVM impacts others snapshot workflows as well. The impacted workflows we mapped are:

Take volume snapshot with running vm
Take volume snapshot with stopped vm
Recurring volume snapshot
Create template from snapshot
Create volume from snapshot
Migrate virtual machine with volumes
Migrate volumes
Revert volume snapshot
Delete volume snapshot

Issue #5124 has more details and flowcharts of all the mapped workflows.

The focus of this PR is to refactor KVM snapshot, others hypervisors workflows should not be affected.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

How Has This Been Tested?

I added unit tests to several methods and tested the workflows:

Take volume snapshot with running vm
Take volume snapshot with stopped vm
Recurring volume snapshot
Create template from snapshot
Create volume from snapshot
Migrate virtual machine with volumes
Migrate volumes
Revert volume snapshot
Delete volume snapshot

These workflows were tested twice, once with the global setting snapshot.backup.to.secondary as true and once as false.

To test workflows 1, 2, and 3, I created a VM called Test1, then I opened its console to create a file, which I called testfile. At first, I wrote gold in this file and took a snapshot that I called gold (this tests workflow 1). Then I wrote silver to the file and configured a recurring snapshot. I waited for the recurring snapshot and, when it was took, I changed the name in database to silver, to facilitate the steps (this tests workflow 2). Finally, I wrote copper to the file and stopped the VM, then I took a snapshot (this tests workflow 3). After that, I destroyed VM Test1.

To test workflow 4, I created a template from snapshot silver and called it template silver. From it, I created a VM called Test2, then I opened its console and retrieved the content of the testfile with the command cat. It returned silver, which is the same value of the file when I took the snapshot on workflow 2. After that, I stopped VM Test2.

To test workflow 5, I created a volume from snapshot gold and called it volume gold, then I changed the root volume of the VM Test2 to the volume gold. I started the VM Test2 and retrieved the content of the testfile with the command cat. It returned gold, which is the same value of the file when I took the snapshot on workflow 1.

To test workflows 6 and 7, I wrote platinum to the file testfile in VM Test2, then I took a snapshot and called it platinum. I migrated the VM and the volume to another host and primary storage with the API migrateVirtualMachineWithVolume. When the global setting snapshot.backup.to.secondary was true, it migrated successfully. When it was false, an exception was threw, warning user about snapshots on primary storage (this tests workflow 6). I stopped the VM and migrated only the volume to another primary storage. When the global setting snapshot.backup.to.secondary was true, it migrated successfully. When it was false, an exception was threw, warning users about snapshots on primary storagee (this tests workflow 7).

To test workflow 8, I started VM Test2 and wrote diamond to the file testfile. Then I stopped the VM Test2 and reverted the volume to the snapshot platinum. Then I opened its console and retrieved the content of the testfile with the command cat. It returned platinum, which is the same value of the file when I took the snapshot before the workflow 6.

To test workflow 9, I deleted all the snapshots.

…lume)

RodrigoDLopez

Nice enhancement @GutoVeronezi
I would like it if you could explain your tests more clearly. I think just organize your text will help a lot.
The code LGTM.

nvazquez · 2021-08-11T03:20:12Z

Thanks @GutoVeronezi I'll post my review this week

slavkap · 2021-08-11T07:46:14Z

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java

+     * 2nd parameter: disk's label (target.dev tag from VM's XML);<br>
+     * 3rd parameter: the absolute path of the base file;
+     */
+    private static final String COMMAND_MERGE_SNAPSHOT = "virsh blockcommit %s %s --base %s --active --wait --delete --pivot";


@GutoVeronezi, one suggestion - the latest Libvirt's Java API bindings support qemu-monitor-command and maybe the command block-commit could replace this.

I spoke offline with @GutoVeronezi, and the flag --delete does not work with the version of libvirt that I have

Using library: libvirt 5.0.0 Using API: QEMU 5.0.0 Running hypervisor: QEMU 2.12.0

Thanks @slavkap, I will make some changes soon.

GutoVeronezi · 2021-08-13T20:36:47Z

@RodrigoDLopez thanks for the review. I just updated the tests description. If there is anything to improve, let me know.

… only if libvirt version is equal or higher 6.0.0

GutoVeronezi · 2021-08-16T14:03:48Z

Regarding to @slavkap #5297 (comment), the feature blockcommit was introduced on Libvirt's version 1.2.9, according to its docs. However some tags were introduced as a future work mark. The tag delete was really implemented only in version 6.0.0, through the commit qemu: block: enable the snapshot image deletion feature.
Therefore, for cases which operators cannot upgrade Libvirt (like in CentOS 7) to a version that supports this flag, we created a workaround, manually deleting the unused snapshot file. As soon as we have everybody using a version equal or higher than 6.0.0, we can remove this workaround.

...chestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

...datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java

...e/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java

nvazquez · 2021-08-23T18:03:46Z

...main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java

+                    storagePoolMgr.getStoragePoolByURI(snapshotImageStore.getUrl());
+                }
+
+                if (primaryPool.getType() == StoragePoolType.CLVM) {


Does it need to be part of storagePoolTypesThatSupportRevertSnapshot?

@nvazquez reverting snapshots that were only in primary storage was not working correctly for KVM. The RBD validation was added in PR #3502 to allow Ceph to revert snapshots that were only in primary storage, however the rest kept not working. This PR fixed this error to some others storage pool types (which were added to the validation), however CLVM was not our focus, therefore we tried to not touch in it.

rohityadavcloud · 2021-08-24T07:16:07Z

@blueorangutan package

nvazquez

Code LGTM - have not tested it

nvazquez · 2022-04-08T17:36:03Z

@blueorangutan package

blueorangutan · 2022-04-08T17:37:03Z

@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.

DaanHoogland · 2022-04-11T06:39:36Z

@blueorangutan package

blueorangutan · 2022-04-11T06:40:03Z

@DaanHoogland 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.

blueorangutan · 2022-04-11T07:06:09Z

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✔️ suse15. SL-JID 3112

GutoVeronezi · 2022-04-11T12:38:30Z

@blueorangutan package

blueorangutan · 2022-04-11T12:39:03Z

@GutoVeronezi 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.

blueorangutan · 2022-04-11T13:15:24Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3118

nvazquez · 2022-04-11T13:23:48Z

@blueorangutan test

blueorangutan · 2022-04-11T13:24:03Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2022-04-11T22:42:40Z

Trillian test result (tid-3853)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32200 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5297-t3853-kvm-centos7.zip
Smoke tests completed. 93 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

nvazquez · 2022-04-12T11:14:19Z

Merging based on approvals and test results

ACS + Xenserver works with differential snapshots. ACS takes a volume full snapshot and the next ones are referenced as a child of the previous snapshot until the chain reaches the limit defined in the global setting snapshot.delta.max; then, a new full snapshot is taken. PR #5297 introduced disk-only snapshots for KVM volumes. Among the changes, the delete process was also refactored. Before the changes, when one was removing a snapshot with children, ACS was marking it as Destroyed and it was keeping the Image entry on the table cloud.snapshot_store_ref as Ready. When ACS was rotating the snapshots (the max delta was reached) and all the children were already marked as removed; then, ACS would start removing the whole hierarchy, completing the differential snapshot cycle. After the changes, the snapshots with children stopped being marked as removed and the differential snapshot cycle was not being completed. This PR intends to honor again the differential snapshot cycle for XenServer, making the snapshots to be marked as removed when deleted while having children and following the differential snapshot cycle. Also, when one takes a volume snapshot and ACS backs it up to the secondary storage, ACS inserts 2 entries on table cloud.snapshot_store_ref (Primary and Image). When one deletes a volume snapshot, ACS first tries to remove the snapshot from the secondary storage and mark the entry Image as removed; then, it tries to remove the snapshot from the primary storage and mark the entry Primary as removed. If ACS cannot remove the snapshot from the primary storage, it will keep the snapshot as BackedUp; however, If it does not exist in the secondary storage and without the entry SNAPSHOT.DELETE on cloud.usage_event. In the end, after the garbage collector flow, the snapshot will be marked as BackedUp, with a value in the field removed and still being rated. This PR also addresses the correction for this situation. Co-authored-by: GutoVeronezi <[email protected]>

…allows This removes the conditional logic where comment notest to remove it after PR apache#5297 is merged that is applicable for ACS 4.18+. Only when the global setting is enabled and memory isn't selected, VM snapshot could be allowed for VMs on KVM that have qemu-guest-agent running. Signed-off-by: Rohit Yadav <[email protected]>

…g allows (#8062) This removes the conditional logic where comment notest to remove it after PR #5297 is merged that is applicable for ACS 4.18+. Only when the global setting is enabled and memory isn't selected, VM snapshot could be allowed for VMs on KVM that have qemu-guest-agent running. Signed-off-by: Rohit Yadav <[email protected]>

…g allows (apache#8062) This removes the conditional logic where comment notest to remove it after PR apache#5297 is merged that is applicable for ACS 4.18+. Only when the global setting is enabled and memory isn't selected, VM snapshot could be allowed for VMs on KVM that have qemu-guest-agent running. Signed-off-by: Rohit Yadav <[email protected]>

…g allows (apache#8062) This removes the conditional logic where comment notest to remove it after PR apache#5297 is merged that is applicable for ACS 4.18+. Only when the global setting is enabled and memory isn't selected, VM snapshot could be allowed for VMs on KVM that have qemu-guest-agent running. Signed-off-by: Rohit Yadav <[email protected]> (cherry picked from commit 8350ce5) Signed-off-by: Rohit Yadav <[email protected]>

GutoVeronezi added 8 commits August 9, 2021 10:10

Refactor create volume snapshot with running VM

f5180dd

Refactor create volume snapshot with stopped VM

0b1ec63

Refactor create volume from snapshot

9653156

Refactor create template from snapshot

2221efa

Refactor volume migration (migrateVolume/ migrateVirtualMachineWithVo…

f41c2dd

…lume)

Refactor snapshot deletion

f68cb2a

Refactor snapshot revertion

2f439d5

Adjusts and fix cherry-pick conflicts

8bc8d54

GutoVeronezi requested review from GabrielBrascher, nvazquez and weizhouapache August 9, 2021 16:27

RodrigoDLopez approved these changes Aug 9, 2021

View reviewed changes

GutoVeronezi added component:kvm type:enhancement labels Aug 9, 2021

GutoVeronezi self-assigned this Aug 9, 2021

slavkap reviewed Aug 11, 2021

View reviewed changes

GutoVeronezi added 2 commits August 16, 2021 10:18

Remove diffuse tests

fbc7efb

Add validation to add flag '--delete' on command 'virsh blockcommand'…

15a790f

… only if libvirt version is equal or higher 6.0.0

Expunge temporary snapshot only if template creation is from snapshot

71bb4ad

nvazquez reviewed Aug 23, 2021

View reviewed changes

Extract strings to constant

5794693

nvazquez approved these changes Aug 25, 2021

View reviewed changes

nvazquez added the status:needs-testing label Aug 25, 2021

nvazquez added this to the 4.16.0.0 milestone Aug 25, 2021

nvazquez linked an issue Aug 25, 2021 that may be closed by this pull request

KVM disk-only based snapshot of volumes instead of taking VM's full snapshot and extracting disks #5124

Closed

slavkap mentioned this pull request Aug 26, 2021

Storage-based Snapshots for KVM VMs #3724

Merged

5 tasks

nvazquez merged commit 39fad2d into apache:main Apr 12, 2022

GutoVeronezi mentioned this pull request May 30, 2022

Fix extract snapshot from vm snapshot on kvm #6422

Merged

12 tasks

harikrishna-patnala mentioned this pull request Jul 19, 2022

Fix error message for newer Libvirt/Qemu with UEFI based VMs cannot do VM snapshot #4184

Closed

GutoVeronezi mentioned this pull request Aug 10, 2022

Fix delete parent snapshot #6630

Merged

10 tasks

weizhouapache mentioned this pull request Oct 19, 2022

Volume Snapshots Failing After 3600 Seconds #6836

Closed

rohityadavcloud mentioned this pull request Oct 4, 2023

KVM volume snapshots are not full snapshots anymore #8034

Closed

harikrishna-patnala mentioned this pull request Oct 5, 2023

Fix: Convert volume to another directory instead of copying it while taking volume snapshots on KVM #8041

Merged

11 tasks

rohityadavcloud mentioned this pull request Oct 10, 2023

storage: allow VM snapshots without memory for KVM when global setting allows #8062

Merged

6 tasks

JoaoJandre mentioned this pull request Apr 12, 2024

KVM Incremental Snapshots/Backups #8907

Closed

This was referenced Jun 13, 2024

Fix snapshot deletion on template creation failure #9239

Merged

Failed Volume Snapshot shows State as BackedUp #8946

Closed

sureshanaparti mentioned this pull request Mar 26, 2025

KVM incremental snapshot feature #9270

Merged

13 tasks

KVM disk-only based snapshot of volumes instead of taking VM's full snapshot and extracting disks #5297

KVM disk-only based snapshot of volumes instead of taking VM's full snapshot and extracting disks #5297

Uh oh!

Conversation

GutoVeronezi commented Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

Uh oh!

RodrigoDLopez left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Aug 11, 2021

Uh oh!

slavkap Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

slavkap Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

GutoVeronezi Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

GutoVeronezi commented Aug 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GutoVeronezi commented Aug 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nvazquez Aug 23, 2021

Choose a reason for hiding this comment

Uh oh!

GutoVeronezi Aug 23, 2021

Choose a reason for hiding this comment

Uh oh!

rohityadavcloud commented Aug 24, 2021

Uh oh!

nvazquez left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Apr 8, 2022

Uh oh!

blueorangutan commented Apr 8, 2022

Uh oh!

DaanHoogland commented Apr 11, 2022

Uh oh!

blueorangutan commented Apr 11, 2022

Uh oh!

blueorangutan commented Apr 11, 2022

Uh oh!

GutoVeronezi commented Apr 11, 2022

Uh oh!

blueorangutan commented Apr 11, 2022

Uh oh!

blueorangutan commented Apr 11, 2022

Uh oh!

nvazquez commented Apr 11, 2022

Uh oh!

blueorangutan commented Apr 11, 2022

Uh oh!

blueorangutan commented Apr 11, 2022

Uh oh!

nvazquez commented Apr 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

GutoVeronezi commented Aug 9, 2021 •

edited

Loading

GutoVeronezi commented Aug 13, 2021 •

edited

Loading

GutoVeronezi commented Aug 16, 2021 •

edited

Loading