Re: [libvirt] [PATCH v4 7/7] qemu: Add luks support for domain disk

> 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.