On Mon, Aug 15, 2016 at 05:35:56PM -0600, Jim Fehlig wrote:
> On 08/15/2016 03:38 PM, John Ferlan wrote:
> >
> > On 08/10/2016 06:01 PM, 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.
> >>
> > Was away on a beach last week - checked out!
>
> Nice!
>
> > So still trying to catch
> > up... I'm CC'd Daniel since he made the qemu adjustments for this.
> >
> > So one question I'd have is do things work if the disk was in the domain
> > at startup?
>
> No, same problem whether hotplugging a disk or starting the domain with already
> existing disk.
>
> > The libvirt code essentially reuses the same qemu command
> > for initial startup and hotplug. The usage of "file.password-secret"
> > seems to indicate to me something's amiss.
>
> I mentioned this issue to Bruce (cc'd) and after peeking at the qemu option
> parsing code, he thought networks disks using curl protocol would suffer the
> same issue, but iscsi protocol would not. If so, my hack would need to be a bit
> smarter.
It appears that the libvirt code for CURL is also using the legacy syntax
so probably needs the same fix too. It looks like ISCSI takes the same
codepath in libvirt as CURL, so probably nedes the same fix.
I can't tell for sure though, since it appears we never added any unit
tests for CURL + ISCSI with password secrets :-(
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list