diff mbox

[v2,3/3] qemu-img: Document backing options

Message ID 20170426134649.9042-4-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Max Reitz April 26, 2017, 1:46 p.m. UTC
The create and convert subcommands have shorthands to set the
backing_file and, in the case of create, the backing_fmt options for the
new image. However, they have not been documented so far, which is
remedied by this patch.

Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img-cmds.hx | 8 ++++----
 qemu-img.texi    | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Eric Blake April 26, 2017, 2:20 p.m. UTC | #1
On 04/26/2017 08:46 AM, Max Reitz wrote:
> The create and convert subcommands have shorthands to set the
> backing_file and, in the case of create, the backing_fmt options for the
> new image. However, they have not been documented so far, which is
> remedied by this patch.
> 
> Reported-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img-cmds.hx | 8 ++++----
>  qemu-img.texi    | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Kevin Wolf April 27, 2017, 3:25 p.m. UTC | #2
Am 26.04.2017 um 15:46 hat Max Reitz geschrieben:
> The create and convert subcommands have shorthands to set the
> backing_file and, in the case of create, the backing_fmt options for the
> new image. However, they have not been documented so far, which is
> remedied by this patch.
> 
> Reported-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

I think originally the idea was to remove the individual special case
options in the long term in favour of the uniform -o. But I don't think
the short options have fallen out of use, so we can as well document
them again...

Maybe we could change the implementation, however, so that the old
options are really just aliases for the respective -o version.

Kevin
Eric Blake April 27, 2017, 3:36 p.m. UTC | #3
On 04/26/2017 08:46 AM, Max Reitz wrote:
> The create and convert subcommands have shorthands to set the
> backing_file and, in the case of create, the backing_fmt options for the
> new image. However, they have not been documented so far, which is
> remedied by this patch.
> 

> +++ b/qemu-img.texi
> @@ -224,7 +224,7 @@ If @code{-r} is specified, exit codes representing the image state refer to the
>  state after (the attempt at) repairing it. That is, a successful @code{-r all}
>  will yield the exit code 0, independently of the image state before.
>  
> -@item create [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
> +@item create [-f @var{fmt}] [-b @var{backing_file}] [-F @var{backing_fmt}] [-o @var{options}] @var{filename} [@var{size}]
>  
>  Create the new disk image @var{filename} of size @var{size} and format
>  @var{fmt}. Depending on the file format, you can add one or more @var{options}
> @@ -302,7 +302,7 @@ Error on reading data
>  
>  @end table
>  
> -@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-m @var{num_coroutines}] [-W] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
> +@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-m @var{num_coroutines}] [-W] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
>  
>  Convert the disk image @var{filename} or a snapshot @var{snapshot_param}(@var{snapshot_id_or_name} is deprecated)
>  to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}


On 04/27/2017 10:25 AM, Kevin Wolf wrote:

> I think originally the idea was to remove the individual special case
> options in the long term in favour of the uniform -o. But I don't think
> the short options have fallen out of use, so we can as well document
> them again...
>
> Maybe we could change the implementation, however, so that the old
> options are really just aliases for the respective -o version.

As in, a followup patch that adds a paragraph or two to the .texi file
stating that '-B @var{backing_file}' is shorthand for '-o
backing_file=@var{backing_file}'?
Kevin Wolf April 28, 2017, 9:38 a.m. UTC | #4
Am 27.04.2017 um 17:36 hat Eric Blake geschrieben:
> On 04/26/2017 08:46 AM, Max Reitz wrote:
> > I think originally the idea was to remove the individual special case
> > options in the long term in favour of the uniform -o. But I don't think
> > the short options have fallen out of use, so we can as well document
> > them again...
> >
> > Maybe we could change the implementation, however, so that the old
> > options are really just aliases for the respective -o version.
> 
> As in, a followup patch that adds a paragraph or two to the .texi file
> stating that '-B @var{backing_file}' is shorthand for '-o
> backing_file=@var{backing_file}'?

First and foremost, I meant in the C code where we have separate
variables for all the shorthand options and then need to handle both
ways explicitly in the code further down. Instead, -B could just
directly add a 'backing_file' key to a QemuOpts (and -o would do the
same instead of building a string that is parsed only later).

But the documentation change that you're suggesting is probably a good
idea as well.

Kevin
Max Reitz April 28, 2017, 2:03 p.m. UTC | #5
On 28.04.2017 11:38, Kevin Wolf wrote:
> Am 27.04.2017 um 17:36 hat Eric Blake geschrieben:
>> On 04/26/2017 08:46 AM, Max Reitz wrote:
>>> I think originally the idea was to remove the individual special case
>>> options in the long term in favour of the uniform -o. But I don't think
>>> the short options have fallen out of use, so we can as well document
>>> them again...

Hm, I'm not really sure what the use of deprecating them would be. We
would be able to use the single-letter options -b/-B/-F for something
else, but would we really want to do that...?

(And it should always be trivial to map these legacy parameters to
normal options.)

>>> Maybe we could change the implementation, however, so that the old
>>> options are really just aliases for the respective -o version.
>>
>> As in, a followup patch that adds a paragraph or two to the .texi file
>> stating that '-B @var{backing_file}' is shorthand for '-o
>> backing_file=@var{backing_file}'?
> 
> First and foremost, I meant in the C code where we have separate
> variables for all the shorthand options and then need to handle both
> ways explicitly in the code further down. Instead, -B could just
> directly add a 'backing_file' key to a QemuOpts (and -o would do the
> same instead of building a string that is parsed only later).

We already do that more or less for convert, actually (for -B, that is,
not for -o). We just have to keep out_baseimg separately because we
won't have a QemuOpts with -n.

Using add_old_style_options() in img_create() should be easy enough.

> But the documentation change that you're suggesting is probably a good
> idea as well.

Well, I more or less deliberately did not really document -b/-B/-F. I
just documented them in the syntax overview and named the parameters
backing_file and backing_fmt, both of which are valid options and are
*as such* documented in the explanatory text. So from my point of view,
it's already documented to be a shorthand.

Max
diff mbox

Patch

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 8ac78222af..bf4ce59019 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@  STEXI
 ETEXI
 
 DEF("create", img_create,
-    "create [-q] [--object objectdef] [-f fmt] [-o options] filename [size]")
+    "create [-q] [--object objectdef] [-f fmt] [-b backing_file] [-F backing_fmt] [-o options] filename [size]")
 STEXI
-@item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-b @var{backing_file}] [-F @var{backing_fmt}] [-o @var{options}] @var{filename} [@var{size}]
 ETEXI
 
 DEF("commit", img_commit,
@@ -40,9 +40,9 @@  STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [-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}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("dd", img_dd,
diff --git a/qemu-img.texi b/qemu-img.texi
index c81db3e81c..8c573ae010 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -224,7 +224,7 @@  If @code{-r} is specified, exit codes representing the image state refer to the
 state after (the attempt at) repairing it. That is, a successful @code{-r all}
 will yield the exit code 0, independently of the image state before.
 
-@item create [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [-f @var{fmt}] [-b @var{backing_file}] [-F @var{backing_fmt}] [-o @var{options}] @var{filename} [@var{size}]
 
 Create the new disk image @var{filename} of size @var{size} and format
 @var{fmt}. Depending on the file format, you can add one or more @var{options}
@@ -302,7 +302,7 @@  Error on reading data
 
 @end table
 
-@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-m @var{num_coroutines}] [-W] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-m @var{num_coroutines}] [-W] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_param}(@var{snapshot_id_or_name} is deprecated)
 to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}