On Mon, Jul 11, 2016 at 02:07:58PM -0400, John Ferlan wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1301021 Generate the luks command line using the AES secret key to encrypt the luks secret. A luks secret object will be in addition to a an AES secret. For hotplug, check if the encinfo exists and if so, add the AES secret for the passphrase for the secret object used to decrypt the device. Modify/augment the fakeSecret* in qemuxml2argvtest in order to handle find a uuid or a volume usage with a specific path prefix in the XML (corresponds to the already generated XML tests). Add error message when the 'usageID' is not 'mycluster_myname'. Commit id '1d632c39' altered the error message generation to rely on the errors from the secret_driver (or it's faked replacement). Add the .args output for adding the LUKS disk to the domain Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/qemu/qemu_command.c | 9 +++ src/qemu/qemu_domain.c | 25 +++++++- src/qemu/qemu_hotplug.c | 74 +++++++++++++++++++++- .../qemuxml2argvdata/qemuxml2argv-luks-disks.args | 36 +++++++++++ tests/qemuxml2argvtest.c | 24 ++++++- 5 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c97d0f6..c356450 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1098,6 +1098,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, int actualType = virStorageSourceGetActualType(disk->src); qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus); if (idx < 0) { @@ -1237,6 +1238,10 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, secinfo->s.aes.alias); } + if (encinfo) + virQEMUBuildLuksOpts(&opt, &disk->src->encryption->encinfo, + encinfo->s.aes.alias); + if (disk->src->format > 0 && disk->src->type != VIR_STORAGE_TYPE_DIR) virBufferAsprintf(&opt, "format=%s,", @@ -1940,6 +1945,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, virDomainDiskDefPtr disk = def->disks[i]; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; /* PowerPC pseries based VMs do not support floppy device */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && @@ -1976,6 +1982,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0) return -1; + if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0) + return -1; + virCommandAddArg(cmd, "-drive"); if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps))) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d046633..ef7e3c3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -993,7 +993,8 @@ qemuDomainSecretSetup(virConnectPtr conn, { if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && - secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH) { + (secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH || + secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME)) { if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias, secretUsageType, username, seclookupdef, isLuks) < 0) @@ -1053,11 +1054,14 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk) { virStorageSourcePtr src = disk->src; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainSecretInfoPtr secinfo = NULL; - if (conn && qemuDomainSecretDiskCapable(src)) { + if (!conn) + return 0; + + if (qemuDomainSecretDiskCapable(src)) { virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI; - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); if (VIR_ALLOC(secinfo) < 0) return -1; @@ -1073,6 +1077,21 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, diskPriv->secinfo = secinfo; } + if (!virStorageSourceIsEmpty(src) && src->encryption && + src->format == VIR_STORAGE_FILE_LUKS) { + + if (VIR_ALLOC(secinfo) < 0) + return -1; + + if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, + VIR_SECRET_USAGE_TYPE_VOLUME, NULL, + &src->encryption->secrets[0]->seclookupdef, + true) < 0) + goto error; + + diskPriv->encinfo = secinfo; + } + return 0; error: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 376e6aa..d8a9fee 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -311,8 +311,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); virJSONValuePtr secobjProps = NULL; + virJSONValuePtr encProps = NULL; qemuDomainDiskPrivatePtr diskPriv; qemuDomainSecretInfoPtr secinfo; + qemuDomainSecretInfoPtr encinfo; if (!disk->info.type) { if (qemuDomainMachineIsS390CCW(vm->def) && @@ -352,6 +354,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; } + encinfo = diskPriv->encinfo; + if (encinfo && qemuBuildSecretInfoProps(encinfo, &encProps) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; @@ -371,6 +377,11 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, secobjProps) < 0) goto exit_monitor; + if (encProps && qemuMonitorAddObject(priv->mon, "secret", + encinfo->s.aes.alias, + encProps) < 0) + goto failaddencsecret;
Naming the labels after what they do instead of where we came from makes the main body easier to read. The downside is that you don't know where you jumped from in the rollback section, but it should be simple enough not to need it. I suggest 'remove_secret' (and the next step would do 'remove_encryption_secret'), if we don't need the bool-based cleanup as I suggested in 5/7.
+ if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -386,6 +397,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, * for successful exit from monitor to clear; otherwise, error * paths wouldn't clean up properly */ secobjProps = NULL; + encProps = NULL;
Same comments as in 5/7 regarding the props stealing and *DiskDevice object removal conditions. ACK with the double frees on qemuDomainObjExitMonitor fixed. Jan -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list