[Qemu-devel] [PATCH 0/2] qemu-img: src_mode option documentation improvements

[Qemu-devel] [PATCH 0/2] qemu-img: src_mode option documentation improvements

[Stefan Hajnoczi]

These patches clarify and fix the documentation for the recent qemu-img
src_mode (-T) option.

Stefan Hajnoczi (2):
  qemu-img: clarify src_cache option documentation
  qemu-img: fix rebase src_cache option documentation

 qemu-img.c    | 3 ++-
 qemu-img.texi | 7 ++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/2] qemu-img: clarify src_cache option documentation

[Stefan Hajnoczi]

The source cache option takes the same values as the cache option.  The
documentation reads a little strange because it starts with "In contrast
the src_cache option ...".  The fact that this is comparing with the
previous documented option (the 'cache' option) is implicit.  Readers
may be confused, especially if they jump to src_cache without reading
cache documentation first.

Suggested-by: Jeff Nelson <jenelson@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-img.c    | 3 ++-
 qemu-img.texi | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ff29ed1..91d1ac3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -95,7 +95,8 @@ static void QEMU_NORETURN help(void)
            "  'cache' is the cache mode used to write the output disk image, the valid\n"
            "    options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n"
            "    'directsync' and 'unsafe' (default for convert)\n"
-           "  'src_cache' in contrast is the cache mode used to read input disk images\n"
+           "  'src_cache' is the cache mode used to read input disk images, the valid\n"
+           "    options are the same as for the 'cache' option\n"
            "  'size' is the disk image size in bytes. Optional suffixes\n"
            "    'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M),\n"
            "    'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte, 1024P)  are\n"
diff --git a/qemu-img.texi b/qemu-img.texi
index cb68948..4380d56 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -73,8 +73,9 @@ specifies the cache mode that should be used with the (destination) file. See
 the documentation of the emulator's @code{-drive cache=...} option for allowed
 values.
 @item -T @var{src_cache}
-in contrast specifies the cache mode that should be used with the source
-file(s).
+specifies the cache mode that should be used with the source file(s). See
+the documentation of the emulator's @code{-drive cache=...} option for allowed
+values.
 @end table
 
 Parameters to snapshot subcommand:
-- 
1.9.3

[Qemu-devel] [PATCH 2/2] qemu-img: fix rebase src_cache option documentation

[Stefan Hajnoczi]

The src_cache option (-T) specifies the cache mode for backing files.
It applies both the image's old backing file as well as the new backing
file:

  ret = bdrv_open(&bs_old_backing, backing_name, NULL, NULL, src_flags,
                  old_backing_drv, &local_err);
  if (ret) {
      ...
  }
  if (out_baseimg[0]) {
      bs_new_backing = bdrv_new("new_backing", &error_abort);
      ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL, src_flags,
                      new_backing_drv, &local_err);
      if (ret) {
          ...
      }
  }

The documentation only mentions the new backing file but it really
applies to both.

Suggested-by: Jeff Nelson <jenelson@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-img.texi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.texi b/qemu-img.texi
index 4380d56..7c2df14 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -341,7 +341,7 @@ string), then the image is rebased onto no backing file (i.e. it will exist
 independently of any backing file).
 
 @var{cache} specifies the cache mode to be used for @var{filename}, whereas
-@var{src_cache} specifies the cache mode for reading the new backing file.
+@var{src_cache} specifies the cache mode for reading backing file files.
 
 There are two different modes in which @code{rebase} can operate:
 @table @option
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/2] qemu-img: clarify src_cache option documentation

[Fam Zheng]

