[...]
>> 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.
>
This is now cleaner with the bool-based cleanup...
>> +
>> 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.
>
Much cleaner now with bool-based cleanup
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list