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(