On 03/27/2017 02:14 PM, Markus Armbruster wrote: >>>> >>>> Removing the .json QMP support is fine. But I'm reluctant to give R-b >>>> without knowing for sure that -drive usage won't regress. >>> >>> auth-supported landed in master only on March 2nd. >> >> auth-supported via -blockdev-add only landed on March 2nd. But >> auth-supported via -drive landed in commit 60390a2, Jan 2016; and is in >> use by libvirt: >> >> src/qemu/qemu_command.c: >> ":key=%s:auth_supported=cephx\\;none", > > That's a key-value part of the pseudo-filename. *Not* reverted by this > patch. Only QemuOpts parameter auth_supported is. > > QemuOpts parameter: -drive driver=rbd,auth_supported.0.auth=none,... > Pseudo-filename: -drive file=rbd:...:auth_supported=none > >>> What libvirt usage exactly do you think this could break? >>> >> >> Libvirt has been managing rbd drives using -drive since at least libvirt >> commit 5745cd1, in Nov 2011, where even back then it was passing: >> virBufferEscape(opt, ":", ":key=%s:auth_supported=cephx none", >> >> (back when it used space instead of ; to separate the list of supported >> auth types).
Ah, so as long as libvirt uses 'drive file=rbd:...' with key-value pairs, our backdoor of =key-values will let it continue to work. And looking more at libvirt, it definitely looks like it is sticking to file=rbd: pseudo-filenames for now; for example: tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args:-drive 'file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\ I'm not sure if (or where) a big fat comment to that effect would be beneficial, but it certainly goes a long ways in explaining the goals of your series. >> >> As I've never personally used RBD (whether through qemu directly, or >> through libvirt), I'm extremely wary of breaking -drive usage that >> "works" (for some definition of "works"), even though I have no qualms >> making the QMP interface extremely limited. > > All clear now? I think so. Commit 60390a2 (Jan 2016) mentions the use of auth_supported in the commit message (matching libvirt usage), but under the pseudo-file format; the actual rbd.c file did not special case it differently from any other key-value pair. "auth" wasn't added to QemuOpts (for the -drive driver=rbd, form) until commit 0a55679, which is unreleased, so libvirt can't have been relying on it, and ripping it out now is safe enough. Under that light, I'm now going to re-read your patch... -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
