JoaoJandre commented on code in PR #13746:
URL: https://github.com/apache/cloudstack/pull/13746#discussion_r3720524696
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java:
##########
@@ -121,6 +121,49 @@ private String convertImageToQCow2(
return dstPath;
}
+ /**
+ * Writes an incremental backup: a qcow2 on secondary storage containing
only the blocks of the
+ * snapshot device that differ from the parent snapshot qcow2, with the
parent as backing file.
+ * The overlay starts out backed by the raw snapshot device itself; the
safe-mode rebase onto the
+ * parent then copies every cluster in which the two backing files differ
into the overlay. The
+ * explicit virtual size clips the DRBD metadata trailing the storage
snapshot device.
+ */
+ private String createIncrementalQCow2(
+ final String srcPath,
+ final SnapshotObjectTO dst,
+ final KVMStoragePool secondaryPool,
+ final File parentFile,
+ final long netSize,
+ int waitMilliSeconds)
+ throws LibvirtException, QemuImgException, IOException
+ {
+ final String dstDir = secondaryPool.getLocalPath() + File.separator +
dst.getPath();
+ FileUtils.forceMkdir(new File(dstDir));
+ final String dstPath = dstDir + File.separator + dst.getName();
+
+ final Script createOverlay = new Script("qemu-img",
Duration.millis(waitMilliSeconds));
Review Comment:
Please use the QemuImg class, extend it if needed.
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java:
##########
@@ -121,6 +121,49 @@ private String convertImageToQCow2(
return dstPath;
}
+ /**
+ * Writes an incremental backup: a qcow2 on secondary storage containing
only the blocks of the
+ * snapshot device that differ from the parent snapshot qcow2, with the
parent as backing file.
+ * The overlay starts out backed by the raw snapshot device itself; the
safe-mode rebase onto the
+ * parent then copies every cluster in which the two backing files differ
into the overlay. The
+ * explicit virtual size clips the DRBD metadata trailing the storage
snapshot device.
+ */
+ private String createIncrementalQCow2(
+ final String srcPath,
+ final SnapshotObjectTO dst,
+ final KVMStoragePool secondaryPool,
+ final File parentFile,
+ final long netSize,
+ int waitMilliSeconds)
+ throws LibvirtException, QemuImgException, IOException
+ {
Review Comment:
This method definition does not follow the coding conventions. (see
https://cwiki.apache.org/confluence/spaces/CLOUDSTACK/pages/29687985/Coding+conventions)
```suggestion
private String createIncrementalQCow2 (final String srcPath, final
SnapshotObjectTO dst, final KVMStoragePool secondaryPool, final File parentFile,
final long netSize, int waitMilliSeconds) throws LibvirtException,
QemuImgException, IOException {
```
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java:
##########
@@ -121,6 +121,49 @@ private String convertImageToQCow2(
return dstPath;
}
+ /**
+ * Writes an incremental backup: a qcow2 on secondary storage containing
only the blocks of the
+ * snapshot device that differ from the parent snapshot qcow2, with the
parent as backing file.
+ * The overlay starts out backed by the raw snapshot device itself; the
safe-mode rebase onto the
+ * parent then copies every cluster in which the two backing files differ
into the overlay. The
+ * explicit virtual size clips the DRBD metadata trailing the storage
snapshot device.
+ */
+ private String createIncrementalQCow2(
+ final String srcPath,
+ final SnapshotObjectTO dst,
+ final KVMStoragePool secondaryPool,
+ final File parentFile,
+ final long netSize,
+ int waitMilliSeconds)
+ throws LibvirtException, QemuImgException, IOException
+ {
+ final String dstDir = secondaryPool.getLocalPath() + File.separator +
dst.getPath();
Review Comment:
```suggestion
final String dstDir = secondaryPool.getLocalPathFor(dst.getPath());
```
##########
plugins/storage/volume/linstor/CHANGELOG.md:
##########
@@ -24,6 +24,12 @@ All notable changes to Linstor CloudStack plugin will be
documented in this file
The format is based on [Keep a
Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic
Versioning](https://semver.org/spec/v2.0.0.html).
+## [2026-07-30]
+
+### Added
+
+- Support for incremental snapshots on secondary storage backuped snapshots
Review Comment:
valid review
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java:
##########
@@ -121,6 +121,49 @@ private String convertImageToQCow2(
return dstPath;
}
+ /**
+ * Writes an incremental backup: a qcow2 on secondary storage containing
only the blocks of the
+ * snapshot device that differ from the parent snapshot qcow2, with the
parent as backing file.
+ * The overlay starts out backed by the raw snapshot device itself; the
safe-mode rebase onto the
+ * parent then copies every cluster in which the two backing files differ
into the overlay. The
+ * explicit virtual size clips the DRBD metadata trailing the storage
snapshot device.
+ */
+ private String createIncrementalQCow2(
+ final String srcPath,
+ final SnapshotObjectTO dst,
+ final KVMStoragePool secondaryPool,
+ final File parentFile,
+ final long netSize,
+ int waitMilliSeconds)
+ throws LibvirtException, QemuImgException, IOException
+ {
+ final String dstDir = secondaryPool.getLocalPath() + File.separator +
dst.getPath();
+ FileUtils.forceMkdir(new File(dstDir));
+ final String dstPath = dstDir + File.separator + dst.getName();
+
+ final Script createOverlay = new Script("qemu-img",
Duration.millis(waitMilliSeconds));
+ createOverlay.add("create", "-f", "qcow2", "-F", "raw", "-b", srcPath,
dstPath, String.valueOf(netSize));
+ final String createResult = createOverlay.execute();
+ if (createResult != null) {
+ throw new QemuImgException("Unable to create qcow2 overlay of " +
srcPath + ": " + createResult);
+ }
+
+ try {
+ final QemuImg qemu = new QemuImg(waitMilliSeconds);
+ final QemuImgFile dstFile = new QemuImgFile(dstPath,
QemuImg.PhysicalDiskFormat.QCOW2);
Review Comment:
Interesting use of the rebase. I have a few questions though:
1. If the VM is running, how can you make sure that the snapshot being
created is consistent? Since you are not quiescing the FS.
2. Considering that the VM will continue writing on disk, can you guarantee
that does the process will end? If qemu keeps copying the current data being
written by the VM, the snapshot is not guaranteed to contain only the data of
the moment the snapshot is taken, since it would contain the writes that the VM
made during the process as well.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
