[PATCH V2 0/3] libxl and docs: small improvements in qdisk support

[PATCH V2 0/3] libxl and docs: small improvements in qdisk support

[Jim Fehlig]

This series contains a few improvments related to libxl's
support for the various qdisk types. Patch1 is a small fix
for libxlu_cfg to error when encountering unknown backslash-
escaped characters instead of silently dropping them. Patch2
is actually unrelated and fixes a typo noticed while reviewing
xl-disk-configuration.txt, which is improved a bit in patch3
wrt target= syntax.

V2:
Add forgotten SOB in all patches

Improve patch3 doc text

Jim Fehlig (3):
  libxlu_cfg: reject unknown characters following '\'
  docs: fix typo in xl-disk-configuration.txt
  docs: add more info about target= in disk config

 docs/misc/xl-disk-configuration.txt | 12 ++++++++++--
 tools/libxl/libxlu_cfg.c            |  5 +++++
 2 files changed, 15 insertions(+), 2 deletions(-)

-- 
2.1.4

[PATCH V2 1/3] libxlu_cfg: reject unknown characters following '\'

[Jim Fehlig]

When dequoting config strings in xlu__cfgl_dequote(), unknown
characters following a '\', and the '\' itself, are discarded.
E.g. a disk configuration string containing

  rbd:pool/image:mon_host=192.168.0.100\:6789

would be dequoted as

  rbd:pool/image:mon_host=192.168.0.1006789

Instead of discarding the '\' and unknown character, reject the
string and set error to EINVAL.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxlu_cfg.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
index 1d70909..5838f68 100644
--- a/tools/libxl/libxlu_cfg.c
+++ b/tools/libxl/libxlu_cfg.c
@@ -533,6 +533,11 @@ char *xlu__cfgl_dequote(CfgParseContext *ctx, const char *src) {
                 NUMERIC_CHAR(2,2,16,"hex");
             } else if (nc>='0' && nc<='7') {
                 NUMERIC_CHAR(1,3,10,"octal");
+            } else {
+                xlu__cfgl_lexicalerror(ctx,
+                           "invalid character after backlash in quoted string");
+                ctx->err= EINVAL;
+                goto x;
             }
             assert(p <= src+len-1);
         } else {
-- 
2.1.4

[PATCH V2 2/3] docs: fix typo in xl-disk-configuration.txt

[Jim Fehlig]

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 docs/misc/xl-disk-configuration.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
index 6a2118d..29f6ddb 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -160,7 +160,7 @@ Mandatory:             No
 Default value:         Automatically determine which backend to use.
 
 This does not affect the guest's view of the device.  It controls
-which software implementation of the Xen backend driver us used.
+which software implementation of the Xen backend driver is used.
 
 Not all backend drivers support all combinations of other options.
 For example, "phy" does not support formats other than "raw".
-- 
2.1.4

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

[Jim Fehlig]

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.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 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..79f1e4a 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 that '\' is used to escape metacharacters. Literal backslashes
+   in target= strings must use '\\'.
 
    Future parameter and flag names will start with an ascii letter and
    contain only ascii alphanumerics, hyphens and underscores, and will
-- 
2.1.4

Re: [PATCH V2 1/3] libxlu_cfg: reject unknown characters following '\'

[Konrad Rzeszutek Wilk]

On Wed, Feb 17, 2016 at 10:20:57AM -0700, Jim Fehlig wrote:
> When dequoting config strings in xlu__cfgl_dequote(), unknown
> characters following a '\', and the '\' itself, are discarded.
> E.g. a disk configuration string containing
> 
>   rbd:pool/image:mon_host=192.168.0.100\:6789
> 
> would be dequoted as
> 
>   rbd:pool/image:mon_host=192.168.0.1006789
> 
> Instead of discarding the '\' and unknown character, reject the
> string and set error to EINVAL.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

applied
> ---
>  tools/libxl/libxlu_cfg.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
> index 1d70909..5838f68 100644
> --- a/tools/libxl/libxlu_cfg.c
> +++ b/tools/libxl/libxlu_cfg.c
> @@ -533,6 +533,11 @@ char *xlu__cfgl_dequote(CfgParseContext *ctx, const char *src) {
>                  NUMERIC_CHAR(2,2,16,"hex");
>              } else if (nc>='0' && nc<='7') {
>                  NUMERIC_CHAR(1,3,10,"octal");
> +            } else {
> +                xlu__cfgl_lexicalerror(ctx,
> +                           "invalid character after backlash in quoted string");
> +                ctx->err= EINVAL;
> +                goto x;
>              }
>              assert(p <= src+len-1);
>          } else {
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH V2 2/3] docs: fix typo in xl-disk-configuration.txt

[Konrad Rzeszutek Wilk]

On Wed, Feb 17, 2016 at 10:20:58AM -0700, Jim Fehlig wrote:
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

applied
> ---
>  docs/misc/xl-disk-configuration.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
> index 6a2118d..29f6ddb 100644
> --- a/docs/misc/xl-disk-configuration.txt
> +++ b/docs/misc/xl-disk-configuration.txt
> @@ -160,7 +160,7 @@ Mandatory:             No
>  Default value:         Automatically determine which backend to use.
>  
>  This does not affect the guest's view of the device.  It controls
> -which software implementation of the Xen backend driver us used.
> +which software implementation of the Xen backend driver is used.
>  
>  Not all backend drivers support all combinations of other options.
>  For example, "phy" does not support formats other than "raw".
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Ian Campbell]

On Wed, 2016-02-17 at 10:20 -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.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

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

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH V2 3/3] docs: add more info about target= in disk config"):
> On Wed, 2016-02-17 at 10:20 -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.
> > 
> > Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

I see you've addressed Ian Campbell's comments but please see my reply
to v1.

Thanks,
Ian.