From 764b22b44712c99bc8093828e56887d9b14fbdd4 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Fri, 2 Oct 2026 16:01:21 +0200 Subject: [PATCH] kvm: fill the template's holes before taking its LUKS clone snapshot An encrypted root is a librbd CoW clone of a plaintext template carrying a LUKS2 header. librbd serves the ranges the clone's own objects do not hold from that parent, as plaintext, which is what makes the inherited filesystem readable. That stops the moment an object exists in the clone: one 4 KiB guest write copies up the whole 4 MiB object, and the ranges inside it that the parent never materialised are from then on read through the crypto layer - where zeros decrypted with AES-XTS are not zeros. Before taking the snapshot encrypted roots are cloned from, rewrite the template image from its own cloudstack-base-snap with qemu-img convert -S 0, so the snapshot has no holes left. The snapshot gets a new name, -luks2, so templates prepared before get a dense one too; roots already cloned from the old -luks snapshot keep it. If another host prepares the same template at the same time, its protected snapshot is used instead of failing the deploy. QemuImg grows setWriteZeroRanges() for the -S 0 this needs, since qemu-img skips the source's zero ranges by default. --- .../kvm/storage/LibvirtStorageAdaptor.java | 19 ++++++++-- .../apache/cloudstack/utils/qemu/QemuImg.java | 14 ++++++++ .../cloudstack/utils/rbd/RbdEncryption.java | 36 +++++++++++++++++++ .../cloudstack/utils/qemu/QemuImgTest.java | 35 ++++++++++++++++++ .../utils/rbd/RbdEncryptionTest.java | 30 ++++++++++++++++ 5 files changed, 131 insertions(+), 3 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 4517ddc3c205..5f5cfdfe7f70 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1535,7 +1535,8 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, */ private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KVMStoragePool destPool, String newUuid, KVMPhysicalDisk disk, byte[] passphrase) { - String luksReservedSnapshotName = rbdTemplateSnapName + "-luks"; + // not "-luks": templates prepared before carry a snapshot of that name that still has holes + String luksReservedSnapshotName = rbdTemplateSnapName + "-luks2"; Rados radosConnection = null; IoCTX ioContext = null; Rbd rbdClient = null; @@ -1558,8 +1559,20 @@ private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KV } if (!luksSnapshotExists) { templateImage.resize(template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES); - templateImage.snapCreate(luksReservedSnapshotName); - templateImage.snapProtect(luksReservedSnapshotName); + // fill the template's holes first: an encrypted clone reads a hole in its parent as + // garbage once a guest write copies up the object around it + new RbdEncryption().copyTemplateDense(destPool.getSourceHost(), destPool.getSourcePort(), + destPool.getAuthUserName(), destPool.getAuthSecret(), destPool.getSourceDir(), + template.getName(), rbdTemplateSnapName); + try { + templateImage.snapCreate(luksReservedSnapshotName); + templateImage.snapProtect(luksReservedSnapshotName); + } catch (RbdException e) { + // fine if another host prepared the same template at the same time + if (!templateImage.snapIsProtected(luksReservedSnapshotName)) { + throw e; + } + } logger.debug("Prepared LUKS-reserved template snapshot {}@{}", template.getName(), luksReservedSnapshotName); } rbdClient.clone(template.getName(), luksReservedSnapshotName, ioContext, newUuid, RBD_FEATURES, rbdOrder); diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java index 49f531ed7c11..0b5010adf365 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java @@ -66,6 +66,7 @@ public class QemuImg { private String cloudQemuImgPath = "cloud-qemu-img"; private long timeout; private boolean skipZero = false; + private boolean writeZeroRanges = false; private boolean skipTargetVolumeCreation = false; private boolean noCache = false; private long version; @@ -508,6 +509,11 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuI script.add("-n"); } + if (writeZeroRanges) { + script.add("-S"); + script.add("0"); + } + if (destImageOpts == null) { script.add("-O"); script.add(destFile.getFormat().toString()); @@ -1011,6 +1017,14 @@ public void setSkipZero(boolean skipZero) { this.skipZero = skipZero; } + /** + * Make convert write the source's zero ranges to the destination ({@code -S 0}) instead of + * leaving them unallocated. + */ + public void setWriteZeroRanges(boolean writeZeroRanges) { + this.writeZeroRanges = writeZeroRanges; + } + public void setSkipTargetVolumeCreation(boolean skipTargetVolumeCreation) { this.skipTargetVolumeCreation = skipTargetVolumeCreation; } diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java index a23ce93d2b82..833fb82807be 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java @@ -253,6 +253,42 @@ public void importTemplate(String srcRbdPool, String srcRbdImage, /** * Seam for unit tests; {@link QemuImg} probes the qemu version through libvirt on construction. */ + /** + * Rewrite an RBD image from one of its snapshots, writing out the zero ranges too ({@code -S 0}), + * so the image has no holes. Used on a template before taking the snapshot encrypted roots are + * cloned from: a hole in that parent reads as garbage through the clone's encryption once a + * guest write copies up the object around it. + */ + public void copyTemplateDense(String monHost, int monPort, String authUser, String authSecret, + String cephPool, String image, String srcSnapshot) { + Path conf = null; + Path keyring = null; + try { + final FileAttribute ownerOnly = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")); + keyring = Files.createTempFile("cs-ceph-", ".keyring", ownerOnly); + Files.writeString(keyring, "[client." + authUser + "]\n\tkey = " + authSecret + "\n"); + conf = Files.createTempFile("cs-ceph-", ".conf", ownerOnly); + Files.writeString(conf, "[global]\nmon_host = " + monSpec(monHost, monPort) + "\nkeyring = " + keyring + "\n"); + + QemuImgFile srcQemuFile = new QemuImgFile(cephPool + "/" + image + "@" + srcSnapshot, QemuImg.PhysicalDiskFormat.RAW); + Map srcParams = rbdImageOptions(cephPool, image, conf.toString(), authUser); + srcParams.put("snapshot", srcSnapshot); + QemuImageOptions srcImageOpts = new QemuImageOptions(srcParams); + srcImageOpts.setImageOptsFlag(true); + QemuImageOptions destImageOpts = new QemuImageOptions(rbdImageOptions(cephPool, image, conf.toString(), authUser)); + + QemuImg qemu = createQemuImg(); + qemu.setWriteZeroRanges(true); + qemu.convertIntoExistingTarget(srcQemuFile, null, null, srcImageOpts, destImageOpts, false); + logger.debug("Rewrote RBD image {}/{} from @{} without holes", cephPool, image, srcSnapshot); + } catch (IOException | QemuImgException | LibvirtException ex) { + throw new CloudRuntimeException(String.format("Failed to rewrite RBD image %s/%s from @%s", cephPool, image, srcSnapshot), ex); + } finally { + deleteQuietly(conf); + deleteQuietly(keyring); + } + } + protected QemuImg createQemuImg() throws QemuImgException, LibvirtException { return new QemuImg(0); } diff --git a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java index 5b089ad37031..22c987e9e5bf 100644 --- a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java +++ b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java @@ -323,6 +323,41 @@ public void testCreateWithBackingFile() throws QemuImgException, LibvirtExceptio } } + @Test + public void testConvertWriteZeroRangesAllocatesTheWholeDestination() throws QemuImgException, LibvirtException { + // qemu-img skips the source's zero ranges by default, so the destination keeps its holes. + // That is wrong for an image whose unallocated ranges are not read as zeros - an RBD image + // with a librbd LUKS header, cloned, is the case setWriteZeroRanges exists for. + String srcPath = "/tmp/" + UUID.randomUUID() + ".raw"; + String sparsePath = "/tmp/" + UUID.randomUUID() + ".raw"; + String densePath = "/tmp/" + UUID.randomUUID() + ".raw"; + long size = 16L * 1024 * 1024; + + QemuImgFile srcFile = new QemuImgFile(srcPath, size, PhysicalDiskFormat.RAW); + QemuImg qemu = new QemuImg(0); + qemu.create(srcFile); + // data in the first MiB only: the remaining 15 MiB of the source are zeros + Script.runSimpleBashScript(String.format("dd if=/dev/urandom of=%s bs=1M count=1 conv=notrunc 2>/dev/null", srcPath)); + + qemu.convert(srcFile, new QemuImgFile(sparsePath, PhysicalDiskFormat.RAW)); + qemu.setWriteZeroRanges(true); + qemu.convert(srcFile, new QemuImgFile(densePath, PhysicalDiskFormat.RAW)); + + long apparent = Long.parseLong(Script.runSimpleBashScript(String.format("stat -c %%s %s", densePath))); + long sparseAllocated = Long.parseLong(Script.runSimpleBashScript(String.format("du --block-size=1 %s | cut -f1", sparsePath))); + long denseAllocated = Long.parseLong(Script.runSimpleBashScript(String.format("du --block-size=1 %s | cut -f1", densePath))); + + assertEquals(size, apparent); + assertTrue("the default convert should have left the zero ranges unallocated, allocated " + sparseAllocated, + sparseAllocated < size); + assertTrue("-S 0 should have written every zero range, allocated " + denseAllocated, + denseAllocated >= size); + + assertTrue(new File(srcPath).delete()); + assertTrue(new File(sparsePath).delete()); + assertTrue(new File(densePath).delete()); + } + @Test public void testConvertBasic() throws QemuImgException, LibvirtException { long srcSize = 20480; diff --git a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/rbd/RbdEncryptionTest.java b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/rbd/RbdEncryptionTest.java index e4c977c676f1..9839188618df 100644 --- a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/rbd/RbdEncryptionTest.java +++ b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/rbd/RbdEncryptionTest.java @@ -143,6 +143,36 @@ public void importTemplateFromFileSourceForcesSourceFormat() throws Exception { Assert.assertTrue(src, src.contains("file.filename=/tmp/tmpl.qcow2")); } + @Test + public void copyTemplateDenseWritesZeroRangesAndLeavesTheCopyPlaintext() throws Exception { + RbdEncryption spy = Mockito.spy(new RbdEncryption()); + QemuImg qemuImg = Mockito.mock(QemuImg.class); + Mockito.doReturn(qemuImg).when(spy).createQemuImg(); + + spy.copyTemplateDense("1.2.3.4", 6789, "cloudstack", "secret", "cloudstack", "tmpl", "cloudstack-base-snap"); + + // without -S 0 the template keeps its holes + Mockito.verify(qemuImg).setWriteZeroRanges(true); + + ArgumentCaptor srcOpts = ArgumentCaptor.forClass(QemuImageOptions.class); + ArgumentCaptor destOpts = ArgumentCaptor.forClass(QemuImageOptions.class); + Mockito.verify(qemuImg).convertIntoExistingTarget(Mockito.any(QemuImgFile.class), Mockito.isNull(), + Mockito.isNull(), srcOpts.capture(), destOpts.capture(), Mockito.eq(false)); + + String src = String.join(" ", srcOpts.getValue().toCommandFlag()); + Assert.assertTrue(src, src.startsWith("--image-opts ")); + Assert.assertTrue(src, src.contains("pool=cloudstack")); + Assert.assertTrue(src, src.contains("image=tmpl")); + Assert.assertTrue(src, src.contains("snapshot=cloudstack-base-snap")); + + String dest = String.join(" ", destOpts.getValue().toCommandFlag(QemuImg.TARGET_IMAGE_OPTS_FLAG)); + Assert.assertTrue(dest, dest.startsWith(QemuImg.TARGET_IMAGE_OPTS_FLAG + " ")); + Assert.assertTrue(dest, dest.contains("image=tmpl")); + // the template is written to itself, not to a snapshot, and stays plaintext + Assert.assertFalse(dest, dest.contains("snapshot=")); + Assert.assertFalse(dest, dest.contains("encrypt.")); + } + @Test public void formatRejectsEmptyPassphrase() { Assert.assertThrows(CloudRuntimeException.class, () -> rbdEncryption.format(