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

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2016-02-19 at 12:06 -0700, Jim Fehlig wrote:
> 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\\:6
> 789\\;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.

It has also been long expected that one can pass an iSCSI target= here.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel