JoaoJandre commented on code in PR #13556:
URL: https://github.com/apache/cloudstack/pull/13556#discussion_r3752832806
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java:
##########
@@ -213,6 +225,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);
Review Comment:
why is this needed?
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -6164,10 +6167,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;
+ }
Review Comment:
Although I do not think this is a huge issue, I think it would be best if we
could differentiate the two kinds of encryption support. Maybe we could have a
flag that is specific to RBD encryption.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##########
@@ -1493,9 +1518,126 @@ private KVMPhysicalDisk
createDiskFromTemplateOnRBD(KVMPhysicalDisk template,
disk = null;
}
}
+
+ // 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;
+ }
+
+ /**
+ * 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()) {
Review Comment:
same here
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##########
@@ -1493,9 +1518,126 @@ private KVMPhysicalDisk
createDiskFromTemplateOnRBD(KVMPhysicalDisk template,
disk = null;
}
}
+
+ // 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;
+ }
+
+ /**
+ * 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;
Review Comment:
We should avoid single letter variables. Giving the variables proper names
makes the code more readable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
