Re: [PATCH 3/3] docs: add more info about target= in disk config

[Jim Fehlig <jfehlig@suse.com>]

On 02/19/2016 10:23 AM, Ian Jackson wrote:
> Jim Fehlig writes ("[PATCH 3/3] docs: add more info about target= in disk config"):
>> target= in disk config can be used to convey arbitrary
>> configuration information to backends. Add a bit more info
>> to xl-disk-configuration.txt to clarify this, including some
>> simple nbd and rbd qdisk configurations.
>> ---
>>  docs/misc/xl-disk-configuration.txt | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
>> index 29f6ddb..0918fb8 100644
>> --- a/docs/misc/xl-disk-configuration.txt
>> +++ b/docs/misc/xl-disk-configuration.txt
>> @@ -75,7 +75,15 @@ Special syntax:
>>     the target was already specified as a positional parameter.  This
>>     is the only way to specify a target string containing metacharacters
>>     such as commas and (in some cases) colons, which would otherwise be
>> -   misinterpreted.
>> +   misinterpreted. Meta-information in a target string can be used to
>> +   specify configuration information for a qdisk block backend. For
>> +   example the nbd and rbd qdisk block backends can be configured with
>> +
>> +     target=nbd:192.168.0.1:5555
>> +     target=rbd:pool/image:mon_host=192.186.0.1\\:6789
>> +
>> +   Note the use of double backslash ('\\') for metacharacters that need
>> +   escaped.
> I'm not entirely comfortable with documenting this as supported.
> The difficulties I see are:
>
>
> In the usual configuration, libxl decides for itself what (libxl)
> backend to use.  Different versions of libxl might make different
> choices, so a configuration that works with one version of libxl might
> not work with another.  That's fine for an undocumented feature but
> not so good if it's actually advertised.  At the very least the docs
> need to say that to rely on this you must specify backend=qdisk.

The text I added in this patch states that meta-information can be used with
qdisk. I didn't go as far as saying _only_ qdisk, since other backends might
interpret such meta-information too. But I can add that if we decide to go with
this doc patch.

>
>
> And this is a layering violation, or rather a violation of the
> expected semantics of the target string.
>
> I think it would be much better to support nbd and rbd explicitly in
> libxl.  Maybe we should have a "protocol=" parameter, so you could
> write something like this:
>    disk=["vdev=xvda, protocol=nbd, target=192.168.0.1:5555"]
>
> This would allow libxl to make better choices about backends, even if
> right now all it does is force the use of the qemu backend and pass
> the target string to qemu.

I agree with your suggestion, which is why I took that approach in the original
RFC post

http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03184.html

Did you see the doc and IDL RFC patch attached to that post? IMO, we need more
than just protocol. An rbd device configuration can include a pool/volume name,
multiple servers, auth type, auth username, and auth passwd/data. E.g.

  disk = [ 'vdev=xvda, backendtype=qdisk, backendprotocol=rbd,
server=192.168.0.1:5555, server=192.168.0.2:5555, auth=joe:joes-secret,
target=some-pool/some-image' ]

>
>
> Finally, if this is actually true, it is a bug.  The specification
> (docs/misc/xl-disk-configuration.txt) says:
>
>   Description:           Block device or image file path.  When this is
>                          used as a path, /dev will be prepended
>                          if the path doesn't start with a '/'.

That is not true. I've successfully used the following disk config

  disk = [ "vdev=xvdb, backendtype=qdisk,
               
target=rbd:libvirtpool/image:auth_supported=none:mon_host=192.168.0.1\\:6789\\;192.168.0.2\\:6789\\;192.168.0.3\\:6789"
]

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

I wouldn't call those targets "image file paths", but they don't start with a
'/' and none is prepended.

> See also what is said under `script='.

Do you mean how the <script> path is determined? Or the relationship between
<script> and <target>?

Regards,
Jim