[PATCH v2 0/2] qemu-img: Add --target-is-zero to indicate that a target is blank

[PATCH v2 0/2] qemu-img: Add --target-is-zero to indicate that a target is blank

[David Edmondson]

qemu-img: Add --target-is-zero to indicate that a target is blank

v2:
- Remove target_is_zero, preferring to set has_zero_init
  directly (Mark Kanda).
- Disallow --target-is-zero in the presence of a backing file (Max
  Reitz).
- Add relevant documentation (Max Reitz).
- @var -> @code for options in qemu-img.texi.


David Edmondson (2):
  qemu-img: Add --target-is-zero to convert
  doc: Use @code rather than @var for options

 qemu-img-cmds.hx |  4 ++--
 qemu-img.c       | 25 ++++++++++++++++++++++---
 qemu-img.texi    | 20 ++++++++++++--------
 3 files changed, 36 insertions(+), 13 deletions(-)

-- 
2.24.1

[PATCH v2 1/2] qemu-img: Add --target-is-zero to convert

[David Edmondson]

In many cases the target of a convert operation is a newly provisioned
target that the user knows is blank (filled with zeroes). In this
situation there is no requirement for qemu-img to wastefully zero out
the entire device.

Add a new option, --target-is-zero, allowing the user to indicate that
an existing target device is already zero filled.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 qemu-img-cmds.hx |  4 ++--
 qemu-img.c       | 25 ++++++++++++++++++++++---
 qemu-img.texi    |  4 ++++
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 1c93e6d185..6f958a0a48 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -44,9 +44,9 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("create", img_create,
diff --git a/qemu-img.c b/qemu-img.c
index 6233b8ca56..46db72dbe0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -70,6 +70,7 @@ enum {
     OPTION_PREALLOCATION = 265,
     OPTION_SHRINK = 266,
     OPTION_SALVAGE = 267,
+    OPTION_TARGET_IS_ZERO = 268,
 };
 
 typedef enum OutputFormat {
@@ -1984,10 +1985,9 @@ static int convert_do_copy(ImgConvertState *s)
     int64_t sector_num = 0;
 
     /* Check whether we have zero initialisation or can get it efficiently */
-    if (s->target_is_new && s->min_sparse && !s->target_has_backing) {
+    if (!s->has_zero_init && s->target_is_new && s->min_sparse &&
+        !s->target_has_backing) {
         s->has_zero_init = bdrv_has_zero_init(blk_bs(s->target));
-    } else {
-        s->has_zero_init = false;
     }
 
     if (!s->has_zero_init && !s->target_has_backing &&
@@ -2086,6 +2086,7 @@ static int img_convert(int argc, char **argv)
             {"force-share", no_argument, 0, 'U'},
             {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
             {"salvage", no_argument, 0, OPTION_SALVAGE},
+            {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
@@ -2209,6 +2210,14 @@ static int img_convert(int argc, char **argv)
         case OPTION_TARGET_IMAGE_OPTS:
             tgt_image_opts = true;
             break;
+        case OPTION_TARGET_IS_ZERO:
+            /*
+             * The user asserting that the target is blank has the
+             * same effect as the target driver supporting zero
+             * initialisation.
+             */
+            s.has_zero_init = true;
+            break;
         }
     }
 
@@ -2247,6 +2256,11 @@ static int img_convert(int argc, char **argv)
         warn_report("This will become an error in future QEMU versions.");
     }
 
+    if (s.has_zero_init && !skip_create) {
+        error_report("--target-is-zero requires use of -n flag");
+        goto fail_getopt;
+    }
+
     s.src_num = argc - optind - 1;
     out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
 
@@ -2380,6 +2394,11 @@ static int img_convert(int argc, char **argv)
     }
     s.target_has_backing = (bool) out_baseimg;
 
+    if (s.has_zero_init && s.target_has_backing) {
+        error_report("Cannot use --target-is-zero with a backing file");
+        goto out;
+    }
+
     if (s.src_num > 1 && out_baseimg) {
         error_report("Having a backing file for the target makes no sense when "
                      "concatenating multiple input images");
diff --git a/qemu-img.texi b/qemu-img.texi
index b5156d6316..3b6dfd8682 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -179,6 +179,10 @@ information.
 Try to ignore I/O errors when reading.  Unless in quiet mode (@code{-q}), errors
 will still be printed.  Areas that cannot be read from the source will be
 treated as containing only zeroes.
+@item --target-is-zero
+Assume that the destination is filled with zeros. This parameter is
+mutually exclusive with the use of a backing file. It is required to
+also use the @code{-n} parameter to skip image creation.
 @end table
 
 Parameters to dd subcommand:
-- 
2.24.1

Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert

[Eric Blake]

On 1/24/20 4:34 AM, David Edmondson wrote:
> In many cases the target of a convert operation is a newly provisioned
> target that the user knows is blank (filled with zeroes). In this
> situation there is no requirement for qemu-img to wastefully zero out
> the entire device.
> 
> Add a new option, --target-is-zero, allowing the user to indicate that
> an existing target device is already zero filled.
> 
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
>   qemu-img-cmds.hx |  4 ++--
>   qemu-img.c       | 25 ++++++++++++++++++++++---
>   qemu-img.texi    |  4 ++++
>   3 files changed, 28 insertions(+), 5 deletions(-)

I'm working up a patch series that tries to auto-set this flag without 
user interaction where possible (for example, if lseek(fd, 0, SEEK_DATA) 
returns EOF, or if fstat() reports 0 blocks allocated, or if qcow2 sees 
no L2 tables allocated, or a proposed extension to NBD passes on the 
same...).  I may rebase my series on top of your patch and tweak things 
in yours accordingly.

But as it stands, the idea makes sense to me; even if we add ways for 
some images to efficiently report initial state (and our existing 
bdrv_has_zero_init() is NOT such a method), there are enough other 
scenarios where the knob will be the only way to let qemu-img know the 
intent.


> +        case OPTION_TARGET_IS_ZERO:
> +            /*
> +             * The user asserting that the target is blank has the
> +             * same effect as the target driver supporting zero
> +             * initialisation.

Hmm. A git grep shows that 'initialization' has 200 hits, 
'initialisation' has only 29. But I think it's a US vs. UK thing, so I 
don't care which spelling you use.


> @@ -2247,6 +2256,11 @@ static int img_convert(int argc, char **argv)
>           warn_report("This will become an error in future QEMU versions.");
>       }
>   
> +    if (s.has_zero_init && !skip_create) {
> +        error_report("--target-is-zero requires use of -n flag");
> +        goto fail_getopt;
> +    }
> +

Makes sense, although we could perhaps relax it to also work even when 
the -n flag is supplied IF the destination image supports my proposal 
for a new status bit set when an image is known to be opened with all 
zero content.

>       s.src_num = argc - optind - 1;
>       out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
>   
> @@ -2380,6 +2394,11 @@ static int img_convert(int argc, char **argv)
>       }
>       s.target_has_backing = (bool) out_baseimg;
>   
> +    if (s.has_zero_init && s.target_has_backing) {
> +        error_report("Cannot use --target-is-zero with a backing file");
> +        goto out;
> +    }
> +

Makes sense, although we could perhaps relax it to also work even when 
there is a backing file IF the backing file supports my proposal for a 
new status bit set when an image is known to be opened with all zero 
content.

As my patch proposal is still not submitted, I'm fine if yours lands as-is:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert

[David Edmondson]

Eric Blake <eblake@redhat.com> writes:

> On 1/24/20 4:34 AM, David Edmondson wrote:
>> In many cases the target of a convert operation is a newly provisioned
>> target that the user knows is blank (filled with zeroes). In this
>> situation there is no requirement for qemu-img to wastefully zero out
>> the entire device.
>> 
>> Add a new option, --target-is-zero, allowing the user to indicate that
>> an existing target device is already zero filled.
>> 
>> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>> ---
>>   qemu-img-cmds.hx |  4 ++--
>>   qemu-img.c       | 25 ++++++++++++++++++++++---
>>   qemu-img.texi    |  4 ++++
>>   3 files changed, 28 insertions(+), 5 deletions(-)
>
> I'm working up a patch series that tries to auto-set this flag without 
> user interaction where possible (for example, if lseek(fd, 0, SEEK_DATA) 
> returns EOF, or if fstat() reports 0 blocks allocated, or if qcow2 sees 
> no L2 tables allocated, or a proposed extension to NBD passes on the 
> same...).  I may rebase my series on top of your patch and tweak things 
> in yours accordingly.
>
> But as it stands, the idea makes sense to me; even if we add ways for 
> some images to efficiently report initial state (and our existing 
> bdrv_has_zero_init() is NOT such a method), there are enough other 
> scenarios where the knob will be the only way to let qemu-img know the 
> intent.

Having qemu-img figure things out on its own is obviously desirable, but
I agree that there are enough cases where this won't be possible and,
given the resulting performance improvement, it will still be useful to
allow the caller to force things.

>> +        case OPTION_TARGET_IS_ZERO:
>> +            /*
>> +             * The user asserting that the target is blank has the
>> +             * same effect as the target driver supporting zero
>> +             * initialisation.
>
> Hmm. A git grep shows that 'initialization' has 200 hits, 
> 'initialisation' has only 29. But I think it's a US vs. UK thing, so I 
> don't care which spelling you use.

Yes, it's British English spelling. It was unconscious - I'll switch if
there is an existing policy.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks.

If the conversion of the documentation to rST is imminent then I'll wait
for that before submitting a followup with corresponding changes applied
to the new docs.

dme.
-- 
I'd come on over but I haven't got a raincoat.

Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert

[Vladimir Sementsov-Ogievskiy]

24.01.2020 13:34, David Edmondson wrote:
> In many cases the target of a convert operation is a newly provisioned
> target that the user knows is blank (filled with zeroes). In this
> situation there is no requirement for qemu-img to wastefully zero out
> the entire device.
> 
> Add a new option, --target-is-zero, allowing the user to indicate that
> an existing target device is already zero filled.

Hi! qemu-img.c part looks OK for me, but other doesn't apply on master now.

I like this thing, and I'd like to make similar option for backup job.

> 
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
>   qemu-img-cmds.hx |  4 ++--
>   qemu-img.c       | 25 ++++++++++++++++++++++---
>   qemu-img.texi    |  4 ++++
>   3 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 1c93e6d185..6f958a0a48 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -44,9 +44,9 @@ STEXI
>   ETEXI
>   
>   DEF("convert", img_convert,
> -    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
> +    "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
>   STEXI
> -@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
> +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
>   ETEXI
>   
>   DEF("create", img_create,
> diff --git a/qemu-img.c b/qemu-img.c
> index 6233b8ca56..46db72dbe0 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -70,6 +70,7 @@ enum {
>       OPTION_PREALLOCATION = 265,
>       OPTION_SHRINK = 266,
>       OPTION_SALVAGE = 267,
> +    OPTION_TARGET_IS_ZERO = 268,
>   };
>   
>   typedef enum OutputFormat {
> @@ -1984,10 +1985,9 @@ static int convert_do_copy(ImgConvertState *s)
>       int64_t sector_num = 0;
>   
>       /* Check whether we have zero initialisation or can get it efficiently */
> -    if (s->target_is_new && s->min_sparse && !s->target_has_backing) {
> +    if (!s->has_zero_init && s->target_is_new && s->min_sparse &&
> +        !s->target_has_backing) {
>           s->has_zero_init = bdrv_has_zero_init(blk_bs(s->target));
> -    } else {
> -        s->has_zero_init = false;
>       }
>   
>       if (!s->has_zero_init && !s->target_has_backing &&
> @@ -2086,6 +2086,7 @@ static int img_convert(int argc, char **argv)
>               {"force-share", no_argument, 0, 'U'},
>               {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
>               {"salvage", no_argument, 0, OPTION_SALVAGE},
> +            {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO},
>               {0, 0, 0, 0}
>           };
>           c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
> @@ -2209,6 +2210,14 @@ static int img_convert(int argc, char **argv)
>           case OPTION_TARGET_IMAGE_OPTS:
>               tgt_image_opts = true;
>               break;
> +        case OPTION_TARGET_IS_ZERO:
> +            /*
> +             * The user asserting that the target is blank has the
> +             * same effect as the target driver supporting zero
> +             * initialisation.
> +             */
> +            s.has_zero_init = true;
> +            break;
>           }
>       }
>   
> @@ -2247,6 +2256,11 @@ static int img_convert(int argc, char **argv)
>           warn_report("This will become an error in future QEMU versions.");
>       }
>   
> +    if (s.has_zero_init && !skip_create) {
> +        error_report("--target-is-zero requires use of -n flag");
> +        goto fail_getopt;
> +    }
> +
>       s.src_num = argc - optind - 1;
>       out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
>   
> @@ -2380,6 +2394,11 @@ static int img_convert(int argc, char **argv)
>       }
>       s.target_has_backing = (bool) out_baseimg;
>   
> +    if (s.has_zero_init && s.target_has_backing) {
> +        error_report("Cannot use --target-is-zero with a backing file");
> +        goto out;
> +    }
> +
>       if (s.src_num > 1 && out_baseimg) {
>           error_report("Having a backing file for the target makes no sense when "
>                        "concatenating multiple input images");
> diff --git a/qemu-img.texi b/qemu-img.texi
> index b5156d6316..3b6dfd8682 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -179,6 +179,10 @@ information.
>   Try to ignore I/O errors when reading.  Unless in quiet mode (@code{-q}), errors
>   will still be printed.  Areas that cannot be read from the source will be
>   treated as containing only zeroes.
> +@item --target-is-zero
> +Assume that the destination is filled with zeros. This parameter is
> +mutually exclusive with the use of a backing file. It is required to
> +also use the @code{-n} parameter to skip image creation.
>   @end table
>   
>   Parameters to dd subcommand:
> 


-- 
Best regards,
Vladimir

Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert

[Eric Blake]

On 2/3/20 12:20 PM, Vladimir Sementsov-Ogievskiy wrote:
> 24.01.2020 13:34, David Edmondson wrote:
>> In many cases the target of a convert operation is a newly provisioned
>> target that the user knows is blank (filled with zeroes). In this
>> situation there is no requirement for qemu-img to wastefully zero out
>> the entire device.
>>
>> Add a new option, --target-is-zero, allowing the user to indicate that
>> an existing target device is already zero filled.
> 
> Hi! qemu-img.c part looks OK for me, but other doesn't apply on master now.

Correct. Patch 2/2 is now obsolete and no longer necessary, and patch 1 
needs some tweaks now that we don't have qemu-img.texi.

> 
> I like this thing, and I'd like to make similar option for backup job.

My followup patches to add an all-zero bit to qcow2 are also useful in 
this regard.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert

[David Edmondson]

On Monday, 2020-02-03 at 21:20:16 +03, Vladimir Sementsov-Ogievskiy wrote:

> 24.01.2020 13:34, David Edmondson wrote:
>> In many cases the target of a convert operation is a newly provisioned
>> target that the user knows is blank (filled with zeroes). In this
>> situation there is no requirement for qemu-img to wastefully zero out
>> the entire device.
>> 
>> Add a new option, --target-is-zero, allowing the user to indicate that
>> an existing target device is already zero filled.
>
> Hi! qemu-img.c part looks OK for me, but other doesn't apply on master now.

Updated v3 just sent.

dme.
-- 
But he said, leave me alone, I'm a family man.

[PATCH v2 2/2] doc: Use @code rather than @var for options

[David Edmondson]

Texinfo defines @var for metasyntactic variables and such terms are
shown in upper-case or italics in the output of makeinfo. When
considering an option to a command, such as "-n", upper-casing is
undesirable as it may confuse the reader or be in conflict with the
equivalent upper-case option.

Replace the use of @var for options with @code to avoid this.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 qemu-img.texi | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/qemu-img.texi b/qemu-img.texi
index 3b6dfd8682..6b4a1ac961 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -74,13 +74,13 @@ keys.
 @item --image-opts
 Indicates that the source @var{filename} parameter is to be interpreted as a
 full option string, not a plain filename. This parameter is mutually
-exclusive with the @var{-f} parameter.
+exclusive with the @code{-f} parameter.
 
 @item --target-image-opts
 Indicates that the @var{output_filename} parameter(s) are to be interpreted as
 a full option string, not a plain filename. This parameter is mutually
-exclusive with the @var{-O} parameters. It is currently required to also use
-the @var{-n} parameter to skip image creation. This restriction may be relaxed
+exclusive with the @code{-O} parameters. It is currently required to also use
+the @code{-n} parameter to skip image creation. This restriction may be relaxed
 in a future release.
 
 @item --force-share (-U)
@@ -103,13 +103,13 @@ with or without a command shows help and lists the supported formats
 
 @item -p
 display progress bar (compare, convert and rebase commands only).
-If the @var{-p} option is not used for a command that supports it, the
+If the @code{-p} option is not used for a command that supports it, the
 progress is reported when the process receives a @code{SIGUSR1} or
 @code{SIGINFO} signal.
 
 @item -q
 Quiet mode - do not print any output (except errors). There's no progress bar
-in case both @var{-q} and @var{-p} options are used.
+in case both @code{-q} and @code{-p} options are used.
 
 @item -S @var{size}
 indicates the consecutive number of bytes that must contain only zeros
@@ -298,14 +298,14 @@ the top image stays valid).
 Check if two images have the same content. You can compare images with
 different format or settings.
 
-The format is probed unless you specify it by @var{-f} (used for
-@var{filename1}) and/or @var{-F} (used for @var{filename2}) option.
+The format is probed unless you specify it by @code{-f} (used for
+@var{filename1}) and/or @code{-F} (used for @var{filename2}) option.
 
 By default, images with different size are considered identical if the larger
 image contains only unallocated and/or zeroed sectors in the area after the end
 of the other image. In addition, if any sector is not allocated in one image
 and contains only zero bytes in the second one, it is evaluated as equal. You
-can use Strict mode by specifying the @var{-s} option. When compare runs in
+can use Strict mode by specifying the @code{-s} option. When compare runs in
 Strict mode, it fails in case image size differs or a sector is allocated in
 one image and is not allocated in the second one.
 
-- 
2.24.1

Re: [PATCH v2 2/2] doc: Use @code rather than @var for options

[Eric Blake]

On 1/24/20 4:34 AM, David Edmondson wrote:
> Texinfo defines @var for metasyntactic variables and such terms are
> shown in upper-case or italics in the output of makeinfo. When
> considering an option to a command, such as "-n", upper-casing is
> undesirable as it may confuse the reader or be in conflict with the
> equivalent upper-case option.
> 
> Replace the use of @var for options with @code to avoid this.
> 
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
>   qemu-img.texi | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)

