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

[John Ferlan]

[...]

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