[libvirt] problem with rbd auth after switch to secret objects

[Daniel P. Berrange]

On Tue, Aug 16, 2016 at 12:14:50PM +0100, Daniel P. Berrange wrote:
> On Wed, Aug 10, 2016 at 04:01:11PM -0600, Jim Fehlig wrote:
> > Hi John,
> > 
> > I've been having problems with rbd auth since the change to using qemu's secret
> > objects. E.g. when hotplugging disk config
> > 
> > <disk type="network" device="disk">
> >   <driver name="qemu" type="raw" cache="none"/>
> >   <source protocol="rbd" name="volumes/volume-f9c33a0a-5313-44fc-9624-c3b09ed21a57">
> >     <host name="xxx.xxx.xxx.xxx" port="6789"/>
> >   </source>
> >   <auth username="cinder">
> >     <secret type="ceph" uuid="dcff478d-8021-42c4-b57a-98b5f5447e8f"/>
> >   </auth>
> >   <target bus="virtio" dev="vdb"/>
> > </disk>
> > 
> > libvirt issues the following monitor commands
> > 
> > 2016-08-08 16:13:41.720+0000: 27504: info : qemuMonitorSend:1006 :
> > QEMU_MONITOR_SEND_MSG: mon=0x7f55c4000f50
> > msg={"execute":"object-add","arguments":{"qom-type":"secret","id":"virtio-disk1-secret0","props":{"data":"w6x17STyqO9tMEOpAJy9Mnx+B5R1qrsJBXZZn/uZCKU=","keyid":"masterKey0","iv":"ZAE6WkKf+jDIl9lJkXGsnQ==","format":"base64"}},"id":"libvirt-12"}
> > 2016-08-08 16:13:41.722+0000: 27504: debug : qemuMonitorJSONCommandWithFd:296 :
> > Send command
> > '{"execute":"human-monitor-command","arguments":{"command-line":"drive_add dummy
> > file=rbd:volumes/volume-f9c33a0a-5313-44fc-9624-c3b09ed21a57:id=cinder:auth_supported=cephx\\;none:mon_host=xxx.xx.xxx.xxx\\:6789,password-secret=virtio-disk1-secret0,format=raw,if=none,id=drive-virtio-disk1,cache=none"},"id":"libvirt-13"}'
> > 
> > The latter fails with
> > 
> > 2016-08-08 16:13:41.733+0000: 27499: debug : virJSONValueFromString:1604 :
> > string={"return": "error connecting\r\n", "id": "libvirt-13"}
> > 
> > Debugging in the qemu rbd code, I found that
> > 
> > secretid = qemu_opt_get(opts, "password-secret");
> > 
> > in $qemu-src/block/rbd.c:qemu_rbd_create() returns NULL. The NULL secretid is
> > later passed to qemu_rbd_set_auth(), which silently returns success when
> > secretid==NULL. Later, rados_connect() fails with "error connecting" since the
> > secret was not configured.
> > 
> > I'm not familiar with qemu option parsing, but it seems the
> > ...,password-secret=xxx,... associates the password-secret option parsing with
> > the drive object, whereas it needs to be associated with the rbd "file" object?
> > As a quick hack test, I made the following change in libvirt and then was able
> > to successfully attach the disk
> > 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 55df23d..eb478fb 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -1287,7 +1287,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
> >      virBufferAddLit(buf, ",");
> >  
> >      if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
> > -        virBufferAsprintf(buf, "password-secret=%s,",
> > +        virBufferAsprintf(buf, "file.password-secret=%s,",
> >                            secinfo->s.aes.alias);
> >      }
> > 
> > I suspect others (including yourself) have done this successfully without that
> > hack, so I'm not quite sure what the problem might be in my configuration. I'm
> > using libvirt.git master and qemu 2.6, but I didn't notice any post-2.6 patches
> > that would help on the qemu side.
> 
> That change is correct. I presume John just implemented libvirt based on
> my QEMU commit message which had the wrong syntax shown as an example.

Ok actually on second glance it is more subtle than that.

There is two distinct syntaxes for block drivers. The legacy "filename"
based one and the modern option based one.

My QEMU commit message used the modern syntax

 -drive driver=rbd,filename=rbd:pool/image:\
    id=myname:auth_supported=cephx,password-secret=secret0

but it appears libvirt is using the legacy syntax for RBD

  -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,\
     id=drive-virtio-disk0' 

If using the legacy syntax, then you need "file.password-secret", but
if using the modern syntax you just need "password-secret"

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|