On Mon, 09/01 16:11, Stefan Hajnoczi wrote:
> The source cache option takes the same values as the cache option.  The
> documentation reads a little strange because it starts with "In contrast
> the src_cache option ...".  The fact that this is comparing with the
> previous documented option (the 'cache' option) is implicit.  Readers
> may be confused, especially if they jump to src_cache without reading
> cache documentation first.
> 
> Suggested-by: Jeff Nelson <jenelson@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qemu-img.c    | 3 ++-
>  qemu-img.texi | 5 +++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index ff29ed1..91d1ac3 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -95,7 +95,8 @@ static void QEMU_NORETURN help(void)
>             "  'cache' is the cache mode used to write the output disk image, the valid\n"
>             "    options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n"
>             "    'directsync' and 'unsafe' (default for convert)\n"
> -           "  'src_cache' in contrast is the cache mode used to read input disk images\n"
> +           "  'src_cache' is the cache mode used to read input disk images, the valid\n"
> +           "    options are the same as for the 'cache' option\n"
>             "  'size' is the disk image size in bytes. Optional suffixes\n"
>             "    'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M),\n"
>             "    'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte, 1024P)  are\n"
> diff --git a/qemu-img.texi b/qemu-img.texi
> index cb68948..4380d56 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -73,8 +73,9 @@ specifies the cache mode that should be used with the (destination) file. See
>  the documentation of the emulator's @code{-drive cache=...} option for allowed
>  values.
>  @item -T @var{src_cache}
> -in contrast specifies the cache mode that should be used with the source
> -file(s).
> +specifies the cache mode that should be used with the source file(s). See
> +the documentation of the emulator's @code{-drive cache=...} option for allowed
> +values.
>  @end table
>  
>  Parameters to snapshot subcommand:
> -- 
> 1.9.3
> 
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH 2/2] qemu-img: fix rebase src_cache option documentation

[Fam Zheng]

On Mon, 09/01 16:11, Stefan Hajnoczi wrote:
> The src_cache option (-T) specifies the cache mode for backing files.
> It applies both the image's old backing file as well as the new backing
> file:
> 
>   ret = bdrv_open(&bs_old_backing, backing_name, NULL, NULL, src_flags,
>                   old_backing_drv, &local_err);
>   if (ret) {
>       ...
>   }
>   if (out_baseimg[0]) {
>       bs_new_backing = bdrv_new("new_backing", &error_abort);
>       ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL, src_flags,
>                       new_backing_drv, &local_err);
>       if (ret) {
>           ...
>       }
>   }
> 
> The documentation only mentions the new backing file but it really
> applies to both.
> 
> Suggested-by: Jeff Nelson <jenelson@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qemu-img.texi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-img.texi b/qemu-img.texi
> index 4380d56..7c2df14 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -341,7 +341,7 @@ string), then the image is rebased onto no backing file (i.e. it will exist
>  independently of any backing file).
>  
>  @var{cache} specifies the cache mode to be used for @var{filename}, whereas
> -@var{src_cache} specifies the cache mode for reading the new backing file.
> +@var{src_cache} specifies the cache mode for reading backing file files.

Is "backing file files" intended, why not just "backing files"? :)

Fam

>  
>  There are two different modes in which @code{rebase} can operate:
>  @table @option
> -- 
> 1.9.3
> 
> 

Re: [Qemu-devel] [PATCH 2/2] qemu-img: fix rebase src_cache option documentation

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

On Tue, Sep 02, 2014 at 10:52:53AM +0800, Fam Zheng wrote:
> On Mon, 09/01 16:11, Stefan Hajnoczi wrote:
> > The src_cache option (-T) specifies the cache mode for backing files.
> > It applies both the image's old backing file as well as the new backing
> > file:
> > 
> >   ret = bdrv_open(&bs_old_backing, backing_name, NULL, NULL, src_flags,
> >                   old_backing_drv, &local_err);
> >   if (ret) {
> >       ...
> >   }
> >   if (out_baseimg[0]) {
> >       bs_new_backing = bdrv_new("new_backing", &error_abort);
> >       ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL, src_flags,
> >                       new_backing_drv, &local_err);
> >       if (ret) {
> >           ...
> >       }
> >   }
> > 
> > The documentation only mentions the new backing file but it really
> > applies to both.
> > 
> > Suggested-by: Jeff Nelson <jenelson@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  qemu-img.texi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/qemu-img.texi b/qemu-img.texi
> > index 4380d56..7c2df14 100644
> > --- a/qemu-img.texi
> > +++ b/qemu-img.texi
> > @@ -341,7 +341,7 @@ string), then the image is rebased onto no backing file (i.e. it will exist
> >  independently of any backing file).
> >  
> >  @var{cache} specifies the cache mode to be used for @var{filename}, whereas
> > -@var{src_cache} specifies the cache mode for reading the new backing file.
> > +@var{src_cache} specifies the cache mode for reading backing file files.
> 
> Is "backing file files" intended, why not just "backing files"? :)

Ha!  Thanks for spotting that.  I managed to typo it.  Fixed in v2.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]