Is this patch still needed given Peter's recent push to move to rST 
documentation?

> 
> diff --git a/qemu-img.texi b/qemu-img.texi
> index 3b6dfd8682..6b4a1ac961 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -74,13 +74,13 @@ keys.
>   @item --image-opts
>   Indicates that the source @var{filename} parameter is to be interpreted as a
>   full option string, not a plain filename. This parameter is mutually
> -exclusive with the @var{-f} parameter.
> +exclusive with the @code{-f} parameter.
>   


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 2/2] doc: Use @code rather than @var for options

[David Edmondson]

Eric Blake <eblake@redhat.com> writes:

> On 1/24/20 4:34 AM, David Edmondson wrote:
>> Texinfo defines @var for metasyntactic variables and such terms are
>> shown in upper-case or italics in the output of makeinfo. When
>> considering an option to a command, such as "-n", upper-casing is
>> undesirable as it may confuse the reader or be in conflict with the
>> equivalent upper-case option.
>> 
>> Replace the use of @var for options with @code to avoid this.
>> 
>> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>> ---
>>   qemu-img.texi | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> Is this patch still needed given Peter's recent push to move to rST 
> documentation?

No, it would be obviated by those changes.

>> 
>> diff --git a/qemu-img.texi b/qemu-img.texi
>> index 3b6dfd8682..6b4a1ac961 100644
>> --- a/qemu-img.texi
>> +++ b/qemu-img.texi
>> @@ -74,13 +74,13 @@ keys.
>>   @item --image-opts
>>   Indicates that the source @var{filename} parameter is to be interpreted as a
>>   full option string, not a plain filename. This parameter is mutually
>> -exclusive with the @var{-f} parameter.
>> +exclusive with the @code{-f} parameter.
>>   
>
>
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org

dme.
-- 
Don't you know you're never going to get to France.

Re: [PATCH v2 2/2] doc: Use @code rather than @var for options

[Peter Maydell]

On Tue, 28 Jan 2020 at 07:39, David Edmondson <dme@dme.org> wrote:
>
> Eric Blake <eblake@redhat.com> writes:
>
> > On 1/24/20 4:34 AM, David Edmondson wrote:
> >> Texinfo defines @var for metasyntactic variables and such terms are
> >> shown in upper-case or italics in the output of makeinfo. When
> >> considering an option to a command, such as "-n", upper-casing is
> >> undesirable as it may confuse the reader or be in conflict with the
> >> equivalent upper-case option.
> >>
> >> Replace the use of @var for options with @code to avoid this.
> >>
> >> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> >> ---
> >>   qemu-img.texi | 16 ++++++++--------
> >>   1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > Is this patch still needed given Peter's recent push to move to rST
> > documentation?
>
> No, it would be obviated by those changes.

Yeah; in particular the rST formatting corrects this minor
inconsistency. (I noticed it because my emacs find-and-replace
operations which turn @var{foo} into *FOO* were incorrectly
turning the @var{-s} into *-S*, which they wouldn't have done
if it were @code{-s}...)

thanks
-- PMM