Re: [Qemu-devel] rbd: Possible regression in 2.9 RCs

[Eric Blake]

On 03/30/2017 08:22 PM, Eric Blake wrote:
> On 03/30/2017 08:05 PM, Alexandru Avadanii wrote:
>> c7cacb3e7a2e9fdf929c993b98268e4179147cbb is the first bad commit
>>     block/rbd: parse all options via bdrv_parse_filename
> 
> Yep, my bisect finished about 2 minutes after your email on the same
> spot. I'm working on a patch.  I can reproduce the problem with a mere:
> 
> ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio
> -drive
> 'file=rbd:volumes/volume-ea141b5c-cdb3-4765-910d-e7008b209a70:id=compute:key=AQAVkvxXAAAAABAA9ZxWFYdRmV+DSwKr7BKKXg==:auth_supported=cephx\;none:mon_host=192.168.1.2\:6789,format=raw,if=none,id=drive-virtio-disk0,serial=ea141b5c-cdb3-4765-910d-e7008b209a70,cache=writeback'
> 
> the good behavior (on my setup) just hangs trying to connect to a
> non-existent machine, the bad behavior gets rather-badly misparsed
> (splitting the escaped : in the host:port portion as if the port were
> the next key-value pair) resulting in an instant error message. I don't
> have an actual RBD setup for testing the fix, but will cc you on the
> patch that I propose once I have something.

I found the culprit, but the patch is taking me longer.

We are unescaping key="mon_host", value="192.168.1.2\:6789" when we
first parse the key/value pairs in qemu_rbd_parse_filename(), then we
slam the unescaped values back into a single string with ':' separators
resulting in "mon_host=192.168.1.2:6789" for stuffing through the QDict,
then we pass that string BACK through qemu_rbd_next_tok() inside
qemu_rbd_set_keypairs(), and since we no longer have the \: escape, we
are trying to treat 6789 as a new key on the second pass.  Moral of the
story: don't parse stuff twice.

My patch will be to use a QList instead of a QString for the hidden
"=keyvalue-pairs" object that we use to pass things around between
parsing the filename and actually passing parameters to RBD, matching
the fact that we already have this telling comment:

            /* FIXME: This is pretty ugly, and not the right way to do this.
             *        These should be contained in a structure, and then
             *        passed explicitly as individual key/value pairs to
             *        rados.  Consider this legacy code that needs to be
             *        updated. */

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature