[libvirt] [PATCH] qemu: Fix the command line generation for rbd auth using aes secrets

[John Ferlan]

On 08/16/2016 10:16 PM, Jim Fehlig wrote:
> On 08/16/2016 02:59 PM, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1182074
>>
>> Since libvirt still uses a legacy qemu arg format to add a disk, the
>> manner in which the 'password-secret' argument is passed to qemu needs
>> to change to prepend a 'file.' If in the future, usage of the more
>> modern disk format, then the prepended 'file.' can be removed.
>>
>> Fix based on Jim Fehlig <jfehlig at suse.com> posting and subsequent
>> upstream list followups, see:
>>
>> http://www.redhat.com/archives/libvir-list/2016-August/msg00777.html
>>
>> for details.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>
>>  Jim - if you want me to change the authorship, just let me know.
> 
> No, that's fine.
> 
>>
>>  NB: Rather than waiting for a new bz, I've reused the bz that was
>>      used for the original patches/support.
> 
> Are bug reports needed for all upstream bug fixes? If so, I'm afraid I bend that
> rule quite a bit :-).

No, but I'm in the habit of including them unless they're RHEL only (eg
locked) as a way of attributing what is the genesis of the patch...  In
this case, the original commit 'a1344f70' had the bz listed, so I'm
using it as an "update" to log issues found with the original change. I
also updated the commit message to list that commit id for completeness.

Tks -

John
> 
>>
>>  src/qemu/qemu_command.c                                            | 7 ++++++-
>>  .../qemuxml2argv-disk-drive-network-rbd-auth-AES.args              | 2 +-
>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index ebedaef..a6dea6a 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -1287,7 +1287,12 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
>>      virBufferAddLit(buf, ",");
>>  
>>      if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
>> -        virBufferAsprintf(buf, "password-secret=%s,",
>> +        /* NB: If libvirt starts using the more modern option based
>> +         *     syntax to build the command line (e.g., "-drive driver=rbd,
>> +         *     filename=%s,...") instead of the legacy model (e.g."-drive
>> +         *     file=%s,..."), then the "file." prefix can be removed
>> +         */
>> +        virBufferAsprintf(buf, "file.password-secret=%s,",
>>                            secinfo->s.aes.alias);
>>      }
>>  
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args
>> index 5034bb7..07d01b6 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args
>> @@ -26,7 +26,7 @@ data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
>>  keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
>>  -drive 'file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\
>>  mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:\
>> -6322,password-secret=virtio-disk0-secret0,format=raw,if=none,\
>> +6322,file.password-secret=virtio-disk0-secret0,format=raw,if=none,\
>>  id=drive-virtio-disk0' \
>>  -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
>>  id=virtio-disk0
> 
> Although there was no functional change from my original hack, it was easy to
> test this in my environment. Looks good.
> 
> ACK.
> 
> Regards,
> Jim
>