| Message ID | 1455681279-28451-4-git-send-email-jfehlig@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2016-02-16 at 20:54 -0700, Jim Fehlig wrote: > 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. Missing S-o-b. > --- > 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. "need to be escaped". However I wouldn't describe "\\" that way, I think I would say "note that \ is used to escape metacharacters and therefore to get a literal backslash "\\" is required". The general concept of escaping metacharaters is not mentioned in this doc at all, i.e. there is no mention of which characters need such escaping nor of the various "special" codes (\t and \n etc), nor of the octal and hex escape codes. Maybe that's a topic for another patch though. Ian.
On 02/17/2016 03:10 AM, Ian Campbell wrote: > On Tue, 2016-02-16 at 20:54 -0700, Jim Fehlig wrote: >> 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. > Missing S-o-b. > >> --- >> 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. > "need to be escaped". > > However I wouldn't describe "\\" that way, I think I would say "note that \ > is used to escape metacharacters and therefore to get a literal backslash > "\\" is required". Agreed. I changed the text and sent a V2 of the series. > > The general concept of escaping metacharaters is not mentioned in this doc > at all, i.e. there is no mention of which characters need such escaping nor > of the various "special" codes (\t and \n etc), nor of the octal and hex > escape codes. Maybe that's a topic for another patch though. I could do that in a follow-up, but I have no clue what those mean or how they are used. Regards, Jim
On Wed, 2016-02-17 at 10:24 -0700, Jim Fehlig wrote: > > > > > The general concept of escaping metacharaters is not mentioned in this doc > > at all, i.e. there is no mention of which characters need such escaping nor > > of the various "special" codes (\t and \n etc), nor of the octal and hex > > escape codes. Maybe that's a topic for another patch though. > > I could do that in a follow-up, but I have no clue what those mean or how they > are used. Fair enough. I've some spare cycles so I'll try and reverse engineer it and write it down. Ian.
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.
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.
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 '/'.
See also what is said under `script='.
Ian.
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
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.
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. Future parameter and flag names will start with an ascii letter and contain only ascii alphanumerics, hyphens and underscores, and will