Re: [RFC] support more qdisk types

[Jim Fehlig <jfehlig@suse.com>]

On 02/03/2016 07:53 PM, Jim Fehlig wrote:
> On 02/03/2016 02:56 AM, Ian Campbell wrote:
>> On Tue, 2016-02-02 at 15:06 -0700, Jim Fehlig wrote:
>>>> And extending
>>>> the structure seems to be the right thing to do.
>>> So what do you think of the approach in the RFC patch? It adds discrete knobs in
>>> the disk config and extends the disk structure similarly. Before I can make
>>> progress on this we need to agree on extending the config and libxl_device_disk
>>> structure.
>> My main concern is that this approach requires us to update libxl for each
>> new possible backend type.
> Yes, understood.
>
>> The intention of the target= in the disk spec is that it consumes the rest
>> of the line so it can be used to encode pretty much anything. Is it not
>> possible (modulo bugs) to pass all the necessary information to qdisk in
>> this form? I thought Dave S had made it possible to use qdisk in this way
>> back in:
>>
>>     commit a8a1f236a2964506a22d1779648d8e1c8668cb1a
>>     Author: David Scott <    dave.scott@eu.citrix.com    >
>>     Date:   Tue Apr 23 10:59:26 2013 +0100
>>
>>         libxl: Only call stat() when adding a disk if we expect a device to exist.
>>         
>>         We consider calling stat() a helpful error check in the following
>>         circumstances only:
>>          1. the disk backend type must be PHYsical
>>          2. the disk backend domain must be the same as the running libxl
>>             code (ie LIBXL_TOOLSTACK_DOMID)
>>          3. there must not be a hotplug script because this would imply that
>>             the device won't be created until after the hotplug script has
>>             run.
>>         
>>         With this fix, it is possible to use qemu's built-in block drivers
>>         such as ceph/rbd, with a xl config disk spec like this:
>>         
>>         disk=[ 'backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img' ]
> I thought I tried disk config along those lines with no success. But I'll
> certainly take a closer look at using target= to encode the config needed by
> these qdisk block drivers. Thanks for the pointer.

I think my previous problem was related to quoting in a disk spec containing
several ceph monitors. According to $qemu-src/hw/block/rbd.c, ':' is used to
delimit option=value tuples. I was escaping the colon between a ceph monitor
server and port with a single '\'. E.g.

disk = [ "vdev=xvdb, backendtype=qdisk,
target=rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\:6789;192.168.0.101\:6789;192.168.0.102\:6789"
]

What I didn't realize was that libxl omitted the entire colon when writing the
string to xenstore

xenstore-read /local/domain/0/backend/qdisk/55/51728/params
aio:rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.1006789;192.168.0.1016789;192.168.0.1026789

which caused the rbd block driver to barf. Using double backslash worked. E.g.

disk = [ "vdev=xvdb, backendtype=qdisk,
target=rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\\:6789;192.168.0.101\\:6789;192.168.0.102\\:6789"
]

with corresponding xenstore entry

aio:rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\:6789;192.168.0.101\:6789;192.168.0.102\:6789

Note that specifying server:port in nbd doesn't require quoting the colon. The
following config works just fine

disk = [ "vdev=xvdb, backendtype=qdisk, target=nbd:192.168.0.150:5555"

In the end, maybe all that's needed is a few more examples in
xl-disk-configuration. Perhaps a paragraph under "Special Syntax" of 'target'
doc, along with example config like the above?

Regards,
Jim