From 43a36f50c97ec32414d82c7c62a162f5b3b1e841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Thu, 25 Jun 2026 11:18:00 +0200 Subject: [PATCH 01/41] storage: enable RBD/Ceph volume encryption support (shared base) Flip StoragePoolType.RBD from EncryptionSupport.Unsupported to Hypervisor so the existing encryption control plane (allocator, endpoint selector, offerings) treats RBD pools as encryption-capable. The agent-side encrypted RBD create path is not implemented yet; it is delivered by two follow-up tracks (qemu-native engine='qemu' and ceph-native engine='librbd'). Until then, fail closed at the two RBD create chokepoints in LibvirtStorageAdaptor (createPhysicalDisk and createDiskFromTemplate) when a passphrase is present, so we never silently produce a plaintext volume that the control plane believes is encrypted. No change for existing unencrypted RBD volumes (guards only fire when a passphrase is set; supportsEncryption() only affects volumes that require encryption). --- api/src/main/java/com/cloud/storage/Storage.java | 2 +- .../kvm/storage/LibvirtStorageAdaptor.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/com/cloud/storage/Storage.java b/api/src/main/java/com/cloud/storage/Storage.java index 3511b4e88cb9..275f6d25268d 100644 --- a/api/src/main/java/com/cloud/storage/Storage.java +++ b/api/src/main/java/com/cloud/storage/Storage.java @@ -172,7 +172,7 @@ public static enum StoragePoolType { LVM(false, false, EncryptionSupport.Unsupported), // XenServer local LVM SR CLVM(true, false, EncryptionSupport.Unsupported), CLVM_NG(true, false, EncryptionSupport.Hypervisor), - RBD(true, true, EncryptionSupport.Unsupported), // http://libvirt.org/storage.html#StorageBackendRBD + RBD(true, true, EncryptionSupport.Hypervisor), // http://libvirt.org/storage.html#StorageBackendRBD ; encrypted natively by librbd (LUKS2, engine='librbd') SharedMountPoint(true, true, EncryptionSupport.Hypervisor), VMFS(true, true, EncryptionSupport.Unsupported), // VMware VMFS storage PreSetup(true, true, EncryptionSupport.Unsupported), // for XenServer, Storage Pool is set up by customers. 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 4bfac31b68f9..e74ca6aeaba3 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 @@ -985,6 +985,13 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, StoragePoolType poolType = pool.getType(); if (StoragePoolType.RBD.equals(poolType)) { + // RBD is advertised as encryption-capable (StoragePoolType.RBD -> EncryptionSupport.Hypervisor), + // but the encrypted RBD create path is not implemented on this branch yet. Fail closed so we never + // silently create a plaintext volume that the control plane believes is encrypted. + // The qemu-native (engine='qemu') and ceph-native (engine='librbd') tracks each replace this guard. + if (passphrase != null && passphrase.length > 0) { + throw new CloudRuntimeException("Encrypted volume creation on RBD is not yet implemented (volume " + name + ")"); + } Map details = pool.getDetails(); String dataPool = (details == null) ? null : details.get(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL); @@ -1244,6 +1251,12 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, KVMPhysicalDisk disk = null; if (destPool.getType() == StoragePoolType.RBD) { + // See createPhysicalDisk(): encrypted root-disk-from-template on RBD is not implemented yet. + // createDiskFromTemplateOnRBD() does not consume the passphrase, so fail closed rather than + // silently producing a plaintext root volume. Tracks A/B replace this with real create logic. + if (passphrase != null && passphrase.length > 0) { + throw new CloudRuntimeException("Encrypted root volume creation from template on RBD is not yet implemented (volume " + name + ")"); + } disk = createDiskFromTemplateOnRBD(template, name, format, provisioningType, size, destPool, timeout); } else { try (KeyFile keyFile = new KeyFile(passphrase)){ From e599375b5d02676cb75b83e070358550aa06325d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Thu, 25 Jun 2026 12:25:10 +0200 Subject: [PATCH 02/41] kvm: Ceph-native LUKS2 encryption for RBD volumes (engine='librbd') Implements encrypted RBD data and root disks using librbd's native LUKS2 encryption, decrypted at runtime by libvirt/qemu via . CloudStack manages the passphrase (existing model). - RbdEncryption: isolated helper wrapping `rbd encryption format luks2`, cephx via --id + keyfile (secret not on the command line), LUKS passphrase via KeyFile. Kept separate so the CLI can later be swapped for a JNA binding (rados-java has no rbd_encryption_format API). - LibvirtStorageAdaptor: create/clone the raw RBD image, then apply `rbd encryption format luks2`; mark the disk LUKS2 so encrypt_format propagates to the volume. Replaces the fail-closed guards. - QemuObject.EncryptFormat: add LUKS2. - LibvirtVMDef: render ; the encrypt details now carry an optional engine. - attach (KVMStorageProcessor) and boot (LibvirtComputingResource): set engine='librbd' for RBD-backed encrypted volumes. NOTE: the CoW-clone-then-format path (encrypted root from an unencrypted template) needs live-cluster validation for the parent-grow / usable-size behaviour described in the Ceph image-encryption docs. Builds: api + plugins/hypervisors/kvm (JDK11). --- .../resource/LibvirtComputingResource.java | 4 +- .../hypervisor/kvm/resource/LibvirtVMDef.java | 13 ++- .../kvm/storage/KVMStorageProcessor.java | 4 +- .../kvm/storage/LibvirtStorageAdaptor.java | 51 ++++++--- .../cloudstack/utils/qemu/QemuObject.java | 1 + .../cloudstack/utils/rbd/RbdEncryption.java | 107 ++++++++++++++++++ 6 files changed, 160 insertions(+), 20 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 4281036d9456..29db986c9e1d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -3882,7 +3882,9 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { if (volumeObjectTO.requiresEncryption() && pool.getType().encryptionSupportMode() == Storage.EncryptionSupport.Hypervisor ) { String secretUuid = createLibvirtVolumeSecret(conn, volumeObjectTO.getPath(), volumeObjectTO.getPassphrase()); - DiskDef.LibvirtDiskEncryptDetails encryptDetails = new DiskDef.LibvirtDiskEncryptDetails(secretUuid, QemuObject.EncryptFormat.enumValue(volumeObjectTO.getEncryptFormat())); + // RBD volumes are encrypted natively by librbd, so request the librbd encryption engine. + String encryptEngine = (pool.getType() == StoragePoolType.RBD) ? "librbd" : null; + DiskDef.LibvirtDiskEncryptDetails encryptDetails = new DiskDef.LibvirtDiskEncryptDetails(secretUuid, QemuObject.EncryptFormat.enumValue(volumeObjectTO.getEncryptFormat()), encryptEngine); disk.setLibvirtDiskEncryptDetails(encryptDetails); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java index 74529d9d5fa2..439e4f663416 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java @@ -788,14 +788,21 @@ public static class DiskDef { public static class LibvirtDiskEncryptDetails { String passphraseUuid; QemuObject.EncryptFormat encryptFormat; + String engine; // optional libvirt encryption engine (e.g. "librbd"); null => libvirt/qemu default public LibvirtDiskEncryptDetails(String passphraseUuid, QemuObject.EncryptFormat encryptFormat) { + this(passphraseUuid, encryptFormat, null); + } + + public LibvirtDiskEncryptDetails(String passphraseUuid, QemuObject.EncryptFormat encryptFormat, String engine) { this.passphraseUuid = passphraseUuid; this.encryptFormat = encryptFormat; + this.engine = engine; } public String getPassphraseUuid() { return this.passphraseUuid; } public QemuObject.EncryptFormat getEncryptFormat() { return this.encryptFormat; } + public String getEngine() { return this.engine; } } public static class DiskGeometry { @@ -1446,7 +1453,11 @@ public String toString() { } if (encryptDetails != null) { - diskBuilder.append("\n"); + diskBuilder.append("\n"); diskBuilder.append("\n"); diskBuilder.append("\n"); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 11acb9546b53..9749c7896c77 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1808,7 +1808,9 @@ public Answer attachVolume(final AttachCommand cmd) { DiskDef.LibvirtDiskEncryptDetails encryptDetails = null; if (vol.requiresEncryption()) { String secretUuid = resource.createLibvirtVolumeSecret(conn, vol.getPath(), vol.getPassphrase()); - encryptDetails = new DiskDef.LibvirtDiskEncryptDetails(secretUuid, QemuObject.EncryptFormat.enumValue(vol.getEncryptFormat())); + // RBD volumes are encrypted natively by librbd, so request the librbd encryption engine. + String encryptEngine = (primaryStore.getPoolType() == StoragePoolType.RBD) ? "librbd" : null; + encryptDetails = new DiskDef.LibvirtDiskEncryptDetails(secretUuid, QemuObject.EncryptFormat.enumValue(vol.getEncryptFormat()), encryptEngine); vol.clearPassphrase(); } 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 e74ca6aeaba3..229feb6a92d1 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 @@ -34,7 +34,9 @@ import com.cloud.agent.properties.AgentProperties; import com.cloud.agent.properties.AgentPropertiesFileHandler; import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.utils.cryptsetup.CryptSetup; import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.apache.cloudstack.utils.rbd.RbdEncryption; import org.apache.cloudstack.utils.qemu.QemuImageOptions; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; @@ -985,18 +987,21 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, StoragePoolType poolType = pool.getType(); if (StoragePoolType.RBD.equals(poolType)) { - // RBD is advertised as encryption-capable (StoragePoolType.RBD -> EncryptionSupport.Hypervisor), - // but the encrypted RBD create path is not implemented on this branch yet. Fail closed so we never - // silently create a plaintext volume that the control plane believes is encrypted. - // The qemu-native (engine='qemu') and ceph-native (engine='librbd') tracks each replace this guard. - if (passphrase != null && passphrase.length > 0) { - throw new CloudRuntimeException("Encrypted volume creation on RBD is not yet implemented (volume " + name + ")"); - } Map details = pool.getDetails(); String dataPool = (details == null) ? null : details.get(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL); - return (dataPool == null) ? createPhysicalDiskByLibVirt(name, pool, PhysicalDiskFormat.RAW, provisioningType, size) : - createPhysicalDiskByQemuImg(name, pool, PhysicalDiskFormat.RAW, provisioningType, size, passphrase); + // Create the raw RBD image first. For encrypted volumes we apply a native librbd LUKS header + // afterwards via `rbd encryption format` (engine='librbd'). We deliberately do NOT hand the + // passphrase to qemu-img, which would instead produce a qemu-native LUKS container. + KVMPhysicalDisk disk = (dataPool == null) ? + createPhysicalDiskByLibVirt(name, pool, PhysicalDiskFormat.RAW, provisioningType, size) : + createPhysicalDiskByQemuImg(name, pool, PhysicalDiskFormat.RAW, provisioningType, size, null); + + if (passphrase != null && passphrase.length > 0) { + formatRbdImageEncryption(pool, name, passphrase); + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); + } + return disk; } else if (QEMU_IMG_MANAGED_POOL_TYPES.contains(poolType)) { switch (format) { case QCOW2: @@ -1251,13 +1256,7 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, KVMPhysicalDisk disk = null; if (destPool.getType() == StoragePoolType.RBD) { - // See createPhysicalDisk(): encrypted root-disk-from-template on RBD is not implemented yet. - // createDiskFromTemplateOnRBD() does not consume the passphrase, so fail closed rather than - // silently producing a plaintext root volume. Tracks A/B replace this with real create logic. - if (passphrase != null && passphrase.length > 0) { - throw new CloudRuntimeException("Encrypted root volume creation from template on RBD is not yet implemented (volume " + name + ")"); - } - disk = createDiskFromTemplateOnRBD(template, name, format, provisioningType, size, destPool, timeout); + disk = createDiskFromTemplateOnRBD(template, name, format, provisioningType, size, destPool, timeout, passphrase); } else { try (KeyFile keyFile = new KeyFile(passphrase)){ String newUuid = name; @@ -1337,7 +1336,7 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, } private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, - String name, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, KVMStoragePool destPool, int timeout){ + String name, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, KVMStoragePool destPool, int timeout, byte[] passphrase){ /* With RBD you can't run qemu-img convert with an existing RBD image as destination @@ -1506,9 +1505,27 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, disk = null; } } + + if (disk != null && passphrase != null && passphrase.length > 0) { + // Apply native librbd LUKS to the freshly created/cloned root image (engine='librbd'). + // NOTE: when the destination is a CoW clone of an (unencrypted) template, `rbd encryption + // format` reserves LUKS-header space; the parent-grow / usable-size behaviour described in the + // Ceph "Image Encryption" docs must be validated against a live cluster (see spec B gotchas). + formatRbdImageEncryption(destPool, newUuid, passphrase); + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); + } return disk; } + /** + * Apply native librbd LUKS encryption to an existing RBD image via the rbd CLI. + * Isolated here so the CLI dependency can later be swapped for a native (JNA) librbd binding. + */ + private void formatRbdImageEncryption(KVMStoragePool pool, String image, byte[] passphrase) { + new RbdEncryption().format(pool.getSourceHost(), pool.getSourcePort(), pool.getAuthUserName(), + pool.getAuthSecret(), pool.getSourceDir(), image, passphrase, CryptSetup.LuksType.LUKS2); + } + @Override public KVMPhysicalDisk createTemplateFromDisk(KVMPhysicalDisk disk, String name, PhysicalDiskFormat format, long size, KVMStoragePool destPool) { return null; diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuObject.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuObject.java index efeee04cb90f..511e91074969 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuObject.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuObject.java @@ -54,6 +54,7 @@ public enum ObjectParameter { */ public enum EncryptFormat { LUKS("luks"), + LUKS2("luks2"), AES("aes"); private final String format; 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 new file mode 100644 index 000000000000..a062334cce25 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java @@ -0,0 +1,107 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.utils.rbd; + +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; +import org.apache.cloudstack.utils.cryptsetup.CryptSetup; +import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +/** + * Thin wrapper around the {@code rbd} CLI to apply native librbd LUKS encryption to an + * RBD image via {@code rbd encryption format}. This is only used at volume create time; + * runtime decryption is handled by libvirt/qemu through {@code }. + * + * The CLI dependency is intentionally isolated in this class so it can later be replaced + * by a native librbd (JNA) binding without touching callers. rados-java (0.x) does not + * expose the rbd_encryption_format API, hence the CLI for now. + */ +public class RbdEncryption { + protected static Logger LOGGER = LogManager.getLogger(RbdEncryption.class); + + protected String commandPath = "rbd"; + + public RbdEncryption() {} + + public RbdEncryption(String commandPath) { + this.commandPath = commandPath; + } + + /** + * Apply a LUKS header to an existing RBD image so librbd can transparently encrypt it. + *

+ * cephx authentication is supplied via {@code --id} plus a temporary keyfile so the secret + * never appears on the command line. The LUKS passphrase is supplied via a temporary file + * ({@link KeyFile}). Both temp files are deleted when this method returns. + * + * @param monHost ceph monitor host + * @param monPort ceph monitor port (0 to omit) + * @param authUser cephx user (e.g. "cloudstack"); null to skip --id + * @param authSecret cephx secret/key (as used for ceph "key" config); null to skip --keyfile + * @param cephPool ceph pool name + * @param image rbd image name + * @param passphrase LUKS passphrase + * @param luksType LUKS1/LUKS2 (librbd engine supports both; LUKS2 recommended) + */ + public void format(String monHost, int monPort, String authUser, String authSecret, + String cephPool, String image, byte[] passphrase, CryptSetup.LuksType luksType) { + final String imageSpec = cephPool + "/" + image; + try (KeyFile passFile = new KeyFile(passphrase); + KeyFile cephKeyFile = new KeyFile(authSecret == null ? null : authSecret.getBytes(StandardCharsets.UTF_8))) { + final Script script = new Script(commandPath); + script.add("encryption"); + script.add("format"); + script.add(imageSpec); + script.add(luksType.toString()); + script.add(passFile.toString()); + script.add("--mon-host"); + script.add(monPort > 0 ? monHost + ":" + monPort : monHost); + if (authUser != null) { + script.add("--id"); + script.add(authUser); + } + if (cephKeyFile.isSet()) { + script.add("--keyfile"); + script.add(cephKeyFile.toString()); + } + + final String result = script.execute(); + if (result != null) { + throw new CloudRuntimeException(String.format("Failed to apply librbd %s encryption to %s: %s", luksType, imageSpec, result)); + } + LOGGER.debug("Applied {} encryption to RBD image {}", luksType, imageSpec); + } catch (IOException ex) { + throw new CloudRuntimeException(String.format("Failed to apply librbd %s encryption to %s", luksType, imageSpec), ex); + } + } + + /** + * Best-effort probe that the local rbd CLI supports the encryption subcommand. + */ + public boolean isSupported() { + final Script script = new Script(commandPath); + script.add("help"); + script.add("encryption"); + script.add("format"); + return script.execute() == null; + } +} From d16730c1f345b9b4f67ee4041d9abfa322e5960f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Thu, 25 Jun 2026 13:01:34 +0200 Subject: [PATCH 03/41] kvm: gate host encryption probe on librbd support for RBD hostSupportsVolumeEncryption() now advertises encryption capability if the host supports EITHER qemu-native LUKS (qemu-img LUKS + cryptsetup) OR librbd native encryption (rbd CLI with the encryption subcommand). Previously a Ceph-only host that lacked cryptsetup would not advertise encryption even though librbd can encrypt RBD volumes. Split into hostSupportsQemuNativeVolumeEncryption() and hostSupportsRbdVolumeEncryption(); kept HOST_VOLUME_ENCRYPTION as the single host-wide flag (documented limitation: not per-pool). --- .../resource/LibvirtComputingResource.java | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 29db986c9e1d..20677c1c08a3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -91,6 +91,7 @@ import org.apache.cloudstack.storage.volume.VolumeOnStorageTO; import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; import org.apache.cloudstack.utils.cryptsetup.CryptSetup; +import org.apache.cloudstack.utils.rbd.RbdEncryption; import org.apache.cloudstack.utils.hypervisor.HypervisorUtils; import org.apache.cloudstack.utils.linux.CPUStat; import org.apache.cloudstack.utils.linux.KVMHostInfo; @@ -6197,10 +6198,28 @@ public boolean isHostSecured() { } /** - * Test host for volume encryption support + * Test host for volume encryption support. A host is considered encryption-capable if it + * supports EITHER mechanism CloudStack can use: + * - qemu-native LUKS (qemu-img LUKS + cryptsetup) for file/block backed pools, or + * - librbd native encryption (rbd encryption format) for RBD/Ceph pools. + * NOTE: HOST_VOLUME_ENCRYPTION is a single host-wide flag and is not per-pool, so a host that + * advertises encryption via only one mechanism could still be selected for a volume that needs + * the other. In practice hosts that do encryption have the qemu-native stack; the librbd branch + * additionally covers Ceph-only hosts. * @return boolean */ public boolean hostSupportsVolumeEncryption() { + boolean supported = hostSupportsQemuNativeVolumeEncryption() || hostSupportsRbdVolumeEncryption(); + if (!supported) { + LOGGER.info("Host does not support volume encryption (no qemu-native LUKS + cryptsetup, and no librbd rbd encryption)"); + } + return supported; + } + + /** + * Test host for qemu-native LUKS volume encryption (qemu-img LUKS support + cryptsetup). + */ + public boolean hostSupportsQemuNativeVolumeEncryption() { // test qemu-img try { QemuImg qemu = new QemuImg(0); @@ -6222,6 +6241,13 @@ public boolean hostSupportsVolumeEncryption() { return true; } + /** + * Test host for librbd native LUKS encryption support (rbd CLI with the encryption subcommand). + */ + public boolean hostSupportsRbdVolumeEncryption() { + return new RbdEncryption().isSupported(); + } + public boolean isSecureMode(String bootMode) { if (StringUtils.isNotBlank(bootMode) && "secure".equalsIgnoreCase(bootMode)) { return true; From af6b8d792c0c0fbfea4b6aa2703d60f6cc61cd35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Thu, 25 Jun 2026 13:12:27 +0200 Subject: [PATCH 04/41] kvm: resize support for librbd-encrypted RBD volumes (#5) Encrypted RBD volumes are encrypted natively by librbd and must be resized with `rbd resize --encryption-passphrase-file` so librbd grows the encrypted payload and keeps the LUKS header consistent. The existing encrypted-resize path (resizeEncryptedQcowFile) uses qemu-img --object secret, which is for qemu-native LUKS and does not fit the librbd LUKS2 layout. - RbdEncryption.resize(): new `rbd resize` wrapper (cephx via --id + keyfile, passphrase via KeyFile, optional --allow-shrink). - LibvirtResizeVolumeCommandWrapper: detect encrypted RBD and route to the rbd resize path, bypassing the libvirt v.resize and qemu-img paths. Snapshot/revert, RBD<->RBD copy, and migration of encrypted RBD volumes need no code changes: they operate on the raw (LUKS-containing) image at the block level, and the destination passphrase secret is already created engine-agnostic in LibvirtPrepareForMigrationCommandWrapper. These still require live validation. Builds: plugins/hypervisors/kvm (JDK11). --- .../LibvirtResizeVolumeCommandWrapper.java | 27 +++++++++++- .../cloudstack/utils/rbd/RbdEncryption.java | 44 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java index a43b584dd6d6..2a1f9cb0cd9e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java @@ -33,6 +33,7 @@ import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuObject; +import org.apache.cloudstack.utils.rbd.RbdEncryption; import org.libvirt.Connect; import org.libvirt.Domain; import org.libvirt.DomainInfo; @@ -93,6 +94,13 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR final String path = vol.getPath(); String type = notifyOnlyType; + // Encrypted RBD volumes are encrypted natively by librbd; they must be resized via + // `rbd resize --encryption-passphrase-file` so librbd grows the encrypted payload and keeps + // the LUKS header consistent. The libvirt/qemu-img resize paths below are for qemu-native + // encryption and would not handle the librbd LUKS2 layout. + final boolean rbdEncrypted = pool.getType() == StoragePoolType.RBD + && command.getPassphrase() != null && command.getPassphrase().length > 0; + if (spool.getType().equals(StoragePoolType.PowerFlex) && vol.getFormat().equals(PhysicalDiskFormat.QCOW2)) { // PowerFlex QCOW2 sizing needs to consider overhead. newSize = ScaleIOStorageAdaptor.getUsableBytesFromRawBytes(newSize); @@ -115,7 +123,7 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR /* libvirt doesn't support resizing (C)LVM devices, and corrupts QCOW2 in some scenarios, so we have to do these via qemu-img */ if (pool.getType() != StoragePoolType.CLVM && pool.getType() != StoragePoolType.CLVM_NG && pool.getType() != StoragePoolType.Linstor && pool.getType() != StoragePoolType.PowerFlex - && vol.getFormat() != PhysicalDiskFormat.QCOW2) { + && vol.getFormat() != PhysicalDiskFormat.QCOW2 && !rbdEncrypted) { logger.debug("Volume " + path + " can be resized by libvirt. Asking libvirt to resize the volume."); try { final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); @@ -143,7 +151,12 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR If VM is online, the existing resize script will call virsh blockresize which works with both encrypted and non-encrypted volumes. */ - if (!vmIsRunning && command.getPassphrase() != null && command.getPassphrase().length > 0 ) { + if (rbdEncrypted) { + logger.debug("Invoking rbd to resize an encrypted (librbd) RBD volume"); + // NOTE: for a running VM this grows the rbd image, but the guest may not see the new size + // until the domain is notified/restarted. Online notification needs live validation. + resizeRbdEncryptedVolume(pool, vol, newSize, shrinkOk, command.getPassphrase()); + } else if (!vmIsRunning && command.getPassphrase() != null && command.getPassphrase().length > 0 ) { logger.debug("Invoking qemu-img to resize an offline, encrypted volume"); QemuObject.EncryptFormat encryptFormat = QemuObject.EncryptFormat.enumValue(command.getEncryptFormat()); resizeEncryptedQcowFile(vol, encryptFormat,newSize, command.getPassphrase(), libvirtComputingResource); @@ -213,6 +226,16 @@ private void resizeEncryptedQcowFile(final KVMPhysicalDisk vol, final QemuObject } } + private void resizeRbdEncryptedVolume(final KVMStoragePool pool, final KVMPhysicalDisk vol, long newSize, + boolean shrinkOk, byte[] passphrase) throws CloudRuntimeException { + try { + new RbdEncryption().resize(pool.getSourceHost(), pool.getSourcePort(), pool.getAuthUserName(), + pool.getAuthSecret(), pool.getSourceDir(), vol.getName(), newSize, shrinkOk, passphrase); + } finally { + Arrays.fill(passphrase, (byte) 0); + } + } + private Answer handleMultipathSCSIResize(ResizeVolumeCommand command, KVMStoragePool pool) { ((MultipathSCSIPool)pool).resize(command.getPath(), command.getInstanceName(), command.getNewSize()); return new ResizeVolumeAnswer(command, true, ""); 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 a062334cce25..c4f7660cf008 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 @@ -94,6 +94,50 @@ public void format(String monHost, int monPort, String authUser, String authSecr } } + /** + * Resize an encrypted RBD image. librbd needs the passphrase so it can resize the encrypted + * payload (not just the raw image) and keep the LUKS header consistent. {@code newSizeBytes} is + * the usable (decrypted) size requested; rbd {@code --size} is expressed in MiB. + * + * @param allowShrink pass --allow-shrink when shrinking is permitted + */ + public void resize(String monHost, int monPort, String authUser, String authSecret, + String cephPool, String image, long newSizeBytes, boolean allowShrink, byte[] passphrase) { + final String imageSpec = cephPool + "/" + image; + final long sizeMiB = newSizeBytes / (1024L * 1024L); + try (KeyFile passFile = new KeyFile(passphrase); + KeyFile cephKeyFile = new KeyFile(authSecret == null ? null : authSecret.getBytes(StandardCharsets.UTF_8))) { + final Script script = new Script(commandPath); + script.add("resize"); + script.add("--size"); + script.add(String.valueOf(sizeMiB)); + script.add(imageSpec); + script.add("--encryption-passphrase-file"); + script.add(passFile.toString()); + if (allowShrink) { + script.add("--allow-shrink"); + } + script.add("--mon-host"); + script.add(monPort > 0 ? monHost + ":" + monPort : monHost); + if (authUser != null) { + script.add("--id"); + script.add(authUser); + } + if (cephKeyFile.isSet()) { + script.add("--keyfile"); + script.add(cephKeyFile.toString()); + } + + final String result = script.execute(); + if (result != null) { + throw new CloudRuntimeException(String.format("Failed to resize encrypted RBD image %s to %d MiB: %s", imageSpec, sizeMiB, result)); + } + LOGGER.debug("Resized encrypted RBD image {} to {} MiB", imageSpec, sizeMiB); + } catch (IOException ex) { + throw new CloudRuntimeException(String.format("Failed to resize encrypted RBD image %s", imageSpec), ex); + } + } + /** * Best-effort probe that the local rbd CLI supports the encryption subcommand. */ From a729f6fec69b7ca55ab8f254ca4638bdb9e0ed9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Thu, 25 Jun 2026 13:24:57 +0200 Subject: [PATCH 05/41] kvm: route online resize of encrypted RBD through virsh blockresize For a running VM, an librbd-encrypted RBD volume must be resized in-band by qemu/librbd, not out-of-band by the rbd CLI. Gate the CLI rbd-resize path on !vmIsRunning so: - offline -> `rbd resize --encryption-passphrase-file` (librbd-aware), and - online -> existing NOTIFYONLY path -> virsh blockresize, where qemu's block_resize delegates to librbd to grow the encrypted payload and notify the guest in one step (no passphrase needed; qemu holds the secret). This avoids notify-less out-of-band growth and qemu/librbd size divergence while the image is open. Online behaviour still needs live validation that blockresize resizes the encrypted payload for engine='librbd' disks. --- .../wrapper/LibvirtResizeVolumeCommandWrapper.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java index 2a1f9cb0cd9e..20e3891478b4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java @@ -147,14 +147,13 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR boolean vmIsRunning = isVmRunning(vmInstanceName, libvirtComputingResource); - /* when VM is offline, we use qemu-img directly to resize encrypted volumes. - If VM is online, the existing resize script will call virsh blockresize which works - with both encrypted and non-encrypted volumes. + /* when VM is offline, we use qemu-img (or rbd, for librbd-encrypted RBD) directly to resize + encrypted volumes. If VM is online, the existing resize script calls virsh blockresize, + which for an librbd-encrypted RBD disk lets qemu/librbd grow the encrypted payload and + notify the guest in one step (no passphrase needed, qemu already has the secret loaded). */ - if (rbdEncrypted) { - logger.debug("Invoking rbd to resize an encrypted (librbd) RBD volume"); - // NOTE: for a running VM this grows the rbd image, but the guest may not see the new size - // until the domain is notified/restarted. Online notification needs live validation. + if (rbdEncrypted && !vmIsRunning) { + logger.debug("Invoking rbd to resize an offline, encrypted (librbd) RBD volume"); resizeRbdEncryptedVolume(pool, vol, newSize, shrinkOk, command.getPassphrase()); } else if (!vmIsRunning && command.getPassphrase() != null && command.getPassphrase().length > 0 ) { logger.debug("Invoking qemu-img to resize an offline, encrypted volume"); From 559af2a8460aeb86d6211409101795e430f3260c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Sat, 4 Jul 2026 09:54:00 +0200 Subject: [PATCH 06/41] kvm: encrypted RBD root disks (thin CoW clone + full-copy fallback) Root disks could not be encrypted: cloning a plaintext template and then `rbd encryption format`ing the clone leaves the inherited OS data unreadable (the LUKS header offsets it), so the guest could not mount root. Fix, in createDiskFromTemplateOnRBD, with two paths: - Option A (same-cluster cached RBD template): grow the template base to reserve LUKS2 header space, snapshot+protect it (cloudstack-base-snap-luks), clone from it, apply the LUKS2 header, resize the clone to the requested size. Inherited template data stays readable through the clone's encryption and the clone is a thin CoW image (only the header is written). - Option B (first-use / non-RBD template): create an empty image, apply a LUKS2 header, then import the template THROUGH the encryption layer via RbdEncryption.importTemplate (qemu-img convert -n into encrypt.key-secret). Correct but a full copy. Validated end-to-end on Ubuntu 26.04 / libvirt 12.0.0: both boot; A is thin (3.5 GiB provisioned, ~120 MiB used); LUKS2 verified at rest on Ceph. --- .../kvm/storage/LibvirtStorageAdaptor.java | 98 +++++++++++++++++-- .../cloudstack/utils/rbd/RbdEncryption.java | 71 ++++++++++++++ 2 files changed, 161 insertions(+), 8 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 229feb6a92d1..3a6b02541743 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 @@ -97,6 +97,9 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { private static final int RBD_FEATURE_DEEP_FLATTEN = 32; public static final int RBD_FEATURES = RBD_FEATURE_LAYERING + RBD_FEATURE_EXCLUSIVE_LOCK + RBD_FEATURE_OBJECT_MAP + RBD_FEATURE_FAST_DIFF + RBD_FEATURE_DEEP_FLATTEN; private int rbdOrder = 0; /* Order 0 means 4MB blocks (the default) */ + /* Space reserved at the front of an encrypted RBD image for the LUKS2 header/keyslots so the + usable (decrypted) size still matches the requested volume size. */ + private static final long LUKS2_HEADER_RESERVE_BYTES = 16L << 20; // 16 MiB private static final Set QEMU_IMG_MANAGED_POOL_TYPES = Set.of(StoragePoolType.NetworkFilesystem, StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint); @@ -1363,6 +1366,91 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, } + if (passphrase != null && passphrase.length > 0) { + boolean sameClusterRbd = srcPool.getType() == StoragePoolType.RBD + && srcPool.getSourceHost().equals(destPool.getSourceHost()) + && srcPool.getSourceDir().equals(destPool.getSourceDir()); + + if (sameClusterRbd) { + /* + * Option A (thin CoW encrypted root). Per the Ceph "Image Encryption" clone recipe: + * grow the template base to reserve LUKS2-header space, snapshot+protect that grown state, + * clone from it, apply a LUKS2 header, then resize the clone to the requested size. The + * inherited (plaintext) template data stays readable through the clone's encryption, and + * the clone is a thin CoW image (only the header is written, not the OS data). + */ + String encSnap = rbdTemplateSnapName + "-luks"; + try { + Rados r = new Rados(destPool.getAuthUserName()); + r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + r.confSet("key", destPool.getAuthSecret()); + r.confSet("client_mount_timeout", "30"); + r.connect(); + IoCTX io = r.ioCtxCreate(destPool.getSourceDir()); + Rbd rbd = new Rbd(io); + RbdImage base = rbd.open(template.getName()); + boolean haveEncSnap = false; + for (RbdSnapInfo s : base.snapList()) { + if (encSnap.equals(s.name)) { + haveEncSnap = true; + break; + } + } + if (!haveEncSnap) { + base.resize(template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES); + base.snapCreate(encSnap); + base.snapProtect(encSnap); + logger.debug("Prepared LUKS-reserved template snapshot " + template.getName() + "@" + encSnap); + } + rbd.close(base); + rbd.clone(template.getName(), encSnap, io, newUuid, RBD_FEATURES, rbdOrder); + r.ioCtxDestroy(io); + } catch (RadosException | RbdException e) { + logger.error("Failed to create encrypted CoW clone " + newUuid + ": " + e.getMessage()); + return null; + } + formatRbdImageEncryption(destPool, newUuid, passphrase); + if (disk.getVirtualSize() > template.getVirtualSize()) { + // grow the clone to the requested root size (encryption-aware) + new RbdEncryption().resize(destPool.getSourceHost(), destPool.getSourcePort(), + destPool.getAuthUserName(), destPool.getAuthSecret(), destPool.getSourceDir(), + newUuid, disk.getVirtualSize(), false, passphrase); + } + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); + return disk; + } + + /* + * Option B (full-copy encrypted root) for templates not on the same RBD cluster (e.g. first + * use from secondary storage). Create an empty image, apply a LUKS2 header, then import the + * template THROUGH the encryption layer (qemu-img convert -n). Correct but not thin (no CoW). + */ + long createSize = disk.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES; + try { + Rados r = new Rados(destPool.getAuthUserName()); + r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + r.confSet("key", destPool.getAuthSecret()); + r.confSet("client_mount_timeout", "30"); + r.connect(); + IoCTX io = r.ioCtxCreate(destPool.getSourceDir()); + Rbd rbd = new Rbd(io); + rbd.create(newUuid, createSize, RBD_FEATURES, rbdOrder); + r.ioCtxDestroy(io); + } catch (RadosException | RbdException e) { + logger.error("Failed to create encrypted RBD image " + newUuid + ": " + e.getMessage()); + return null; + } + formatRbdImageEncryption(destPool, newUuid, passphrase); + boolean srcIsRbd = srcPool.getType() == StoragePoolType.RBD; + new RbdEncryption().importTemplate( + srcIsRbd ? srcPool.getSourceDir() : null, srcIsRbd ? template.getName() : null, + srcIsRbd ? null : template.getPath(), srcIsRbd ? null : template.getFormat().toString(), + destPool.getSourceHost(), destPool.getSourcePort(), destPool.getAuthUserName(), destPool.getAuthSecret(), + destPool.getSourceDir(), newUuid, passphrase, CryptSetup.LuksType.LUKS2); + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); + return disk; + } + QemuImgFile srcFile; QemuImgFile destFile = new QemuImgFile(KVMPhysicalDisk.RBDStringBuilder(destPool, disk.getPath())); destFile.setFormat(format); @@ -1506,14 +1594,8 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, } } - if (disk != null && passphrase != null && passphrase.length > 0) { - // Apply native librbd LUKS to the freshly created/cloned root image (engine='librbd'). - // NOTE: when the destination is a CoW clone of an (unencrypted) template, `rbd encryption - // format` reserves LUKS-header space; the parent-grow / usable-size behaviour described in the - // Ceph "Image Encryption" docs must be validated against a live cluster (see spec B gotchas). - formatRbdImageEncryption(destPool, newUuid, passphrase); - disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); - } + // Encrypted volumes are handled by the early return above (create empty -> luks2 format -> + // import template through encryption); the clone/convert path here is for plaintext volumes. return disk; } 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 c4f7660cf008..ce574ca70449 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 @@ -25,6 +25,8 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; /** * Thin wrapper around the {@code rbd} CLI to apply native librbd LUKS encryption to an @@ -39,6 +41,7 @@ public class RbdEncryption { protected static Logger LOGGER = LogManager.getLogger(RbdEncryption.class); protected String commandPath = "rbd"; + protected String qemuImgPath = "qemu-img"; public RbdEncryption() {} @@ -138,6 +141,74 @@ public void resize(String monHost, int monPort, String authUser, String authSecr } } + /** + * Import a template into an already-created, already-LUKS-formatted RBD image by writing it + * THROUGH the librbd encryption layer with {@code qemu-img convert -n} (so the data lands + * encrypted). This is how encrypted root disks are populated: we never clone-then-format a + * plaintext template (that leaves the inherited OS data unreadable) — instead we format an + * empty image and convert the template into it. + * + * Exactly one source must be given: an RBD image ({@code srcRbdPool}+{@code srcRbdImage}) or a + * local file ({@code srcFilePath}[+{@code srcFileFormat}]). cephx auth is provided to qemu-img + * via a temporary ceph.conf + keyring (deleted on return). + */ + public void importTemplate(String srcRbdPool, String srcRbdImage, + String srcFilePath, String srcFileFormat, + String monHost, int monPort, String authUser, String authSecret, + String cephPool, String destImage, byte[] passphrase, CryptSetup.LuksType luksType) { + final String imageSpec = cephPool + "/" + destImage; + final String monSpec = monPort > 0 ? monHost + ":" + monPort : monHost; + Path conf = null; + Path keyring = null; + try (KeyFile passFile = new KeyFile(passphrase)) { + keyring = Files.createTempFile("cs-ceph-", ".keyring"); // 0600 by default on POSIX + Files.writeString(keyring, "[client." + authUser + "]\n\tkey = " + authSecret + "\n"); + conf = Files.createTempFile("cs-ceph-", ".conf"); + Files.writeString(conf, "[global]\nmon_host = " + monSpec + "\nkeyring = " + keyring + "\n"); + + final Script q = new Script(qemuImgPath); + q.add("convert"); + q.add("-n"); // target already exists (pre-created + luks-formatted) + if (srcRbdImage != null) { + q.add("--image-opts"); + q.add("driver=rbd,pool=" + srcRbdPool + ",image=" + srcRbdImage + ",conf=" + conf + ",user=" + authUser); + } else { + if (srcFileFormat != null) { + q.add("-f"); + q.add(srcFileFormat.toLowerCase()); + } + q.add(srcFilePath); + } + q.add("--object"); + q.add("secret,id=luks0,file=" + passFile.toString()); + q.add("--target-image-opts"); + q.add("driver=rbd,pool=" + cephPool + ",image=" + destImage + ",conf=" + conf + ",user=" + authUser + + ",encrypt.format=" + luksType + ",encrypt.key-secret=luks0"); + + final String result = q.execute(); + if (result != null) { + throw new CloudRuntimeException(String.format("Failed to import template into encrypted RBD image %s: %s", imageSpec, result)); + } + LOGGER.debug("Imported template into encrypted RBD image {}", imageSpec); + } catch (IOException ex) { + throw new CloudRuntimeException(String.format("Failed to import template into encrypted RBD image %s", imageSpec), ex); + } finally { + deleteQuietly(conf); + deleteQuietly(keyring); + } + } + + private static void deleteQuietly(Path p) { + if (p == null) { + return; + } + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + // best-effort cleanup of the temporary ceph auth files + } + } + /** * Best-effort probe that the local rbd CLI supports the encryption subcommand. */ From b27d33bcd768b2dcf9abaa6f3cadc0de5e5c537c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Tue, 7 Jul 2026 10:38:16 +0200 Subject: [PATCH 07/41] kvm: harden and align librbd-encrypted RBD volume code Review pass over the librbd LUKS2 encryption feature to fix latent issues and bring it in line with CloudStack conventions: - RbdEncryption: reject empty/null passphrase with a clear error; round rbd --size up to MiB so a non-aligned request never shrinks the volume below what was asked for; create the temporary cephx conf/keyring 0600 explicitly instead of relying on the umask. - LibvirtStorageAdaptor: close Rados/IoCTX/RbdImage in a finally block on the encrypted-root paths (mirrors deleteVolume) so handles are not leaked on exceptions; use parameterized log messages instead of string concatenation; extract the encrypted-root Option A/B logic into createEncryptedRootCoWClone / createEncryptedRootFullCopy. - RbdEncryption: use an instance logger (matching the plugin convention) and split argv construction into build{Format,Resize,Convert}Script so the generated commands can be unit-tested. --- .../kvm/storage/LibvirtStorageAdaptor.java | 184 ++++++++++-------- .../cloudstack/utils/rbd/RbdEncryption.java | 171 +++++++++------- 2 files changed, 212 insertions(+), 143 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 3a6b02541743..86c7fd0c58bd 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 @@ -1370,85 +1370,10 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, boolean sameClusterRbd = srcPool.getType() == StoragePoolType.RBD && srcPool.getSourceHost().equals(destPool.getSourceHost()) && srcPool.getSourceDir().equals(destPool.getSourceDir()); - if (sameClusterRbd) { - /* - * Option A (thin CoW encrypted root). Per the Ceph "Image Encryption" clone recipe: - * grow the template base to reserve LUKS2-header space, snapshot+protect that grown state, - * clone from it, apply a LUKS2 header, then resize the clone to the requested size. The - * inherited (plaintext) template data stays readable through the clone's encryption, and - * the clone is a thin CoW image (only the header is written, not the OS data). - */ - String encSnap = rbdTemplateSnapName + "-luks"; - try { - Rados r = new Rados(destPool.getAuthUserName()); - r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); - r.confSet("key", destPool.getAuthSecret()); - r.confSet("client_mount_timeout", "30"); - r.connect(); - IoCTX io = r.ioCtxCreate(destPool.getSourceDir()); - Rbd rbd = new Rbd(io); - RbdImage base = rbd.open(template.getName()); - boolean haveEncSnap = false; - for (RbdSnapInfo s : base.snapList()) { - if (encSnap.equals(s.name)) { - haveEncSnap = true; - break; - } - } - if (!haveEncSnap) { - base.resize(template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES); - base.snapCreate(encSnap); - base.snapProtect(encSnap); - logger.debug("Prepared LUKS-reserved template snapshot " + template.getName() + "@" + encSnap); - } - rbd.close(base); - rbd.clone(template.getName(), encSnap, io, newUuid, RBD_FEATURES, rbdOrder); - r.ioCtxDestroy(io); - } catch (RadosException | RbdException e) { - logger.error("Failed to create encrypted CoW clone " + newUuid + ": " + e.getMessage()); - return null; - } - formatRbdImageEncryption(destPool, newUuid, passphrase); - if (disk.getVirtualSize() > template.getVirtualSize()) { - // grow the clone to the requested root size (encryption-aware) - new RbdEncryption().resize(destPool.getSourceHost(), destPool.getSourcePort(), - destPool.getAuthUserName(), destPool.getAuthSecret(), destPool.getSourceDir(), - newUuid, disk.getVirtualSize(), false, passphrase); - } - disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); - return disk; + return createEncryptedRootCoWClone(template, destPool, newUuid, disk, passphrase); } - - /* - * Option B (full-copy encrypted root) for templates not on the same RBD cluster (e.g. first - * use from secondary storage). Create an empty image, apply a LUKS2 header, then import the - * template THROUGH the encryption layer (qemu-img convert -n). Correct but not thin (no CoW). - */ - long createSize = disk.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES; - try { - Rados r = new Rados(destPool.getAuthUserName()); - r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); - r.confSet("key", destPool.getAuthSecret()); - r.confSet("client_mount_timeout", "30"); - r.connect(); - IoCTX io = r.ioCtxCreate(destPool.getSourceDir()); - Rbd rbd = new Rbd(io); - rbd.create(newUuid, createSize, RBD_FEATURES, rbdOrder); - r.ioCtxDestroy(io); - } catch (RadosException | RbdException e) { - logger.error("Failed to create encrypted RBD image " + newUuid + ": " + e.getMessage()); - return null; - } - formatRbdImageEncryption(destPool, newUuid, passphrase); - boolean srcIsRbd = srcPool.getType() == StoragePoolType.RBD; - new RbdEncryption().importTemplate( - srcIsRbd ? srcPool.getSourceDir() : null, srcIsRbd ? template.getName() : null, - srcIsRbd ? null : template.getPath(), srcIsRbd ? null : template.getFormat().toString(), - destPool.getSourceHost(), destPool.getSourcePort(), destPool.getAuthUserName(), destPool.getAuthSecret(), - destPool.getSourceDir(), newUuid, passphrase, CryptSetup.LuksType.LUKS2); - disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); - return disk; + return createEncryptedRootFullCopy(srcPool, template, destPool, newUuid, disk, passphrase); } QemuImgFile srcFile; @@ -1599,6 +1524,111 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, return disk; } + /** + * Option A (thin CoW encrypted root), used when the template already lives on the same RBD cluster + * as the destination pool. Per the Ceph "Image Encryption" clone recipe: grow the template base to + * reserve LUKS2-header space, snapshot+protect that grown state, clone from it, apply a LUKS2 header, + * then resize the clone to the requested size. The inherited (plaintext) template data stays readable + * through the clone's encryption, and the clone is a thin CoW image (only the header is written). + * + * @return the encrypted CoW clone, or {@code null} if the Ceph operations failed + */ + private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KVMStoragePool destPool, + String newUuid, KVMPhysicalDisk disk, byte[] passphrase) { + String encSnap = rbdTemplateSnapName + "-luks"; + Rados r = null; + IoCTX io = null; + Rbd rbd = null; + RbdImage base = null; + try { + r = new Rados(destPool.getAuthUserName()); + r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + r.confSet("key", destPool.getAuthSecret()); + r.confSet("client_mount_timeout", "30"); + r.connect(); + io = r.ioCtxCreate(destPool.getSourceDir()); + rbd = new Rbd(io); + base = rbd.open(template.getName()); + boolean haveEncSnap = false; + for (RbdSnapInfo s : base.snapList()) { + if (encSnap.equals(s.name)) { + haveEncSnap = true; + break; + } + } + if (!haveEncSnap) { + base.resize(template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES); + base.snapCreate(encSnap); + base.snapProtect(encSnap); + logger.debug("Prepared LUKS-reserved template snapshot {}@{}", template.getName(), encSnap); + } + rbd.clone(template.getName(), encSnap, io, newUuid, RBD_FEATURES, rbdOrder); + } catch (RadosException | RbdException e) { + logger.error("Failed to create encrypted CoW clone {}: {}", newUuid, e.getMessage()); + return null; + } finally { + if (rbd != null && base != null) { + try { + rbd.close(base); + } catch (RbdException ignored) { + // best-effort close of the template handle + } + } + if (r != null && io != null) { + r.ioCtxDestroy(io); + } + } + formatRbdImageEncryption(destPool, newUuid, passphrase); + if (disk.getVirtualSize() > template.getVirtualSize()) { + // grow the clone to the requested root size (encryption-aware) + new RbdEncryption().resize(destPool.getSourceHost(), destPool.getSourcePort(), + destPool.getAuthUserName(), destPool.getAuthSecret(), destPool.getSourceDir(), + newUuid, disk.getVirtualSize(), false, passphrase); + } + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); + return disk; + } + + /** + * Option B (full-copy encrypted root), used when the template is not on the same RBD cluster (e.g. first + * use from secondary storage). Create an empty image, apply a LUKS2 header, then import the template + * THROUGH the encryption layer (qemu-img convert -n). Correct but not thin (no CoW). + * + * @return the encrypted image, or {@code null} if the Ceph operations failed + */ + private KVMPhysicalDisk createEncryptedRootFullCopy(KVMStoragePool srcPool, KVMPhysicalDisk template, + KVMStoragePool destPool, String newUuid, KVMPhysicalDisk disk, byte[] passphrase) { + long createSize = disk.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES; + Rados r = null; + IoCTX io = null; + try { + r = new Rados(destPool.getAuthUserName()); + r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + r.confSet("key", destPool.getAuthSecret()); + r.confSet("client_mount_timeout", "30"); + r.connect(); + io = r.ioCtxCreate(destPool.getSourceDir()); + Rbd rbd = new Rbd(io); + rbd.create(newUuid, createSize, RBD_FEATURES, rbdOrder); + } catch (RadosException | RbdException e) { + logger.error("Failed to create encrypted RBD image {}: {}", newUuid, e.getMessage()); + return null; + } finally { + if (r != null && io != null) { + r.ioCtxDestroy(io); + } + } + formatRbdImageEncryption(destPool, newUuid, passphrase); + boolean srcIsRbd = srcPool.getType() == StoragePoolType.RBD; + new RbdEncryption().importTemplate( + srcIsRbd ? srcPool.getSourceDir() : null, srcIsRbd ? template.getName() : null, + srcIsRbd ? null : template.getPath(), srcIsRbd ? null : template.getFormat().toString(), + destPool.getSourceHost(), destPool.getSourcePort(), destPool.getAuthUserName(), destPool.getAuthSecret(), + destPool.getSourceDir(), newUuid, passphrase, CryptSetup.LuksType.LUKS2); + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); + return disk; + } + /** * Apply native librbd LUKS encryption to an existing RBD image via the rbd CLI. * Isolated here so the CLI dependency can later be swapped for a native (JNA) librbd binding. 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 ce574ca70449..0cb9b21fd3ca 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 @@ -27,6 +27,8 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermissions; /** * Thin wrapper around the {@code rbd} CLI to apply native librbd LUKS encryption to an @@ -36,9 +38,12 @@ * The CLI dependency is intentionally isolated in this class so it can later be replaced * by a native librbd (JNA) binding without touching callers. rados-java (0.x) does not * expose the rbd_encryption_format API, hence the CLI for now. + * + * The command builders ({@code build*Script}) are separated from execution so the generated + * argv can be unit-tested without a live Ceph cluster (see {@code RbdEncryptionTest}). */ public class RbdEncryption { - protected static Logger LOGGER = LogManager.getLogger(RbdEncryption.class); + protected Logger logger = LogManager.getLogger(getClass()); protected String commandPath = "rbd"; protected String qemuImgPath = "qemu-img"; @@ -49,6 +54,10 @@ public RbdEncryption(String commandPath) { this.commandPath = commandPath; } + private static String monSpec(String monHost, int monPort) { + return monPort > 0 ? monHost + ":" + monPort : monHost; + } + /** * Apply a LUKS header to an existing RBD image so librbd can transparently encrypt it. *

@@ -68,35 +77,44 @@ public RbdEncryption(String commandPath) { public void format(String monHost, int monPort, String authUser, String authSecret, String cephPool, String image, byte[] passphrase, CryptSetup.LuksType luksType) { final String imageSpec = cephPool + "/" + image; + if (passphrase == null || passphrase.length == 0) { + throw new CloudRuntimeException("Cannot LUKS-format RBD image " + imageSpec + ": empty passphrase"); + } try (KeyFile passFile = new KeyFile(passphrase); KeyFile cephKeyFile = new KeyFile(authSecret == null ? null : authSecret.getBytes(StandardCharsets.UTF_8))) { - final Script script = new Script(commandPath); - script.add("encryption"); - script.add("format"); - script.add(imageSpec); - script.add(luksType.toString()); - script.add(passFile.toString()); - script.add("--mon-host"); - script.add(monPort > 0 ? monHost + ":" + monPort : monHost); - if (authUser != null) { - script.add("--id"); - script.add(authUser); - } - if (cephKeyFile.isSet()) { - script.add("--keyfile"); - script.add(cephKeyFile.toString()); - } - + final Script script = buildFormatScript(imageSpec, luksType, passFile.toString(), + monSpec(monHost, monPort), authUser, cephKeyFile.isSet() ? cephKeyFile.toString() : null); final String result = script.execute(); if (result != null) { throw new CloudRuntimeException(String.format("Failed to apply librbd %s encryption to %s: %s", luksType, imageSpec, result)); } - LOGGER.debug("Applied {} encryption to RBD image {}", luksType, imageSpec); + logger.debug("Applied {} encryption to RBD image {}", luksType, imageSpec); } catch (IOException ex) { throw new CloudRuntimeException(String.format("Failed to apply librbd %s encryption to %s", luksType, imageSpec), ex); } } + protected Script buildFormatScript(String imageSpec, CryptSetup.LuksType luksType, String passFilePath, + String monSpec, String authUser, String cephKeyFilePath) { + final Script script = new Script(commandPath); + script.add("encryption"); + script.add("format"); + script.add(imageSpec); + script.add(luksType.toString()); + script.add(passFilePath); + script.add("--mon-host"); + script.add(monSpec); + if (authUser != null) { + script.add("--id"); + script.add(authUser); + } + if (cephKeyFilePath != null) { + script.add("--keyfile"); + script.add(cephKeyFilePath); + } + return script; + } + /** * Resize an encrypted RBD image. librbd needs the passphrase so it can resize the encrypted * payload (not just the raw image) and keep the LUKS header consistent. {@code newSizeBytes} is @@ -107,40 +125,50 @@ public void format(String monHost, int monPort, String authUser, String authSecr public void resize(String monHost, int monPort, String authUser, String authSecret, String cephPool, String image, long newSizeBytes, boolean allowShrink, byte[] passphrase) { final String imageSpec = cephPool + "/" + image; - final long sizeMiB = newSizeBytes / (1024L * 1024L); + if (passphrase == null || passphrase.length == 0) { + throw new CloudRuntimeException("Cannot resize encrypted RBD image " + imageSpec + ": empty passphrase"); + } + // rbd --size is in MiB; round up so a non-MiB-aligned request never shrinks the volume below what was asked for. + final long sizeMiB = (newSizeBytes + (1024L * 1024L) - 1) / (1024L * 1024L); try (KeyFile passFile = new KeyFile(passphrase); KeyFile cephKeyFile = new KeyFile(authSecret == null ? null : authSecret.getBytes(StandardCharsets.UTF_8))) { - final Script script = new Script(commandPath); - script.add("resize"); - script.add("--size"); - script.add(String.valueOf(sizeMiB)); - script.add(imageSpec); - script.add("--encryption-passphrase-file"); - script.add(passFile.toString()); - if (allowShrink) { - script.add("--allow-shrink"); - } - script.add("--mon-host"); - script.add(monPort > 0 ? monHost + ":" + monPort : monHost); - if (authUser != null) { - script.add("--id"); - script.add(authUser); - } - if (cephKeyFile.isSet()) { - script.add("--keyfile"); - script.add(cephKeyFile.toString()); - } - + final Script script = buildResizeScript(imageSpec, sizeMiB, passFile.toString(), allowShrink, + monSpec(monHost, monPort), authUser, cephKeyFile.isSet() ? cephKeyFile.toString() : null); final String result = script.execute(); if (result != null) { throw new CloudRuntimeException(String.format("Failed to resize encrypted RBD image %s to %d MiB: %s", imageSpec, sizeMiB, result)); } - LOGGER.debug("Resized encrypted RBD image {} to {} MiB", imageSpec, sizeMiB); + logger.debug("Resized encrypted RBD image {} to {} MiB", imageSpec, sizeMiB); } catch (IOException ex) { throw new CloudRuntimeException(String.format("Failed to resize encrypted RBD image %s", imageSpec), ex); } } + protected Script buildResizeScript(String imageSpec, long sizeMiB, String passFilePath, boolean allowShrink, + String monSpec, String authUser, String cephKeyFilePath) { + final Script script = new Script(commandPath); + script.add("resize"); + script.add("--size"); + script.add(String.valueOf(sizeMiB)); + script.add(imageSpec); + script.add("--encryption-passphrase-file"); + script.add(passFilePath); + if (allowShrink) { + script.add("--allow-shrink"); + } + script.add("--mon-host"); + script.add(monSpec); + if (authUser != null) { + script.add("--id"); + script.add(authUser); + } + if (cephKeyFilePath != null) { + script.add("--keyfile"); + script.add(cephKeyFilePath); + } + return script; + } + /** * Import a template into an already-created, already-LUKS-formatted RBD image by writing it * THROUGH the librbd encryption layer with {@code qemu-img convert -n} (so the data lands @@ -157,39 +185,26 @@ public void importTemplate(String srcRbdPool, String srcRbdImage, String monHost, int monPort, String authUser, String authSecret, String cephPool, String destImage, byte[] passphrase, CryptSetup.LuksType luksType) { final String imageSpec = cephPool + "/" + destImage; - final String monSpec = monPort > 0 ? monHost + ":" + monPort : monHost; + if (passphrase == null || passphrase.length == 0) { + throw new CloudRuntimeException("Cannot import template into encrypted RBD image " + imageSpec + ": empty passphrase"); + } Path conf = null; Path keyring = null; try (KeyFile passFile = new KeyFile(passphrase)) { - keyring = Files.createTempFile("cs-ceph-", ".keyring"); // 0600 by default on POSIX + // These temp files hold the cephx secret; create them 0600 up front (matching KeyFile) rather than relying on the umask. + 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"); - Files.writeString(conf, "[global]\nmon_host = " + monSpec + "\nkeyring = " + keyring + "\n"); - - final Script q = new Script(qemuImgPath); - q.add("convert"); - q.add("-n"); // target already exists (pre-created + luks-formatted) - if (srcRbdImage != null) { - q.add("--image-opts"); - q.add("driver=rbd,pool=" + srcRbdPool + ",image=" + srcRbdImage + ",conf=" + conf + ",user=" + authUser); - } else { - if (srcFileFormat != null) { - q.add("-f"); - q.add(srcFileFormat.toLowerCase()); - } - q.add(srcFilePath); - } - q.add("--object"); - q.add("secret,id=luks0,file=" + passFile.toString()); - q.add("--target-image-opts"); - q.add("driver=rbd,pool=" + cephPool + ",image=" + destImage + ",conf=" + conf + ",user=" + authUser - + ",encrypt.format=" + luksType + ",encrypt.key-secret=luks0"); + conf = Files.createTempFile("cs-ceph-", ".conf", ownerOnly); + Files.writeString(conf, "[global]\nmon_host = " + monSpec(monHost, monPort) + "\nkeyring = " + keyring + "\n"); + final Script q = buildConvertScript(srcRbdPool, srcRbdImage, srcFilePath, srcFileFormat, + cephPool, destImage, conf.toString(), authUser, passFile.toString(), luksType); final String result = q.execute(); if (result != null) { throw new CloudRuntimeException(String.format("Failed to import template into encrypted RBD image %s: %s", imageSpec, result)); } - LOGGER.debug("Imported template into encrypted RBD image {}", imageSpec); + logger.debug("Imported template into encrypted RBD image {}", imageSpec); } catch (IOException ex) { throw new CloudRuntimeException(String.format("Failed to import template into encrypted RBD image %s", imageSpec), ex); } finally { @@ -198,6 +213,30 @@ public void importTemplate(String srcRbdPool, String srcRbdImage, } } + protected Script buildConvertScript(String srcRbdPool, String srcRbdImage, String srcFilePath, String srcFileFormat, + String cephPool, String destImage, String confPath, String authUser, + String passFilePath, CryptSetup.LuksType luksType) { + final Script q = new Script(qemuImgPath); + q.add("convert"); + q.add("-n"); // target already exists (pre-created + luks-formatted) + if (srcRbdImage != null) { + q.add("--image-opts"); + q.add("driver=rbd,pool=" + srcRbdPool + ",image=" + srcRbdImage + ",conf=" + confPath + ",user=" + authUser); + } else { + if (srcFileFormat != null) { + q.add("-f"); + q.add(srcFileFormat.toLowerCase()); + } + q.add(srcFilePath); + } + q.add("--object"); + q.add("secret,id=luks0,file=" + passFilePath); + q.add("--target-image-opts"); + q.add("driver=rbd,pool=" + cephPool + ",image=" + destImage + ",conf=" + confPath + ",user=" + authUser + + ",encrypt.format=" + luksType + ",encrypt.key-secret=luks0"); + return q; + } + private static void deleteQuietly(Path p) { if (p == null) { return; From dca2494213919d523d7c6b009d6ec6e5a7fc7693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Tue, 7 Jul 2026 10:38:16 +0200 Subject: [PATCH 08/41] kvm: add RbdEncryption unit tests Assert the rbd/qemu-img argv built for format, resize and convert-through-encryption (RBD and file sources), and that empty/null passphrases are rejected. Command construction is verified without a live Ceph cluster. --- .../utils/rbd/RbdEncryptionTest.java | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/rbd/RbdEncryptionTest.java 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 new file mode 100644 index 000000000000..d31c63d2197e --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/rbd/RbdEncryptionTest.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.utils.rbd; + +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; +import org.apache.cloudstack.utils.cryptsetup.CryptSetup; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +import java.nio.charset.StandardCharsets; + +/** + * Unit tests for {@link RbdEncryption}. These assert the {@code rbd}/{@code qemu-img} argv that + * would be handed to {@link Script}, so the command construction is verified without a live Ceph + * cluster (the actual execution needs a real cluster and is covered by end-to-end testing). + */ +@RunWith(MockitoJUnitRunner.class) +public class RbdEncryptionTest { + + private final RbdEncryption rbdEncryption = new RbdEncryption(); + + @Test + public void buildFormatScriptWithCephxAuth() { + Script script = rbdEncryption.buildFormatScript("cloudstack/img", CryptSetup.LuksType.LUKS2, + "/tmp/pass", "1.2.3.4:6789", "cloudstack", "/tmp/key"); + String cmd = script.toString(); + Assert.assertTrue(cmd, cmd.contains("rbd encryption format cloudstack/img luks2 /tmp/pass")); + Assert.assertTrue(cmd, cmd.contains("--mon-host 1.2.3.4:6789")); + Assert.assertTrue(cmd, cmd.contains("--id cloudstack")); + Assert.assertTrue(cmd, cmd.contains("--keyfile /tmp/key")); + } + + @Test + public void buildFormatScriptWithoutCephxAuth() { + // authUser == null and cephKeyFilePath == null (e.g. auth-less cluster): no --id / --keyfile. + Script script = rbdEncryption.buildFormatScript("pool/vol", CryptSetup.LuksType.LUKS2, + "/tmp/pass", "mon:6789", null, null); + String cmd = script.toString(); + Assert.assertTrue(cmd, cmd.contains("rbd encryption format pool/vol luks2 /tmp/pass --mon-host mon:6789")); + Assert.assertFalse(cmd, cmd.contains("--id")); + Assert.assertFalse(cmd, cmd.contains("--keyfile")); + } + + @Test + public void buildResizeScriptGrowDoesNotAllowShrink() { + Script script = rbdEncryption.buildResizeScript("cloudstack/img", 10240L, "/tmp/pass", false, + "1.2.3.4:6789", "cloudstack", "/tmp/key"); + String cmd = script.toString(); + Assert.assertTrue(cmd, cmd.contains("rbd resize --size 10240 cloudstack/img --encryption-passphrase-file /tmp/pass")); + Assert.assertTrue(cmd, cmd.contains("--id cloudstack")); + Assert.assertFalse(cmd, cmd.contains("--allow-shrink")); + } + + @Test + public void buildResizeScriptShrinkPassesAllowShrink() { + Script script = rbdEncryption.buildResizeScript("cloudstack/img", 5120L, "/tmp/pass", true, + "1.2.3.4:6789", "cloudstack", "/tmp/key"); + Assert.assertTrue(script.toString(), script.toString().contains("--allow-shrink")); + } + + @Test + public void buildConvertScriptFromRbdSource() { + Script q = rbdEncryption.buildConvertScript("srcpool", "srcimg", null, null, + "cloudstack", "dst", "/tmp/conf", "cloudstack", "/tmp/pass", CryptSetup.LuksType.LUKS2); + String cmd = q.toString(); + Assert.assertTrue(cmd, cmd.contains("qemu-img convert -n")); + Assert.assertTrue(cmd, cmd.contains("--image-opts driver=rbd,pool=srcpool,image=srcimg,conf=/tmp/conf,user=cloudstack")); + Assert.assertTrue(cmd, cmd.contains("--object secret,id=luks0,file=/tmp/pass")); + Assert.assertTrue(cmd, cmd.contains("--target-image-opts driver=rbd,pool=cloudstack,image=dst,conf=/tmp/conf,user=cloudstack,encrypt.format=luks2,encrypt.key-secret=luks0")); + } + + @Test + public void buildConvertScriptFromFileSource() { + Script q = rbdEncryption.buildConvertScript(null, null, "/tmp/tmpl.qcow2", "QCOW2", + "cloudstack", "dst", "/tmp/conf", "cloudstack", "/tmp/pass", CryptSetup.LuksType.LUKS2); + String cmd = q.toString(); + Assert.assertTrue(cmd, cmd.contains("-f qcow2 /tmp/tmpl.qcow2")); + Assert.assertFalse(cmd, cmd.contains("--image-opts")); + Assert.assertTrue(cmd, cmd.contains("encrypt.format=luks2,encrypt.key-secret=luks0")); + } + + @Test + public void formatRejectsEmptyPassphrase() { + Assert.assertThrows(CloudRuntimeException.class, () -> rbdEncryption.format( + "1.2.3.4", 6789, "cloudstack", "secret", "cloudstack", "img", + new byte[0], CryptSetup.LuksType.LUKS2)); + } + + @Test + public void resizeRejectsNullPassphrase() { + Assert.assertThrows(CloudRuntimeException.class, () -> rbdEncryption.resize( + "1.2.3.4", 6789, "cloudstack", "secret", "cloudstack", "img", + 1L << 30, false, null)); + } + + @Test + public void importTemplateRejectsEmptyPassphrase() { + Assert.assertThrows(CloudRuntimeException.class, () -> rbdEncryption.importTemplate( + "srcpool", "srcimg", null, null, "1.2.3.4", 6789, "cloudstack", "secret", + "cloudstack", "dst", "".getBytes(StandardCharsets.UTF_8), CryptSetup.LuksType.LUKS2)); + } +} From bf87c885d9f0efcde6428a40fabe8bb0088d15cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Tue, 7 Jul 2026 11:40:04 +0200 Subject: [PATCH 09/41] kvm: refuse encrypted RBD hot-plug on libvirt < 10.1.0 libvirt 10.0.0 has an object apply-order bug (fixed in 10.1.0) that breaks hot-plug of an encrypted rbd blockdev: on attach the disk is opened before its LUKS secret object is defined, so the attach fails with "No secret with id '...-format-encryption-secret0'". Booting a VM from an encrypted RBD disk is unaffected (the QEMU command line resolves all -object before -blockdev). Refuse the attach up front with a clear error (mirroring the existing openvswitch/io_uring libvirt-version gates) instead of letting libvirt fail opaquely. Only the RBD hot-plug path is gated; boot/root/detach are untouched. --- .../kvm/storage/KVMStorageProcessor.java | 23 +++++++++++++++++++ .../kvm/storage/KVMStorageProcessorTest.java | 22 ++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 9749c7896c77..3466ebe389c3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -182,6 +182,12 @@ public class KVMStorageProcessor implements StorageProcessor { private static final String CEPH_AUTH_KEY = "key"; private static final String CEPH_CLIENT_MOUNT_TIMEOUT = "client_mount_timeout"; private static final String CEPH_DEFAULT_MOUNT_TIMEOUT = "30"; + + // libvirt < 10.1.0 has an object apply-order bug (fixed in 10.1.0) that breaks hot-plug of an encrypted + // blockdev: on attach the disk is opened before its LUKS secret object is defined, so the attach fails with + // "No secret with id '...-format-encryption-secret0'". Booting a VM from an encrypted disk is unaffected (the + // QEMU command line resolves all -object before -blockdev). See qemuBlockStorageSourceAttachApply() in libvirt. + private static final long MIN_LIBVIRT_VERSION_FOR_RBD_ENCRYPTED_HOTPLUG = 10001000L; // libvirt 10.1.0 /** * Time interval before rechecking virsh commands */ @@ -1795,6 +1801,20 @@ protected DiskDef.DiskBus getAttachDiskBusType(int deviceId, List disks return DiskDef.DiskBus.VIRTIO; } + /** + * libvirt < 10.1.0 cannot hot-plug an encrypted rbd blockdev (the LUKS secret is applied after the disk is + * opened), so refuse the attach with a clear message rather than letting libvirt fail with an opaque + * "No secret with id ..." error. Only the RBD hot-plug path is affected; booting a VM from an encrypted RBD + * disk works on older libvirt, so this does not gate the boot/root path. + */ + protected void ensureLibvirtSupportsEncryptedRbdHotplug(StoragePoolType poolType) { + if (poolType == StoragePoolType.RBD + && resource.getHypervisorLibvirtVersion() < MIN_LIBVIRT_VERSION_FOR_RBD_ENCRYPTED_HOTPLUG) { + throw new CloudRuntimeException("Libvirt version 10.1.0 required to attach an encrypted RBD volume to a running VM, but version " + + resource.getHypervisorLibvirtVersion() + " detected. Booting a VM from an encrypted RBD disk is not affected."); + } + } + @Override public Answer attachVolume(final AttachCommand cmd) { final DiskTO disk = cmd.getDisk(); @@ -1807,6 +1827,9 @@ public Answer attachVolume(final AttachCommand cmd) { final Connect conn = LibvirtConnection.getConnectionByVmName(vmName); DiskDef.LibvirtDiskEncryptDetails encryptDetails = null; if (vol.requiresEncryption()) { + // Encrypted RBD is decrypted by librbd inside qemu; hot-plugging it needs a libvirt new enough to + // emit the LUKS secret before the rbd blockdev. Booting from an encrypted RBD disk is unaffected. + ensureLibvirtSupportsEncryptedRbdHotplug(primaryStore.getPoolType()); String secretUuid = resource.createLibvirtVolumeSecret(conn, vol.getPath(), vol.getPassphrase()); // RBD volumes are encrypted natively by librbd, so request the librbd encryption engine. String encryptEngine = (primaryStore.getPoolType() == StoragePoolType.RBD) ? "librbd" : null; diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java index 0a58dfc79b0d..b895aa65ff59 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java @@ -21,6 +21,7 @@ import com.cloud.exception.InternalErrorException; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser; +import com.cloud.storage.Storage.StoragePoolType; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef; import com.cloud.storage.Storage; import com.cloud.storage.template.TemplateConstants; @@ -666,4 +667,25 @@ public void testComputeMd5Hash_ConsistentResults() { Assert.fail("Failed to test computeMd5Hash: " + e.getMessage()); } } + + @Test + public void ensureLibvirtSupportsEncryptedRbdHotplugRejectsOldLibvirt() { + Mockito.when(resource.getHypervisorLibvirtVersion()).thenReturn(10000000L); // libvirt 10.0.0 + CloudRuntimeException ex = Assert.assertThrows(CloudRuntimeException.class, + () -> storageProcessor.ensureLibvirtSupportsEncryptedRbdHotplug(StoragePoolType.RBD)); + Assert.assertTrue(ex.getMessage(), ex.getMessage().contains("Libvirt version 10.1.0 required")); + } + + @Test + public void ensureLibvirtSupportsEncryptedRbdHotplugAllowsNewLibvirt() { + Mockito.when(resource.getHypervisorLibvirtVersion()).thenReturn(10001000L); // libvirt 10.1.0 + storageProcessor.ensureLibvirtSupportsEncryptedRbdHotplug(StoragePoolType.RBD); // must not throw + } + + @Test + public void ensureLibvirtSupportsEncryptedRbdHotplugIgnoresNonRbd() { + // The libvirt hot-plug apply-order bug is RBD-specific; other pool types are not gated, and the libvirt + // version is not even consulted for them. + storageProcessor.ensureLibvirtSupportsEncryptedRbdHotplug(StoragePoolType.NetworkFilesystem); + } } From bc31693808bca40cf0626f9d19922424cdc22edf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Tue, 7 Jul 2026 11:59:26 +0200 Subject: [PATCH 10/41] docs: add PendingReleaseNotes entry for librbd-encrypted RBD volumes --- PendingReleaseNotes | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 9670b6e7c13a..76c040af002f 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -39,3 +39,13 @@ example.ver.1 > example.ver.2: which can now be attached to Instances. This is to prevent the Secondary Storage to grow to enormous sizes as Linux Distributions keep growing in size while a stripped down Linux should fit on a 2.88MB floppy. + + +4.22.0.0 > 4.23.0.0: + * KVM/Ceph: RBD volumes can now be encrypted at rest using native librbd + LUKS2 (), for both data disks + and root disks. Encryption is transparent to the guest and reuses the + existing CloudStack volume-encryption passphrase handling, so no + additional key store is required. Note: attaching an encrypted RBD volume + to a running Instance requires libvirt >= 10.1.0; booting an Instance from + an encrypted RBD root disk works on older libvirt. From 385fda2d3b2d5470a9e9d17ff77f1fd44e168ec8 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:53:30 +0200 Subject: [PATCH 11/41] kvm: route the encrypted RBD template import through QemuImg RbdEncryption built its own 'qemu-img convert' command line, which duplicated qemu-img knowledge outside of QemuImg. QemuImg could only write to a plain filename destination, so importing a template through the librbd encryption layer was not expressible with it. QemuImg now supports a destination described by image options (--target-image-opts, with -n implied since such a target always exists already), exposed as convertIntoExistingTarget(). QemuImageOptions can render its parameters under either image-opts flag. RbdEncryption.importTemplate now composes QemuImageOptions and a QemuObject secret and delegates to QemuImg; its hand-built convert script is removed. The rbd CLI calls (encryption format, resize, support probe) stay, as qemu-img cannot perform them. No functional change to the generated command. --- .../utils/qemu/QemuImageOptions.java | 11 ++- .../apache/cloudstack/utils/qemu/QemuImg.java | 56 +++++++++++- .../cloudstack/utils/rbd/RbdEncryption.java | 91 +++++++++++++------ .../utils/rbd/RbdEncryptionTest.java | 81 +++++++++++++---- 4 files changed, 186 insertions(+), 53 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImageOptions.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImageOptions.java index 4a577ef3400c..10e39cb5ffcb 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImageOptions.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImageOptions.java @@ -86,8 +86,17 @@ public String[] toCommandFlag() { return new String[] { params.get(FILENAME_PARAM_KEY) }; } } + return toCommandFlag(QemuImg.IMAGE_OPTS_FLAG); + } + + /** + * Converts QemuImageOptions into the command strings under the given qemu-img flag, + * e.g. {@link QemuImg#TARGET_IMAGE_OPTS_FLAG} for a convert destination. + * @return array of strings representing the flag and its options value + */ + public String[] toCommandFlag(String flagName) { Map sorted = new TreeMap<>(params); String paramString = Joiner.on(",").withKeyValueSeparator("=").join(sorted); - return new String[] {"--image-opts", paramString}; + return new String[] {flagName, paramString}; } } 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 cae6832999eb..49f531ed7c11 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 @@ -52,6 +52,8 @@ public class QemuImg { public static final String ENCRYPT_FORMAT = "encrypt.format"; public static final String ENCRYPT_KEY_SECRET = "encrypt.key-secret"; public static final String TARGET_ZERO_FLAG = "--target-is-zero"; + public static final String IMAGE_OPTS_FLAG = "--image-opts"; + public static final String TARGET_IMAGE_OPTS_FLAG = "--target-image-opts"; public static final String PREALLOCATION = "preallocation"; public static final long QEMU_2_10 = 2010000; public static final long QEMU_5_1 = 5001000; @@ -402,6 +404,21 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, convert(srcFile, destFile, null, options, qemuObjects, srcImageOpts, snapshotName, forceSourceFormat, false, false, false, null, null); } + /** + * Converts an image into an existing destination that is described by explicit image options + * ({@code --target-image-opts}) instead of a plain filename - for example an RBD image written + * through librbd encryption ({@code driver=rbd,...,encrypt.format=...}). The destination is + * never created ({@code -n} is implied) and must already exist with the wanted size and format. + * + * @param destImageOpts + * image options describing the existing destination; passed as --target-image-opts. + */ + public void convertIntoExistingTarget(final QemuImgFile srcFile, final Map options, + final List qemuObjects, final QemuImageOptions srcImageOpts, final QemuImageOptions destImageOpts, + final boolean forceSourceFormat) throws QemuImgException { + convert(srcFile, null, null, options, qemuObjects, srcImageOpts, destImageOpts, null, forceSourceFormat, false, false, false, null, null); + } + protected Map getResizeOptionsFromConvertOptions(final Map options) { if (MapUtils.isEmpty(options)) { return null; @@ -450,6 +467,25 @@ protected Map getResizeOptionsFromConvertOptions(final Map options, final List qemuObjects, final QemuImageOptions srcImageOpts, final String snapshotName, final boolean forceSourceFormat, boolean keepBitmaps, boolean outOfOrderWrites, boolean compress, Integer coroutines, Integer rateLimit) throws QemuImgException { + convert(srcFile, destFile, backingFile, options, qemuObjects, srcImageOpts, null, snapshotName, forceSourceFormat, keepBitmaps, outOfOrderWrites, compress, coroutines, + rateLimit); + } + + /** + * Converts an image from source to destination, optionally into an existing destination described by explicit image options + * ({@code --target-image-opts}) instead of a plain filename; see {@link #convertIntoExistingTarget}. All other parameters + * behave as documented above. + * + * @param destImageOpts + * If not null, the destination is described by these image options and {@code destFile} is unused. + */ + public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuImgFile backingFile, final Map options, final List qemuObjects, + final QemuImageOptions srcImageOpts, final QemuImageOptions destImageOpts, final String snapshotName, final boolean forceSourceFormat, boolean keepBitmaps, + boolean outOfOrderWrites, boolean compress, Integer coroutines, Integer rateLimit) throws QemuImgException { + if (destImageOpts != null && this.version < QEMU_2_10) { + throw new QemuImgException(String.format("qemu >= 2.10 is required to convert into a destination described by %s", TARGET_IMAGE_OPTS_FLAG)); + } + Script script = new Script(_qemuImgPath, timeout); if (StringUtils.isNotBlank(snapshotName)) { String qemuPath = Script.runSimpleBashScript(getQemuImgPathScript); @@ -458,7 +494,10 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuI script.add("convert"); - if (skipZero && Files.exists(Paths.get(destFile.getFileName()))) { + if (destImageOpts != null) { + // a destination described by image options always exists already; qemu-img requires -n with --target-image-opts + script.add("-n"); + } else if (skipZero && Files.exists(Paths.get(destFile.getFileName()))) { script.add("-n"); script.add(TARGET_ZERO_FLAG); script.add("-W"); @@ -469,8 +508,10 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuI script.add("-n"); } - script.add("-O"); - script.add(destFile.getFormat().toString()); + if (destImageOpts == null) { + script.add("-O"); + script.add(destFile.getFormat().toString()); + } addBackingFileToConvertCommand(script, backingFile); addScriptOptionsFromMap(options, script); @@ -524,14 +565,19 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuI script.add("--bitmaps"); } - script.add(destFile.getFileName()); + if (destImageOpts != null) { + script.add(destImageOpts.toCommandFlag(TARGET_IMAGE_OPTS_FLAG)); + } else { + script.add(destFile.getFileName()); + } final String result = script.execute(); if (result != null) { throw new QemuImgException(result); } - if (srcFile.getSize() < destFile.getSize()) { + // an image-options destination already exists with its final size; 'qemu-img resize' cannot address it by filename + if (destImageOpts == null && srcFile.getSize() < destFile.getSize()) { this.resize(destFile, destFile.getSize(), getResizeOptionsFromConvertOptions(options)); } } 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 0cb9b21fd3ca..a23ce93d2b82 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 @@ -20,8 +20,14 @@ import com.cloud.utils.script.Script; import org.apache.cloudstack.utils.cryptsetup.CryptSetup; import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.apache.cloudstack.utils.qemu.QemuImageOptions; +import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImgException; +import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.cloudstack.utils.qemu.QemuObject; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.libvirt.LibvirtException; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -29,6 +35,10 @@ import java.nio.file.Path; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumMap; +import java.util.HashMap; +import java.util.List; +import java.util.Map; /** * Thin wrapper around the {@code rbd} CLI to apply native librbd LUKS encryption to an @@ -46,7 +56,9 @@ public class RbdEncryption { protected Logger logger = LogManager.getLogger(getClass()); protected String commandPath = "rbd"; - protected String qemuImgPath = "qemu-img"; + + /** qemu secret id used to hand the LUKS passphrase to qemu-img during template import */ + protected static final String LUKS_SECRET_ID = "luks0"; public RbdEncryption() {} @@ -178,7 +190,8 @@ protected Script buildResizeScript(String imageSpec, long sizeMiB, String passFi * * Exactly one source must be given: an RBD image ({@code srcRbdPool}+{@code srcRbdImage}) or a * local file ({@code srcFilePath}[+{@code srcFileFormat}]). cephx auth is provided to qemu-img - * via a temporary ceph.conf + keyring (deleted on return). + * via a temporary ceph.conf + keyring (deleted on return). The conversion itself goes through + * {@link QemuImg#convertIntoExistingTarget}. */ public void importTemplate(String srcRbdPool, String srcRbdImage, String srcFilePath, String srcFileFormat, @@ -198,14 +211,38 @@ public void importTemplate(String srcRbdPool, String srcRbdImage, conf = Files.createTempFile("cs-ceph-", ".conf", ownerOnly); Files.writeString(conf, "[global]\nmon_host = " + monSpec(monHost, monPort) + "\nkeyring = " + keyring + "\n"); - final Script q = buildConvertScript(srcRbdPool, srcRbdImage, srcFilePath, srcFileFormat, - cephPool, destImage, conf.toString(), authUser, passFile.toString(), luksType); - final String result = q.execute(); - if (result != null) { - throw new CloudRuntimeException(String.format("Failed to import template into encrypted RBD image %s: %s", imageSpec, result)); + QemuImgFile srcQemuFile; + QemuImageOptions srcImageOpts; + boolean forceSourceFormat = false; + if (srcRbdImage != null) { + srcQemuFile = new QemuImgFile(srcRbdPool + "/" + srcRbdImage, QemuImg.PhysicalDiskFormat.RAW); + srcImageOpts = new QemuImageOptions(rbdImageOptions(srcRbdPool, srcRbdImage, conf.toString(), authUser)); + srcImageOpts.setImageOptsFlag(true); + } else { + QemuImg.PhysicalDiskFormat srcFormat = srcFileFormat != null ? QemuImg.PhysicalDiskFormat.valueOf(srcFileFormat.toUpperCase()) : null; + srcQemuFile = srcFormat != null ? new QemuImgFile(srcFilePath, srcFormat) : new QemuImgFile(srcFilePath); + srcImageOpts = new QemuImageOptions(srcFilePath); + if (srcFormat != null) { + // emit the source as --image-opts driver=,file.filename= (the -f equivalent) + srcImageOpts.setImageOptsFlag(true); + forceSourceFormat = true; + } } + + Map destParams = rbdImageOptions(cephPool, destImage, conf.toString(), authUser); + destParams.put("encrypt.format", luksType.toString()); + destParams.put("encrypt.key-secret", LUKS_SECRET_ID); + QemuImageOptions destImageOpts = new QemuImageOptions(destParams); + + EnumMap secretParams = new EnumMap<>(QemuObject.ObjectParameter.class); + secretParams.put(QemuObject.ObjectParameter.ID, LUKS_SECRET_ID); + secretParams.put(QemuObject.ObjectParameter.FILE, passFile.toString()); + QemuObject luksSecret = new QemuObject(QemuObject.ObjectType.SECRET, secretParams); + + QemuImg qemu = createQemuImg(); + qemu.convertIntoExistingTarget(srcQemuFile, null, List.of(luksSecret), srcImageOpts, destImageOpts, forceSourceFormat); logger.debug("Imported template into encrypted RBD image {}", imageSpec); - } catch (IOException ex) { + } catch (IOException | QemuImgException | LibvirtException ex) { throw new CloudRuntimeException(String.format("Failed to import template into encrypted RBD image %s", imageSpec), ex); } finally { deleteQuietly(conf); @@ -213,28 +250,24 @@ public void importTemplate(String srcRbdPool, String srcRbdImage, } } - protected Script buildConvertScript(String srcRbdPool, String srcRbdImage, String srcFilePath, String srcFileFormat, - String cephPool, String destImage, String confPath, String authUser, - String passFilePath, CryptSetup.LuksType luksType) { - final Script q = new Script(qemuImgPath); - q.add("convert"); - q.add("-n"); // target already exists (pre-created + luks-formatted) - if (srcRbdImage != null) { - q.add("--image-opts"); - q.add("driver=rbd,pool=" + srcRbdPool + ",image=" + srcRbdImage + ",conf=" + confPath + ",user=" + authUser); - } else { - if (srcFileFormat != null) { - q.add("-f"); - q.add(srcFileFormat.toLowerCase()); - } - q.add(srcFilePath); + /** + * Seam for unit tests; {@link QemuImg} probes the qemu version through libvirt on construction. + */ + protected QemuImg createQemuImg() throws QemuImgException, LibvirtException { + return new QemuImg(0); + } + + /** qemu image options addressing an RBD image (as used with --image-opts / --target-image-opts). */ + private static Map rbdImageOptions(String cephPool, String image, String confPath, String authUser) { + Map opts = new HashMap<>(); + opts.put("driver", "rbd"); + opts.put("pool", cephPool); + opts.put("image", image); + opts.put("conf", confPath); + if (authUser != null) { + opts.put("user", authUser); } - q.add("--object"); - q.add("secret,id=luks0,file=" + passFilePath); - q.add("--target-image-opts"); - q.add("driver=rbd,pool=" + cephPool + ",image=" + destImage + ",conf=" + confPath + ",user=" + authUser - + ",encrypt.format=" + luksType + ",encrypt.key-secret=luks0"); - return q; + return opts; } private static void deleteQuietly(Path p) { 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 d31c63d2197e..e4c977c676f1 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 @@ -21,17 +21,26 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; import org.apache.cloudstack.utils.cryptsetup.CryptSetup; +import org.apache.cloudstack.utils.qemu.QemuImageOptions; +import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.cloudstack.utils.qemu.QemuObject; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.stream.Collectors; /** - * Unit tests for {@link RbdEncryption}. These assert the {@code rbd}/{@code qemu-img} argv that - * would be handed to {@link Script}, so the command construction is verified without a live Ceph - * cluster (the actual execution needs a real cluster and is covered by end-to-end testing). + * Unit tests for {@link RbdEncryption}. These assert the {@code rbd} argv that would be handed to + * {@link Script} and the image options handed to {@link QemuImg}, so the command construction is + * verified without a live Ceph cluster (the actual execution needs a real cluster and is covered + * by end-to-end testing). */ @RunWith(MockitoJUnitRunner.class) public class RbdEncryptionTest { @@ -78,24 +87,60 @@ public void buildResizeScriptShrinkPassesAllowShrink() { } @Test - public void buildConvertScriptFromRbdSource() { - Script q = rbdEncryption.buildConvertScript("srcpool", "srcimg", null, null, - "cloudstack", "dst", "/tmp/conf", "cloudstack", "/tmp/pass", CryptSetup.LuksType.LUKS2); - String cmd = q.toString(); - Assert.assertTrue(cmd, cmd.contains("qemu-img convert -n")); - Assert.assertTrue(cmd, cmd.contains("--image-opts driver=rbd,pool=srcpool,image=srcimg,conf=/tmp/conf,user=cloudstack")); - Assert.assertTrue(cmd, cmd.contains("--object secret,id=luks0,file=/tmp/pass")); - Assert.assertTrue(cmd, cmd.contains("--target-image-opts driver=rbd,pool=cloudstack,image=dst,conf=/tmp/conf,user=cloudstack,encrypt.format=luks2,encrypt.key-secret=luks0")); + public void importTemplateFromRbdSourceConvertsThroughQemuImg() throws Exception { + RbdEncryption spy = Mockito.spy(new RbdEncryption()); + QemuImg qemuImg = Mockito.mock(QemuImg.class); + Mockito.doReturn(qemuImg).when(spy).createQemuImg(); + + spy.importTemplate("srcpool", "srcimg", null, null, "1.2.3.4", 6789, "cloudstack", "secret", + "cloudstack", "dst", "passphrase".getBytes(StandardCharsets.UTF_8), CryptSetup.LuksType.LUKS2); + + ArgumentCaptor srcOpts = ArgumentCaptor.forClass(QemuImageOptions.class); + ArgumentCaptor destOpts = ArgumentCaptor.forClass(QemuImageOptions.class); + ArgumentCaptor> objects = ArgumentCaptor.forClass(List.class); + Mockito.verify(qemuImg).convertIntoExistingTarget(Mockito.any(QemuImgFile.class), Mockito.isNull(), + objects.capture(), 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("driver=rbd")); + Assert.assertTrue(src, src.contains("pool=srcpool")); + Assert.assertTrue(src, src.contains("image=srcimg")); + Assert.assertTrue(src, src.contains("user=cloudstack")); + Assert.assertTrue(src, src.contains("conf=")); + + 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("driver=rbd")); + Assert.assertTrue(dest, dest.contains("pool=cloudstack")); + Assert.assertTrue(dest, dest.contains("image=dst")); + Assert.assertTrue(dest, dest.contains("encrypt.format=luks2")); + Assert.assertTrue(dest, dest.contains("encrypt.key-secret=luks0")); + + String secretObjects = objects.getValue().stream() + .map(o -> String.join(" ", o.toCommandFlag())).collect(Collectors.joining(" ")); + Assert.assertTrue(secretObjects, secretObjects.contains("--object secret,")); + Assert.assertTrue(secretObjects, secretObjects.contains("id=luks0")); + Assert.assertTrue(secretObjects, secretObjects.contains("file=")); } @Test - public void buildConvertScriptFromFileSource() { - Script q = rbdEncryption.buildConvertScript(null, null, "/tmp/tmpl.qcow2", "QCOW2", - "cloudstack", "dst", "/tmp/conf", "cloudstack", "/tmp/pass", CryptSetup.LuksType.LUKS2); - String cmd = q.toString(); - Assert.assertTrue(cmd, cmd.contains("-f qcow2 /tmp/tmpl.qcow2")); - Assert.assertFalse(cmd, cmd.contains("--image-opts")); - Assert.assertTrue(cmd, cmd.contains("encrypt.format=luks2,encrypt.key-secret=luks0")); + public void importTemplateFromFileSourceForcesSourceFormat() throws Exception { + RbdEncryption spy = Mockito.spy(new RbdEncryption()); + QemuImg qemuImg = Mockito.mock(QemuImg.class); + Mockito.doReturn(qemuImg).when(spy).createQemuImg(); + + spy.importTemplate(null, null, "/tmp/tmpl.qcow2", "QCOW2", "1.2.3.4", 6789, "cloudstack", "secret", + "cloudstack", "dst", "passphrase".getBytes(StandardCharsets.UTF_8), CryptSetup.LuksType.LUKS2); + + ArgumentCaptor srcFile = ArgumentCaptor.forClass(QemuImgFile.class); + ArgumentCaptor srcOpts = ArgumentCaptor.forClass(QemuImageOptions.class); + Mockito.verify(qemuImg).convertIntoExistingTarget(srcFile.capture(), Mockito.isNull(), + Mockito.anyList(), srcOpts.capture(), Mockito.any(QemuImageOptions.class), Mockito.eq(true)); + + Assert.assertEquals(QemuImg.PhysicalDiskFormat.QCOW2, srcFile.getValue().getFormat()); + String src = String.join(" ", srcOpts.getValue().toCommandFlag()); + Assert.assertTrue(src, src.contains("file.filename=/tmp/tmpl.qcow2")); } @Test From b5f780b725efed7c2c9abbabc7110483d0ebfe09 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:27:58 +0200 Subject: [PATCH 12/41] kvm: use readable variable names in the encrypted RBD root helpers Review feedback: single-letter and abbreviated names are hard to read. Renamed in the two methods added by this PR only (renaming the rest of the class is out of scope here): r -> radosConnection, io -> ioContext, rbd -> rbdClient, base -> templateImage, s -> snapshotInfo, encSnap -> luksReservedSnapshotName, haveEncSnap -> luksSnapshotExists, createSize -> imageSizeWithLuksHeader, srcIsRbd -> sourceIsRbdPool. No functional change. --- .../kvm/storage/LibvirtStorageAdaptor.java | 86 +++++++++---------- 1 file changed, 43 insertions(+), 43 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 86c7fd0c58bd..4517ddc3c205 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,47 +1535,47 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, */ private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KVMStoragePool destPool, String newUuid, KVMPhysicalDisk disk, byte[] passphrase) { - String encSnap = rbdTemplateSnapName + "-luks"; - Rados r = null; - IoCTX io = null; - Rbd rbd = null; - RbdImage base = null; + String luksReservedSnapshotName = rbdTemplateSnapName + "-luks"; + Rados radosConnection = null; + IoCTX ioContext = null; + Rbd rbdClient = null; + RbdImage templateImage = null; try { - r = new Rados(destPool.getAuthUserName()); - r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); - r.confSet("key", destPool.getAuthSecret()); - r.confSet("client_mount_timeout", "30"); - r.connect(); - io = r.ioCtxCreate(destPool.getSourceDir()); - rbd = new Rbd(io); - base = rbd.open(template.getName()); - boolean haveEncSnap = false; - for (RbdSnapInfo s : base.snapList()) { - if (encSnap.equals(s.name)) { - haveEncSnap = true; + radosConnection = new Rados(destPool.getAuthUserName()); + radosConnection.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + radosConnection.confSet("key", destPool.getAuthSecret()); + radosConnection.confSet("client_mount_timeout", "30"); + radosConnection.connect(); + ioContext = radosConnection.ioCtxCreate(destPool.getSourceDir()); + rbdClient = new Rbd(ioContext); + templateImage = rbdClient.open(template.getName()); + boolean luksSnapshotExists = false; + for (RbdSnapInfo snapshotInfo : templateImage.snapList()) { + if (luksReservedSnapshotName.equals(snapshotInfo.name)) { + luksSnapshotExists = true; break; } } - if (!haveEncSnap) { - base.resize(template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES); - base.snapCreate(encSnap); - base.snapProtect(encSnap); - logger.debug("Prepared LUKS-reserved template snapshot {}@{}", template.getName(), encSnap); + if (!luksSnapshotExists) { + templateImage.resize(template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES); + templateImage.snapCreate(luksReservedSnapshotName); + templateImage.snapProtect(luksReservedSnapshotName); + logger.debug("Prepared LUKS-reserved template snapshot {}@{}", template.getName(), luksReservedSnapshotName); } - rbd.clone(template.getName(), encSnap, io, newUuid, RBD_FEATURES, rbdOrder); + rbdClient.clone(template.getName(), luksReservedSnapshotName, ioContext, newUuid, RBD_FEATURES, rbdOrder); } catch (RadosException | RbdException e) { logger.error("Failed to create encrypted CoW clone {}: {}", newUuid, e.getMessage()); return null; } finally { - if (rbd != null && base != null) { + if (rbdClient != null && templateImage != null) { try { - rbd.close(base); + rbdClient.close(templateImage); } catch (RbdException ignored) { // best-effort close of the template handle } } - if (r != null && io != null) { - r.ioCtxDestroy(io); + if (radosConnection != null && ioContext != null) { + radosConnection.ioCtxDestroy(ioContext); } } formatRbdImageEncryption(destPool, newUuid, passphrase); @@ -1598,31 +1598,31 @@ private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KV */ private KVMPhysicalDisk createEncryptedRootFullCopy(KVMStoragePool srcPool, KVMPhysicalDisk template, KVMStoragePool destPool, String newUuid, KVMPhysicalDisk disk, byte[] passphrase) { - long createSize = disk.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES; - Rados r = null; - IoCTX io = null; + long imageSizeWithLuksHeader = disk.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES; + Rados radosConnection = null; + IoCTX ioContext = null; try { - r = new Rados(destPool.getAuthUserName()); - r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); - r.confSet("key", destPool.getAuthSecret()); - r.confSet("client_mount_timeout", "30"); - r.connect(); - io = r.ioCtxCreate(destPool.getSourceDir()); - Rbd rbd = new Rbd(io); - rbd.create(newUuid, createSize, RBD_FEATURES, rbdOrder); + radosConnection = new Rados(destPool.getAuthUserName()); + radosConnection.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + radosConnection.confSet("key", destPool.getAuthSecret()); + radosConnection.confSet("client_mount_timeout", "30"); + radosConnection.connect(); + ioContext = radosConnection.ioCtxCreate(destPool.getSourceDir()); + Rbd rbdClient = new Rbd(ioContext); + rbdClient.create(newUuid, imageSizeWithLuksHeader, RBD_FEATURES, rbdOrder); } catch (RadosException | RbdException e) { logger.error("Failed to create encrypted RBD image {}: {}", newUuid, e.getMessage()); return null; } finally { - if (r != null && io != null) { - r.ioCtxDestroy(io); + if (radosConnection != null && ioContext != null) { + radosConnection.ioCtxDestroy(ioContext); } } formatRbdImageEncryption(destPool, newUuid, passphrase); - boolean srcIsRbd = srcPool.getType() == StoragePoolType.RBD; + boolean sourceIsRbdPool = srcPool.getType() == StoragePoolType.RBD; new RbdEncryption().importTemplate( - srcIsRbd ? srcPool.getSourceDir() : null, srcIsRbd ? template.getName() : null, - srcIsRbd ? null : template.getPath(), srcIsRbd ? null : template.getFormat().toString(), + sourceIsRbdPool ? srcPool.getSourceDir() : null, sourceIsRbdPool ? template.getName() : null, + sourceIsRbdPool ? null : template.getPath(), sourceIsRbdPool ? null : template.getFormat().toString(), destPool.getSourceHost(), destPool.getSourcePort(), destPool.getAuthUserName(), destPool.getAuthSecret(), destPool.getSourceDir(), newUuid, passphrase, CryptSetup.LuksType.LUKS2); disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); From 702ff0d13be5d353068ad6e0dfcf3c72a20e2821 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:28:06 +0200 Subject: [PATCH 13/41] server, kvm: report and require RBD volume encryption separately Review feedback: distinguish the two volume encryption mechanisms instead of advertising them under one host flag. host.volume.encryption goes back to meaning qemu-native LUKS only (qemu-img LUKS + cryptsetup), as it did before this PR, and hosts now additionally report host.volume.encryption.rbd for librbd encryption (rbd encryption format). The deployment planner requires the flag matching the pool type of each encrypted volume - librbd for volumes on RBD pools, qemu-native for any other pool type - at all three places it validated encryption support before. The pool is taken from the pools proposed alongside the host when present, so first deployments are matched accurately too; an encrypted volume with no pool yet accepts either mechanism and the storage pool allocator picks a pool the host can serve. This also stops a host whose librbd is too old for 'rbd encryption format' from being selected for encrypted RBD volumes; it previously advertised encryption through the qemu stack and the VM failed to start. --- api/src/main/java/com/cloud/host/Host.java | 1 + .../resource/LibvirtComputingResource.java | 25 ++----- .../deploy/DeploymentPlanningManagerImpl.java | 59 +++++++++++++-- .../DeploymentPlanningManagerImplTest.java | 73 +++++++++++++++++++ 4 files changed, 131 insertions(+), 27 deletions(-) diff --git a/api/src/main/java/com/cloud/host/Host.java b/api/src/main/java/com/cloud/host/Host.java index c110e4ca94e1..cca2edd70c6b 100644 --- a/api/src/main/java/com/cloud/host/Host.java +++ b/api/src/main/java/com/cloud/host/Host.java @@ -56,6 +56,7 @@ public static String[] toStrings(Host.Type... types) { String HOST_UEFI_ENABLE = "host.uefi.enable"; String HOST_VOLUME_ENCRYPTION = "host.volume.encryption"; + String HOST_RBD_VOLUME_ENCRYPTION = "host.volume.encryption.rbd"; String HOST_INSTANCE_CONVERSION = "host.instance.conversion"; String HOST_VDDK_SUPPORT = "host.vddk.support"; String HOST_VDDK_LIB_DIR = "vddk.lib.dir"; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 20677c1c08a3..9946c85cfd9d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -19,6 +19,7 @@ import static com.cloud.host.Host.HOST_CDROM_MAX_COUNT; import static com.cloud.host.Host.HOST_INSTANCE_CONVERSION; import static com.cloud.host.Host.HOST_OVFTOOL_VERSION; +import static com.cloud.host.Host.HOST_RBD_VOLUME_ENCRYPTION; import static com.cloud.host.Host.HOST_VDDK_LIB_DIR; import static com.cloud.host.Host.HOST_VDDK_SUPPORT; import static com.cloud.host.Host.HOST_VDDK_VERSION; @@ -4413,6 +4414,7 @@ public StartupCommand[] initialize() { cmd.setGatewayIpAddress(localGateway); cmd.setIqn(getIqn()); cmd.getHostDetails().put(HOST_VOLUME_ENCRYPTION, String.valueOf(hostSupportsVolumeEncryption())); + cmd.getHostDetails().put(HOST_RBD_VOLUME_ENCRYPTION, String.valueOf(hostSupportsRbdVolumeEncryption())); cmd.setHostTags(getHostTags()); boolean instanceConversionSupported = hostSupportsInstanceConversion(); cmd.getHostDetails().put(HOST_INSTANCE_CONVERSION, String.valueOf(instanceConversionSupported)); @@ -6198,28 +6200,13 @@ public boolean isHostSecured() { } /** - * Test host for volume encryption support. A host is considered encryption-capable if it - * supports EITHER mechanism CloudStack can use: - * - qemu-native LUKS (qemu-img LUKS + cryptsetup) for file/block backed pools, or - * - librbd native encryption (rbd encryption format) for RBD/Ceph pools. - * NOTE: HOST_VOLUME_ENCRYPTION is a single host-wide flag and is not per-pool, so a host that - * advertises encryption via only one mechanism could still be selected for a volume that needs - * the other. In practice hosts that do encryption have the qemu-native stack; the librbd branch - * additionally covers Ceph-only hosts. + * Test host for qemu-native LUKS volume encryption (qemu-img LUKS support + cryptsetup), + * reported as {@code host.volume.encryption}. RBD/librbd encryption support is a separate + * capability, reported as {@code host.volume.encryption.rbd} + * (see {@link #hostSupportsRbdVolumeEncryption()}). * @return boolean */ public boolean hostSupportsVolumeEncryption() { - boolean supported = hostSupportsQemuNativeVolumeEncryption() || hostSupportsRbdVolumeEncryption(); - if (!supported) { - LOGGER.info("Host does not support volume encryption (no qemu-native LUKS + cryptsetup, and no librbd rbd encryption)"); - } - return supported; - } - - /** - * Test host for qemu-native LUKS volume encryption (qemu-img LUKS support + cryptsetup). - */ - public boolean hostSupportsQemuNativeVolumeEncryption() { // test qemu-img try { QemuImg qemu = new QemuImg(0); diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java index 32e904c8de38..4c062b1eaff1 100644 --- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; @@ -111,6 +112,7 @@ import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOSVO; import com.cloud.storage.ScopeType; +import com.cloud.storage.Storage; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; @@ -428,7 +430,7 @@ public DeployDestination planDeployment(VirtualMachineProfile vmProfile, Deploym long hostId = dest.getHost().getId(); avoids.addHost(dest.getHost().getId()); - if (volumesRequireEncryption && !Boolean.parseBoolean(_hostDetailsDao.findDetail(hostId, Host.HOST_VOLUME_ENCRYPTION).getValue())) { + if (volumesRequireEncryption && !hostMeetsVolumeEncryptionRequirements(hostId, _volsDao.findByInstance(vm.getId()), null)) { logger.warn("VM's volumes require encryption support, and the planner-provided host {} can't handle it", dest.getHost()); continue; } else { @@ -599,8 +601,8 @@ private boolean canUseLastHost(HostVO host, ExcludeList avoids, DeploymentPlan p return false; } - if (volumesRequireEncryption && !Boolean.parseBoolean(host.getDetail(Host.HOST_VOLUME_ENCRYPTION))) { - logger.warn("The last host of this VM {} does not support volume encryption, which is required by this VM.", host); + if (volumesRequireEncryption && !hostMeetsVolumeEncryptionRequirements(host.getId(), _volsDao.findByInstance(vm.getId()), null)) { + logger.warn("The last host of this VM {} does not support the volume encryption required by this VM.", host); return false; } return true; @@ -689,6 +691,51 @@ protected boolean anyVolumeRequiresEncryption(List volumes) { return false; } + private boolean hostHasEncryptionDetail(long hostId, String detailName) { + DetailVO detail = _hostDetailsDao.findDetail(hostId, detailName); + return detail != null && Boolean.parseBoolean(detail.getValue()); + } + + /** + * Checks that the host advertises the encryption mechanism each encrypted volume needs: + * {@link Host#HOST_RBD_VOLUME_ENCRYPTION} (librbd) for volumes on RBD pools, + * {@link Host#HOST_VOLUME_ENCRYPTION} (qemu-native LUKS) for volumes on any other pool type. + * The pool of a volume is taken from {@code proposedPools} when present (pools being allocated + * along with the host), falling back to the volume's persisted pool. For an encrypted volume + * with no pool yet (initial deployment without a proposed pool), either mechanism is accepted; + * the storage pool allocator selects a pool the host can serve. + */ + protected boolean hostMeetsVolumeEncryptionRequirements(long hostId, Collection volumes, Map proposedPools) { + for (Volume volume : volumes) { + if (volume.getPassphraseId() == null && volume.getKmsKeyId() == null) { + continue; + } + Storage.StoragePoolType poolType = null; + StoragePool proposedPool = proposedPools != null ? proposedPools.get(volume) : null; + if (proposedPool != null) { + poolType = proposedPool.getPoolType(); + } else if (volume.getPoolId() != null) { + StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId()); + poolType = pool != null ? pool.getPoolType() : null; + } + boolean hostMeets; + if (poolType == Storage.StoragePoolType.RBD) { + hostMeets = hostHasEncryptionDetail(hostId, Host.HOST_RBD_VOLUME_ENCRYPTION); + } else if (poolType != null) { + hostMeets = hostHasEncryptionDetail(hostId, Host.HOST_VOLUME_ENCRYPTION); + } else { + hostMeets = hostHasEncryptionDetail(hostId, Host.HOST_VOLUME_ENCRYPTION) + || hostHasEncryptionDetail(hostId, Host.HOST_RBD_VOLUME_ENCRYPTION); + } + if (!hostMeets) { + logger.debug("Host [{}] does not support the encryption mechanism required by volume [{}] (pool type [{}])", + hostId, volume, poolType); + return false; + } + } + return true; + } + private boolean isDeployAsIs(VirtualMachine vm) { long templateId = vm.getTemplateId(); VMTemplateVO template = templateDao.findById(templateId); @@ -1648,11 +1695,7 @@ public int compare(Volume v1, Volume v2) { } } - HostVO potentialHostVO = _hostDao.findById(potentialHost.getId()); - _hostDao.loadDetails(potentialHostVO); - - boolean hostHasEncryption = Boolean.parseBoolean(potentialHostVO.getDetail(Host.HOST_VOLUME_ENCRYPTION)); - boolean hostMeetsEncryptionRequirements = !anyVolumeRequiresEncryption(new ArrayList<>(volumesOrderBySizeDesc)) || hostHasEncryption; + boolean hostMeetsEncryptionRequirements = hostMeetsVolumeEncryptionRequirements(potentialHost.getId(), volumesOrderBySizeDesc, storage); boolean hostFitsPlannerUsage = checkIfHostFitsPlannerUsage(potentialHost, resourceUsageRequired); if (hostCanAccessPool && haveEnoughSpace && hostAffinityCheck && hostMeetsEncryptionRequirements && hostFitsPlannerUsage) { diff --git a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java index 5b03260d2d66..1c803e936fdb 100644 --- a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java +++ b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java @@ -40,6 +40,7 @@ import com.cloud.gpu.GPU; import com.cloud.gpu.dao.HostGpuGroupsDao; import com.cloud.gpu.dao.VgpuProfileDao; +import com.cloud.host.DetailVO; import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.Status; @@ -197,6 +198,9 @@ public class DeploymentPlanningManagerImplTest { @Inject HostDao hostDao; + @Inject + HostDetailsDao hostDetailsDao; + @Inject CapacityManager capacityMgr; @@ -548,6 +552,69 @@ public void volumesDoNotRequireEncryptionTest() { Assert.assertFalse("Volumes do not require encryption, but reporting they do", _dpm.anyVolumeRequiresEncryption(volumes)); } + private void stubHostEncryptionDetails(long hostId, boolean qemuNative, boolean rbd) { + Mockito.when(hostDetailsDao.findDetail(hostId, Host.HOST_VOLUME_ENCRYPTION)) + .thenReturn(new DetailVO(hostId, Host.HOST_VOLUME_ENCRYPTION, String.valueOf(qemuNative))); + Mockito.when(hostDetailsDao.findDetail(hostId, Host.HOST_RBD_VOLUME_ENCRYPTION)) + .thenReturn(new DetailVO(hostId, Host.HOST_RBD_VOLUME_ENCRYPTION, String.valueOf(rbd))); + } + + private VolumeVO encryptedVolumeOnPool(Long poolId, Storage.StoragePoolType poolType) { + VolumeVO volume = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); + volume.setPassphraseId(1L); + if (poolId != null) { + volume.setPoolId(poolId); + StoragePoolVO pool = new StoragePoolVO(); + pool.setPoolType(poolType); + Mockito.when(primaryDataStoreDao.findById(poolId)).thenReturn(pool); + } + return volume; + } + + @Test + public void hostMeetsVolumeEncryptionRequirementsRbdPoolNeedsRbdSupportTest() { + VolumeVO volume = encryptedVolumeOnPool(77L, Storage.StoragePoolType.RBD); + stubHostEncryptionDetails(5L, true, false); // qemu-native only + stubHostEncryptionDetails(6L, false, true); // librbd only + Assert.assertFalse("qemu-native-only host must not pass for an encrypted volume on an RBD pool", + _dpm.hostMeetsVolumeEncryptionRequirements(5L, List.of(volume), null)); + Assert.assertTrue("librbd-capable host must pass for an encrypted volume on an RBD pool", + _dpm.hostMeetsVolumeEncryptionRequirements(6L, List.of(volume), null)); + } + + @Test + public void hostMeetsVolumeEncryptionRequirementsNonRbdPoolNeedsQemuSupportTest() { + VolumeVO volume = encryptedVolumeOnPool(78L, Storage.StoragePoolType.NetworkFilesystem); + stubHostEncryptionDetails(5L, true, false); // qemu-native only + stubHostEncryptionDetails(6L, false, true); // librbd only + Assert.assertTrue("qemu-native host must pass for an encrypted volume on a non-RBD pool", + _dpm.hostMeetsVolumeEncryptionRequirements(5L, List.of(volume), null)); + Assert.assertFalse("librbd-only host must not pass for an encrypted volume on a non-RBD pool", + _dpm.hostMeetsVolumeEncryptionRequirements(6L, List.of(volume), null)); + } + + @Test + public void hostMeetsVolumeEncryptionRequirementsNoPoolAcceptsEitherMechanismTest() { + VolumeVO volume = encryptedVolumeOnPool(null, null); + stubHostEncryptionDetails(5L, false, true); // librbd only + stubHostEncryptionDetails(6L, false, false); // no encryption at all + Assert.assertTrue("a volume with no pool yet accepts any encryption mechanism", + _dpm.hostMeetsVolumeEncryptionRequirements(5L, List.of(volume), null)); + Assert.assertFalse("a host with no encryption support must not pass for an encrypted volume", + _dpm.hostMeetsVolumeEncryptionRequirements(6L, List.of(volume), null)); + } + + @Test + public void hostMeetsVolumeEncryptionRequirementsUsesProposedPoolTest() { + VolumeVO volume = encryptedVolumeOnPool(null, null); + StoragePoolVO proposedRbdPool = new StoragePoolVO(); + proposedRbdPool.setPoolType(Storage.StoragePoolType.RBD); + Map proposedPools = Map.of(volume, proposedRbdPool); + stubHostEncryptionDetails(5L, true, false); // qemu-native only + Assert.assertFalse("the proposed pool's type must drive the required encryption mechanism", + _dpm.hostMeetsVolumeEncryptionRequirements(5L, List.of(volume), proposedPools)); + } + /** * Root requires encryption, chosen host supports it */ @@ -863,6 +930,12 @@ private DeploymentClusterPlanner setupMocksForPlanDeploymentHostTests(HostVO hos Mockito.when(vmProfile.getHypervisorType()).thenReturn(HypervisorType.KVM); Mockito.when(hostDao.findById(hostId)).thenReturn(host); Mockito.doNothing().when(hostDao).loadDetails(host); + // encryption capability checks read host_details through the dao; answer them from the test host's detail map + Mockito.when(hostDetailsDao.findDetail(ArgumentMatchers.anyLong(), ArgumentMatchers.anyString())).thenAnswer(invocation -> { + String detailName = invocation.getArgument(1); + String detailValue = host.getDetails() != null ? host.getDetails().get(detailName) : null; + return detailValue != null ? new DetailVO(host.getId(), detailName, detailValue) : null; + }); Mockito.doReturn(volumeVOs).when(volDao).findByInstance(ArgumentMatchers.anyLong()); Mockito.doReturn(suitable).when(_dpm).findSuitablePoolsForVolumes( ArgumentMatchers.any(VirtualMachineProfile.class), From 56ea831f945c9b6979244996f1795df2f3de9ad8 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Mon, 10 Aug 2026 07:41:33 +0200 Subject: [PATCH 14/41] kvm: fix RBD exclusive-lock leak that breaks revertSnapshot on Ceph takeRbdVolumeSnapshotOfStoppedVm() called image.snapCreate(snapshotName) twice. The first call creates the RBD snapshot, the second one always throws RbdException ("Failed to create snapshot ") because the snapshot already exists. The duplicate is a merge artifact: 30d306622a9 ("Merge branch '4.20' into 4.22") resolved a conflict by keeping the call from both sides - each parent had exactly one. Because there was no finally block, that exception skipped rbd.close(image) and r.ioCtxDestroy(io), so the agent kept the image open and held its RBD exclusive-lock indefinitely. The exception is only logged, so the snapshot job still reported success and the fault stayed invisible. Consequences observed on a KVM + Ceph/RBD cluster: - revertSnapshot fails with "com.ceph.rbd.RbdException: Failed to rollback snapshot ". librbd returns EROFS because a live peer holds the exclusive-lock; 'rbd snap rollback' only succeeds once that client dies and librbd can break the lock, which makes the failure look intermittent. - getRbdSnapshotSize() is never reached, so every snapshot is reported with physical size 0 when snapshot.backup.to.secondary is false. - The leaked watchers keep the image busy, so 'rbd rm' fails and the volume cannot be expunged - it stays stuck in state Destroy. Note the method also runs for RUNNING VMs: createSnapshot() branches on "RUNNING && !primaryPool.isExternalSnapshot()", and RBD is an external-snapshot pool, so every RBD volume snapshot took this path. Remove the duplicated call and move the image/IO-context cleanup into a finally block so the lock is released even if the snapshot itself fails. --- .../kvm/storage/KVMStorageProcessor.java | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 3466ebe389c3..8ae62917985f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -2595,26 +2595,44 @@ private CreateObjectAnswer takeClvmVolumeSnapshotOfStoppedVm(KVMPhysicalDisk dis */ private Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMPhysicalDisk disk, String snapshotName) { Long snapshotSize = null; + Rados r = null; + IoCTX io = null; + Rbd rbd = null; + RbdImage image = null; try { - Rados r = radosConnect(primaryPool); + r = radosConnect(primaryPool); - final IoCTX io = r.ioCtxCreate(primaryPool.getSourceDir()); - final Rbd rbd = new Rbd(io); - final RbdImage image = rbd.open(disk.getName()); + io = r.ioCtxCreate(primaryPool.getSourceDir()); + rbd = new Rbd(io); + image = rbd.open(disk.getName()); logger.debug("Attempting to create RBD snapshot {}@{}", disk.getName(), snapshotName); image.snapCreate(snapshotName); - image.snapCreate(snapshotName); long rbdSnapshotSize = getRbdSnapshotSize(primaryPool.getSourceDir(), disk.getName(), snapshotName, primaryPool.getSourceHost(), primaryPool.getAuthUserName(), primaryPool.getAuthSecret()); if (rbdSnapshotSize > 0) { snapshotSize = rbdSnapshotSize; } - - rbd.close(image); - r.ioCtxDestroy(io); } catch (final Exception e) { logger.error("A RBD snapshot operation on [{}] failed. The error was: {}", disk.getName(), e.getMessage(), e); + } finally { + // The image MUST be closed on every path. While it stays open this client holds the RBD + // exclusive-lock, and a later 'rbd snap rollback' (revertSnapshot) issued from any other host + // cannot take a live peer's lock - librbd then fails it with EROFS. + if (image != null) { + try { + rbd.close(image); + } catch (final Exception e) { + logger.warn("Failed to close RBD image [{}] after a snapshot operation. The error was: {}", disk.getName(), e.getMessage(), e); + } + } + if (io != null) { + try { + r.ioCtxDestroy(io); + } catch (final Exception e) { + logger.warn("Failed to destroy the RADOS IO context used to snapshot [{}]. The error was: {}", disk.getName(), e.getMessage(), e); + } + } } return snapshotSize; } From 064286aad8491b43f46ff1846e6be5f4b116a684 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Mon, 10 Aug 2026 07:41:33 +0200 Subject: [PATCH 15/41] kvm: release RBD handles on every path when cloning a volume from a snapshot createRBDvolumeFromRBDSnapshot() closed the source image, the cloned image and the RADOS IO context only on the success path, and called snapUnprotect() only there too. Two paths escaped that cleanup: - the early "Could not find snapshot ... on RBD" return, and - any RadosException/RbdException from clone(), resize() or flatten(), which is caught and turned into a null disk. Both leave the images open, so this client keeps the RBD exclusive-lock. That later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS from another host, and keeps the image busy so 'rbd rm' cannot remove it - the volume then stays stuck in state Destroy. The failure paths after snapProtect() are worse: the snapshot stays protected, and a protected snapshot can be deleted neither on its own nor together with its volume. Move the cleanup into a finally block, tracking whether the snapshot was actually protected so it is unprotected exactly when it needs to be. Failures during cleanup are logged and never mask the original outcome; a failed snapUnprotect is logged at ERROR since it needs manual intervention. This is the same class of defect as the leak fixed in takeRbdVolumeSnapshotOfStoppedVm(); no behaviour changes on the success path. --- .../kvm/storage/KVMStorageProcessor.java | 58 +++++++++++++++---- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 8ae62917985f..9d96fa2a7021 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -3089,17 +3089,24 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S disk.setSize(size > volume.getVirtualSize() ? size : volume.getVirtualSize()); disk.setVirtualSize(size > volume.getVirtualSize() ? size : disk.getSize()); + Rados r = null; + IoCTX io = null; + Rbd rbd = null; + RbdImage srcImage = null; + RbdImage diskImage = null; + boolean snapProtected = false; + try { - Rados r = new Rados(srcPool.getAuthUserName()); + r = new Rados(srcPool.getAuthUserName()); r.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort()); r.confSet("key", srcPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); r.connect(); - IoCTX io = r.ioCtxCreate(srcPool.getSourceDir()); - Rbd rbd = new Rbd(io); - RbdImage srcImage = rbd.open(volume.getName()); + io = r.ioCtxCreate(srcPool.getSourceDir()); + rbd = new Rbd(io); + srcImage = rbd.open(volume.getName()); List snaps = srcImage.snapList(); boolean snapFound = false; @@ -3115,23 +3122,54 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S return null; } srcImage.snapProtect(snapshotName); + snapProtected = true; logger.debug(String.format("Try to clone snapshot %s on RBD", snapshotName)); rbd.clone(volume.getName(), snapshotName, io, disk.getName(), LibvirtStorageAdaptor.RBD_FEATURES, 0); - RbdImage diskImage = rbd.open(disk.getName()); + diskImage = rbd.open(disk.getName()); if (disk.getVirtualSize() > volume.getVirtualSize()) { diskImage.resize(disk.getVirtualSize()); } diskImage.flatten(); - rbd.close(diskImage); - - srcImage.snapUnprotect(snapshotName); - rbd.close(srcImage); - r.ioCtxDestroy(io); } catch (RadosException | RbdException e) { logger.error(String.format("Failed due to %s", e.getMessage()), e); disk = null; + } finally { + // Every handle has to be released on all paths, including the "snapshot not found" return and + // any failure of clone/resize/flatten. An image left open keeps this client's RBD + // exclusive-lock, which later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS and + // keeps the image busy so it cannot be removed. + if (diskImage != null) { + try { + rbd.close(diskImage); + } catch (final Exception e) { + logger.warn(String.format("Failed to close the cloned RBD image %s. The error was: %s", newUuid, e.getMessage()), e); + } + } + // A snapshot left protected cannot be deleted, and neither can its volume. + if (snapProtected) { + try { + srcImage.snapUnprotect(snapshotName); + } catch (final Exception e) { + logger.error(String.format("Failed to unprotect RBD snapshot %s; it and its volume cannot be deleted until this is " + + "resolved manually. The error was: %s", snapshotName, e.getMessage()), e); + } + } + if (srcImage != null) { + try { + rbd.close(srcImage); + } catch (final Exception e) { + logger.warn(String.format("Failed to close the source RBD image %s. The error was: %s", volume.getName(), e.getMessage()), e); + } + } + if (io != null) { + try { + r.ioCtxDestroy(io); + } catch (final Exception e) { + logger.warn(String.format("Failed to destroy the RADOS IO context used to clone %s. The error was: %s", snapshotName, e.getMessage()), e); + } + } } return disk; From bc9f998093f895bdda828f5b2ccbc2b11196f755 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Mon, 10 Aug 2026 07:41:33 +0200 Subject: [PATCH 16/41] kvm: add regression tests for the RBD snapshot handle leak Two tests around takeRbdVolumeSnapshotOfStoppedVm, using the MockedConstruction pattern already used in this test class (the Rbd instance is created inside the method under test, so it cannot be injected): - createsSnapshotExactlyOnce guards the duplicated snapCreate call from coming back, and checks the image and IO context are released. - releasesHandlesWhenSnapshotFails makes snapCreate throw and asserts the image is still closed and the IO context destroyed, so a future failure cannot leak the RBD exclusive-lock again. takeRbdVolumeSnapshotOfStoppedVm, radosConnect and getRbdSnapshotSize widened from private to protected so the test can stub the Ceph interactions. --- .../kvm/storage/KVMStorageProcessor.java | 6 +- .../kvm/storage/KVMStorageProcessorTest.java | 76 +++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 9d96fa2a7021..0cc7786f2567 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -2593,7 +2593,7 @@ private CreateObjectAnswer takeClvmVolumeSnapshotOfStoppedVm(KVMPhysicalDisk dis * barriers properly (>2.6.32) this won't be any different then pulling the power * cord out of a running machine. */ - private Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMPhysicalDisk disk, String snapshotName) { + protected Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMPhysicalDisk disk, String snapshotName) { Long snapshotSize = null; Rados r = null; IoCTX io = null; @@ -2637,7 +2637,7 @@ private Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMPhy return snapshotSize; } - private long getRbdSnapshotSize(String poolPath, String diskName, String snapshotName, String rbdMonitor, String authUser, String authSecret) { + protected long getRbdSnapshotSize(String poolPath, String diskName, String snapshotName, String rbdMonitor, String authUser, String authSecret) { logger.debug("Get RBD snapshot size for {}/{}@{}", poolPath, diskName, snapshotName); //cmd: rbd du /@ --format json --mon-host --id --key 2>/dev/null String snapshotDetailsInJson = Script.runSimpleBashScript(String.format("rbd du %s/%s@%s --format json --mon-host %s --id %s --key %s 2>/dev/null", poolPath, diskName, snapshotName, rbdMonitor, authUser, authSecret)); @@ -2924,7 +2924,7 @@ protected boolean isAvailablePoolSizeDividedByDiskSizeLesserThanMinRate(long ava return ((availablePoolSize * 1d) / (diskSize * 1d)) < MIN_RATE_BETWEEN_AVAILABLE_POOL_AND_DISK_SIZE_TO_TAKE_DISK_SNAPSHOT; } - private Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException { + protected Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException { Rados r = new Rados(primaryPool.getAuthUserName()); r.confSet(CEPH_MON_HOST, primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort()); r.confSet(CEPH_AUTH_KEY, primaryPool.getAuthSecret()); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java index b895aa65ff59..f0a98471ac92 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java @@ -18,6 +18,11 @@ */ package com.cloud.hypervisor.kvm.storage; +import com.ceph.rados.IoCTX; +import com.ceph.rados.Rados; +import com.ceph.rbd.Rbd; +import com.ceph.rbd.RbdException; +import com.ceph.rbd.RbdImage; import com.cloud.exception.InternalErrorException; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser; @@ -111,6 +116,11 @@ public class KVMStorageProcessorTest { private static final String directDownloadTemporaryPath = "/var/lib/libvirt/images/dd"; private static final long templateSize = 80000L; + private static final String RBD_POOL_NAME = "cloudstack"; + private static final String RBD_IMAGE_NAME = "b7a1f0a9-0f0e-4a1a-9a35-1c1a2e0f1b5e"; + private static final String SNAPSHOT_NAME = "8f1c1f0b-9d3e-4c2a-8a3d-6f0b2c9e1d47"; + private static final long SNAPSHOT_SIZE = 196624L; + private AutoCloseable closeable; @Before @@ -688,4 +698,70 @@ public void ensureLibvirtSupportsEncryptedRbdHotplugIgnoresNonRbd() { // version is not even consulted for them. storageProcessor.ensureLibvirtSupportsEncryptedRbdHotplug(StoragePoolType.NetworkFilesystem); } + /** + * Wires a mocked Ceph stack for {@link KVMStorageProcessor#takeRbdVolumeSnapshotOfStoppedVm} and returns the + * mocked disk. The Rbd instance is created inside the method under test, so it is mocked by construction. + */ + private KVMPhysicalDisk prepareRbdSnapshotMocks(Rados radosMock, IoCTX ioCtxMock) throws Exception { + KVMPhysicalDisk diskMock = Mockito.mock(KVMPhysicalDisk.class); + Mockito.lenient().doReturn(RBD_IMAGE_NAME).when(diskMock).getName(); + + Mockito.lenient().doReturn(RBD_POOL_NAME).when(kvmStoragePoolMock).getSourceDir(); + Mockito.lenient().doReturn("10.0.0.1").when(kvmStoragePoolMock).getSourceHost(); + Mockito.lenient().doReturn("cloudstack").when(kvmStoragePoolMock).getAuthUserName(); + Mockito.lenient().doReturn("secret").when(kvmStoragePoolMock).getAuthSecret(); + + Mockito.doReturn(radosMock).when(storageProcessorSpy).radosConnect(kvmStoragePoolMock); + Mockito.doReturn(ioCtxMock).when(radosMock).ioCtxCreate(RBD_POOL_NAME); + Mockito.lenient().doReturn(SNAPSHOT_SIZE).when(storageProcessorSpy).getRbdSnapshotSize(Mockito.anyString(), Mockito.anyString(), + Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()); + + return diskMock; + } + + /** + * A duplicated snapCreate call used to throw "snapshot already exists" on every single RBD snapshot, which then + * skipped the cleanup below and leaked the image's exclusive-lock. + */ + @Test + public void takeRbdVolumeSnapshotOfStoppedVmTestCreatesSnapshotExactlyOnce() throws Exception { + Rados radosMock = Mockito.mock(Rados.class); + IoCTX ioCtxMock = Mockito.mock(IoCTX.class); + RbdImage rbdImageMock = Mockito.mock(RbdImage.class); + KVMPhysicalDisk diskMock = prepareRbdSnapshotMocks(radosMock, ioCtxMock); + + try (MockedConstruction rbd = Mockito.mockConstruction(Rbd.class, ((mock, context) -> + Mockito.doReturn(rbdImageMock).when(mock).open(RBD_IMAGE_NAME)))) { + + Long result = storageProcessorSpy.takeRbdVolumeSnapshotOfStoppedVm(kvmStoragePoolMock, diskMock, SNAPSHOT_NAME); + + Assert.assertEquals(Long.valueOf(SNAPSHOT_SIZE), result); + Mockito.verify(rbdImageMock, Mockito.times(1)).snapCreate(SNAPSHOT_NAME); + Mockito.verify(rbd.constructed().get(0)).close(rbdImageMock); + Mockito.verify(radosMock).ioCtxDestroy(ioCtxMock); + } + } + + /** + * While the image stays open this client holds the RBD exclusive-lock, and a later 'rbd snap rollback' + * (revertSnapshot) from another host fails with EROFS. The handles must be released even when the snapshot fails. + */ + @Test + public void takeRbdVolumeSnapshotOfStoppedVmTestReleasesHandlesWhenSnapshotFails() throws Exception { + Rados radosMock = Mockito.mock(Rados.class); + IoCTX ioCtxMock = Mockito.mock(IoCTX.class); + RbdImage rbdImageMock = Mockito.mock(RbdImage.class); + KVMPhysicalDisk diskMock = prepareRbdSnapshotMocks(radosMock, ioCtxMock); + Mockito.doThrow(new RbdException("Failed to create snapshot")).when(rbdImageMock).snapCreate(SNAPSHOT_NAME); + + try (MockedConstruction rbd = Mockito.mockConstruction(Rbd.class, ((mock, context) -> + Mockito.doReturn(rbdImageMock).when(mock).open(RBD_IMAGE_NAME)))) { + + Long result = storageProcessorSpy.takeRbdVolumeSnapshotOfStoppedVm(kvmStoragePoolMock, diskMock, SNAPSHOT_NAME); + + Assert.assertNull(result); + Mockito.verify(rbd.constructed().get(0)).close(rbdImageMock); + Mockito.verify(radosMock).ioCtxDestroy(ioCtxMock); + } + } } From 3d8eb2a9da3d9c272da366d333673820e35d0484 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:58:09 +0200 Subject: [PATCH 17/41] kvm: extract RBD handle teardown into null-safe helper methods Address review feedback: the nested try-catch teardown blocks in the finally clauses of takeRbdVolumeSnapshotOfStoppedVm and createRBDvolumeFromRBDSnapshot are extracted into two reusable, null-safe helpers that log but never throw: - closeRbdImage (3 call sites) - destroyRadosIoCtx (2 call sites) No behavior change. --- .../kvm/storage/KVMStorageProcessor.java | 75 +++++++++---------- 1 file changed, 34 insertions(+), 41 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 0cc7786f2567..45c76fe0cd55 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -2616,23 +2616,8 @@ protected Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMP } catch (final Exception e) { logger.error("A RBD snapshot operation on [{}] failed. The error was: {}", disk.getName(), e.getMessage(), e); } finally { - // The image MUST be closed on every path. While it stays open this client holds the RBD - // exclusive-lock, and a later 'rbd snap rollback' (revertSnapshot) issued from any other host - // cannot take a live peer's lock - librbd then fails it with EROFS. - if (image != null) { - try { - rbd.close(image); - } catch (final Exception e) { - logger.warn("Failed to close RBD image [{}] after a snapshot operation. The error was: {}", disk.getName(), e.getMessage(), e); - } - } - if (io != null) { - try { - r.ioCtxDestroy(io); - } catch (final Exception e) { - logger.warn("Failed to destroy the RADOS IO context used to snapshot [{}]. The error was: {}", disk.getName(), e.getMessage(), e); - } - } + closeRbdImage(rbd, image, disk.getName()); + destroyRadosIoCtx(r, io, disk.getName()); } return snapshotSize; } @@ -2934,6 +2919,34 @@ protected Rados radosConnect(final KVMStoragePool primaryPool) throws RadosExcep return r; } + /** + * Closes an RBD image if it was opened; never throws. An image left open keeps this client's RBD + * exclusive-lock, which later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS and keeps + * the image busy so it cannot be removed. + */ + protected void closeRbdImage(Rbd rbd, RbdImage image, String imageName) { + if (image == null) { + return; + } + try { + rbd.close(image); + } catch (final Exception e) { + logger.warn("Failed to close RBD image [{}]. The error was: {}", imageName, e.getMessage(), e); + } + } + + /** Destroys a RADOS IO context if it was created; never throws. */ + protected void destroyRadosIoCtx(Rados r, IoCTX io, String contextDescription) { + if (io == null) { + return; + } + try { + r.ioCtxDestroy(io); + } catch (final Exception e) { + logger.warn("Failed to destroy the RADOS IO context used for [{}]. The error was: {}", contextDescription, e.getMessage(), e); + } + } + @Override public Answer deleteVolume(final DeleteCommand cmd) { final VolumeObjectTO vol = (VolumeObjectTO)cmd.getData(); @@ -3137,16 +3150,8 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S disk = null; } finally { // Every handle has to be released on all paths, including the "snapshot not found" return and - // any failure of clone/resize/flatten. An image left open keeps this client's RBD - // exclusive-lock, which later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS and - // keeps the image busy so it cannot be removed. - if (diskImage != null) { - try { - rbd.close(diskImage); - } catch (final Exception e) { - logger.warn(String.format("Failed to close the cloned RBD image %s. The error was: %s", newUuid, e.getMessage()), e); - } - } + // any failure of clone/resize/flatten. + closeRbdImage(rbd, diskImage, newUuid); // A snapshot left protected cannot be deleted, and neither can its volume. if (snapProtected) { try { @@ -3156,20 +3161,8 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S "resolved manually. The error was: %s", snapshotName, e.getMessage()), e); } } - if (srcImage != null) { - try { - rbd.close(srcImage); - } catch (final Exception e) { - logger.warn(String.format("Failed to close the source RBD image %s. The error was: %s", volume.getName(), e.getMessage()), e); - } - } - if (io != null) { - try { - r.ioCtxDestroy(io); - } catch (final Exception e) { - logger.warn(String.format("Failed to destroy the RADOS IO context used to clone %s. The error was: %s", snapshotName, e.getMessage()), e); - } - } + closeRbdImage(rbd, srcImage, volume.getName()); + destroyRadosIoCtx(r, io, snapshotName); } return disk; From fb8fc6a9aed48fb469f4d33c0259a0cb4115c267 Mon Sep 17 00:00:00 2001 From: calvix <7136358+calvix@users.noreply.github.com> Date: Tue, 11 Aug 2026 09:47:38 +0200 Subject: [PATCH 18/41] kvm: extract RBD snapshot unprotect into a helper method Follow-up to the teardown helpers: the snapUnprotect block in the finally clause of createRBDvolumeFromRBDSnapshot moves into a never-throwing unprotectRbdSnapshot helper, so the finally clause is now free of inline try-catch constructions entirely. No behavior change. --- .../kvm/storage/KVMStorageProcessor.java | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 45c76fe0cd55..b8a01b848343 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -2947,6 +2947,22 @@ protected void destroyRadosIoCtx(Rados r, IoCTX io, String contextDescription) { } } + /** + * Unprotects an RBD snapshot if it was protected; never throws. A snapshot left protected cannot + * be deleted, and neither can its volume. + */ + protected void unprotectRbdSnapshot(RbdImage image, String snapshotName, boolean snapProtected) { + if (!snapProtected) { + return; + } + try { + image.snapUnprotect(snapshotName); + } catch (final Exception e) { + logger.error("Failed to unprotect RBD snapshot [{}]; it and its volume cannot be deleted until this is resolved manually. The error was: {}", + snapshotName, e.getMessage(), e); + } + } + @Override public Answer deleteVolume(final DeleteCommand cmd) { final VolumeObjectTO vol = (VolumeObjectTO)cmd.getData(); @@ -3152,15 +3168,7 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S // Every handle has to be released on all paths, including the "snapshot not found" return and // any failure of clone/resize/flatten. closeRbdImage(rbd, diskImage, newUuid); - // A snapshot left protected cannot be deleted, and neither can its volume. - if (snapProtected) { - try { - srcImage.snapUnprotect(snapshotName); - } catch (final Exception e) { - logger.error(String.format("Failed to unprotect RBD snapshot %s; it and its volume cannot be deleted until this is " + - "resolved manually. The error was: %s", snapshotName, e.getMessage()), e); - } - } + unprotectRbdSnapshot(srcImage, snapshotName, snapProtected); closeRbdImage(rbd, srcImage, volume.getName()); destroyRadosIoCtx(r, io, snapshotName); } From 00f739097604dd4e9f78548ec5057437a014e5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Tue, 18 Aug 2026 22:40:56 +0200 Subject: [PATCH 19/41] kvm: restore backups into the volume they were taken from An in-place restore of a KVM instance with more than one data disk can write a backed up disk into a different volume than the one it came from. NASBackupProvider.restoreVMBackup pairs the backed up volumes with the instance's current volumes by position in two lists that are each sorted by device id, and LibvirtRestoreBackupCommandWrapper.restoreVolumesOfExistingVM consumes those lists index by index. The volume UUID is carried along but only used to build error messages. That pairing is only correct while the instance's device ids still match the ones recorded in the backup, and the restore itself breaks that: on completion KVMGuru.importVirtualMachineFromBackup re-attaches every data disk with getNextAvailableDeviceId(). The volumes are never detached first, so their current ids still count as in use and the helper cannot return the id a volume already holds - every data disk is shifted by one slot on each restore. The root disk is unaffected because it is always re-attached at device id 0. A second restore therefore maps the backup files onto the wrong volumes. With equally sized disks the contents are silently exchanged and the API reports success. With disks of different sizes the first volume is overwritten and the restore then fails converting an image into a smaller volume, leaving that volume half rewritten with no rollback. Three changes: - KVMGuru: re-attach data disks with the device id recorded in the backup, falling back to the next free id only when the backup has none. - LibvirtRestoreBackupCommandWrapper: select the target volume by UUID instead of by list position, and fail explicitly when a backed up volume is no longer attached to the instance. - NASBackupProvider: derive the UUID list and the backup file list from the same sorted collection so the two cannot drift apart. The create-instance-from-backup path (restoreVolumesOfDestroyedVMs) is left alone: it provisions new volumes with new UUIDs, so it has nothing to match on. --- .../cloudstack/backup/NASBackupProvider.java | 9 +++++-- .../LibvirtRestoreBackupCommandWrapper.java | 24 +++++++++++++++---- .../java/com/cloud/hypervisor/KVMGuru.java | 13 ++++++++-- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java index 8e6d33c4e668..be1abb72e8e1 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java @@ -689,9 +689,14 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup, boolean qui } private Pair restoreVMBackup(VirtualMachine vm, Backup backup) { - List backedVolumes = backup.getBackedUpVolumes(); - List backedVolumesUUIDs = backedVolumes.stream() + // Sort once and derive every per-volume list from that same ordering. The UUID list and the + // backup file list are consumed index-by-index on the agent side, so they have to agree; + // sorting only one of them leaves the two out of step whenever the stored order of the + // backed up volumes differs from their device id order. + List backedVolumes = backup.getBackedUpVolumes().stream() .sorted(Comparator.comparingLong(Backup.VolumeInfo::getDeviceId)) + .collect(Collectors.toList()); + List backedVolumesUUIDs = backedVolumes.stream() .map(Backup.VolumeInfo::getUuid) .collect(Collectors.toList()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java index 3839d7f6bdbd..521894a7b44c 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java @@ -25,8 +25,10 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; import org.apache.cloudstack.backup.BackupAnswer; @@ -131,11 +133,25 @@ private void restoreVolumesOfExistingVM(KVMStoragePoolManager storagePoolMgr, Li String backupPath, List backupFiles, String mountDirectory, int timeout) { String diskType = "root"; try { - for (int idx = 0; idx < restoreVolumePaths.size(); idx++) { - PrimaryDataStoreTO restoreVolumePool = restoreVolumePools.get(idx); - String restoreVolumePath = restoreVolumePaths.get(idx); - String backupFile = backupFiles.get(idx); + // Match each backed up volume to the instance's volume with the SAME UUID. Both lists + // arrive ordered by device id, but device ids are not stable across restores, so relying + // on the position within the list can write a backup into a different volume than the one + // it was taken from. + Map targetIndexByVolumeUuid = new HashMap<>(); + for (int i = 0; i < restoreVolumePaths.size(); i++) { + targetIndexByVolumeUuid.put(getVolumeUuidFromPath(restoreVolumePaths.get(i), restoreVolumePools.get(i)), i); + } + + for (int idx = 0; idx < backedVolumesUUIDs.size(); idx++) { String backupVolumeUuid = backedVolumesUUIDs.get(idx); + Integer targetIdx = targetIndexByVolumeUuid.get(backupVolumeUuid); + if (targetIdx == null) { + throw new CloudRuntimeException(String.format("Unable to restore backup: volume [%s] recorded in the backup" + + " is not attached to the instance any more.", backupVolumeUuid)); + } + PrimaryDataStoreTO restoreVolumePool = restoreVolumePools.get(targetIdx); + String restoreVolumePath = restoreVolumePaths.get(targetIdx); + String backupFile = backupFiles.get(idx); String fullPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); diskType = "datadisk"; diff --git a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java index 4aa67c5835f0..bdf524078417 100644 --- a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java +++ b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java @@ -381,9 +381,18 @@ public VirtualMachine importVirtualMachineFromBackup(long zoneId, long domainId, _volumeDao.update(volume.getId(), volume); _volumeDao.attachVolume(volume.getId(), vm.getId(), 0L); } else if (VMVolToRestore.getType() == Volume.Type.DATADISK) { - List vmVolumes = _volumeDao.findByInstance(vm.getId()); _volumeDao.update(volume.getId(), volume); - _volumeDao.attachVolume(volume.getId(), vm.getId(), getNextAvailableDeviceId(vmVolumes)); + // Reinstate the device id the volume had when the backup was taken. The volumes are + // not detached before this loop, so their current device ids still count as "in use" + // and getNextAvailableDeviceId() can never return the id a volume already holds. Using + // it here shifts every data disk by one slot on every restore, which then misaligns the + // device-id ordering that the restore itself uses to map backup files onto volumes. + Long deviceId = VMVolToRestore.getDeviceId(); + if (deviceId == null) { + // Backups taken before the device id was recorded in the backup metadata. + deviceId = getNextAvailableDeviceId(_volumeDao.findByInstance(vm.getId())); + } + _volumeDao.attachVolume(volume.getId(), vm.getId(), deviceId); } UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_ATTACH, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), volume.getDiskOfferingId(), volume.getTemplateId(), volume.getSize(), Volume.class.getName(), volume.getUuid(), vm.getId(), volume.isDisplay()); From 4b7aa66f3881a492f932c8f0115bdd90aca6f671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=A1clav=20Rozsyp=C3=A1lek?= Date: Tue, 18 Aug 2026 22:47:04 +0200 Subject: [PATCH 20/41] kvm: add regression tests for backup restore volume mapping LibvirtRestoreBackupCommandWrapperTest: restoring an instance whose volumes arrive in a different order than the backed up volumes must still write each backup into the volume it was taken from, and must fail explicitly when a backed up volume is no longer attached to the instance. KVMGuruTest: importing an instance from a backup must re-attach its data disks on the device ids recorded in the backup rather than allocating new ones. --- .../LibvirtRestoreBackupCommandWrapper.java | 2 - ...ibvirtRestoreBackupCommandWrapperTest.java | 116 ++++++++++++++++++ .../com/cloud/hypervisor/KVMGuruTest.java | 51 ++++++++ 3 files changed, 167 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java index 521894a7b44c..14b68e664006 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java @@ -23,8 +23,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Locale; diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java index fc2341632984..58e5fe020227 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java @@ -581,4 +581,120 @@ public void testExecuteWithMultipleVolumes() throws Exception { } } } + + /** + * An in-place restore renumbers the device ids of the data disks, so the instance's volumes and + * the backed up volumes do not necessarily arrive in the same order. The backup of a volume must + * still be written into that same volume, matched by UUID rather than by position in the list. + */ + @Test + public void testRestoreOfExistingVmMapsBackupsToVolumesByUuid() throws Exception { + when(command.getVmName()).thenReturn("test-vm"); + when(command.getBackupPath()).thenReturn("backup/path"); + when(command.getBackupRepoAddress()).thenReturn("192.168.1.100:/backup"); + when(command.getBackupRepoType()).thenReturn("nfs"); + when(command.getMountOptions()).thenReturn("rw"); + when(command.isVmExists()).thenReturn(true); + when(command.getDiskType()).thenReturn("root"); + PrimaryDataStoreTO pool = Mockito.mock(PrimaryDataStoreTO.class); + lenient().when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(command.getRestoreVolumePools()).thenReturn(Arrays.asList(pool, pool, pool)); + // the instance's volumes, ordered by their CURRENT device ids: the data disks are swapped + // relative to the backup, which is the state left behind by a previous restore + when(command.getRestoreVolumePaths()).thenReturn(Arrays.asList( + "/var/lib/libvirt/images/root-vol", + "/var/lib/libvirt/images/data-vol-b", + "/var/lib/libvirt/images/data-vol-a" + )); + // the backed up volumes, ordered by the device ids recorded in the backup + when(command.getBackupVolumesUUIDs()).thenReturn(Arrays.asList("root-vol", "data-vol-a", "data-vol-b")); + when(command.getBackupFiles()).thenReturn(Arrays.asList("root-vol", "data-vol-a", "data-vol-b")); + when(command.getMountTimeout()).thenReturn(30); + + try (MockedStatic filesMock = mockStatic(Files.class)) { + Path tempPath = Mockito.mock(Path.class); + when(tempPath.toString()).thenReturn("/tmp/csbackup.abc123"); + filesMock.when(() -> Files.createTempDirectory(anyString())).thenReturn(tempPath); + + try (MockedStatic