[Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Mike Day]

For the create subcommand the backing file (-b) option is documented
on-line but not in the binary. Add it.

Signed-off-by: Mike Day <ncmike@ncultra.org>
---
 qemu-img-cmds.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index d029609..7724709 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -16,9 +16,9 @@ STEXI
 ETEXI
 
 DEF("create", img_create,
-    "create [-q] [-f fmt] [-o options] filename [size]")
+    "create [-q] [-f fmt] [-b backing-file] [-o options] filename [size]")
 STEXI
-@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [-q] [-f @var{fmt}] [-b @var{backing_file}] [-o @var{options}] @var{filename} [@var{size}]
 ETEXI
 
 DEF("commit", img_commit,
-- 
1.9.0

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Eric Blake]

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

On 05/12/2014 06:35 AM, Mike Day wrote:
> For the create subcommand the backing file (-b) option is documented
> on-line but not in the binary. Add it.
> 
> Signed-off-by: Mike Day <ncmike@ncultra.org>
> ---
>  qemu-img-cmds.hx | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index d029609..7724709 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -16,9 +16,9 @@ STEXI
>  ETEXI
>  
>  DEF("create", img_create,
> -    "create [-q] [-f fmt] [-o options] filename [size]")
> +    "create [-q] [-f fmt] [-b backing-file] [-o options] filename [size]")
>  STEXI
> -@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
> +@item create [-q] [-f @var{fmt}] [-b @var{backing_file}] [-o @var{options}] @var{filename} [@var{size}]

There are two spellings for this:

-b file
-o backing_file=file

is -o considered to be preferred over -b, as an explanation for why it
is not documented?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Kevin Wolf]

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

Am 12.05.2014 um 17:05 hat Eric Blake geschrieben:
> On 05/12/2014 06:35 AM, Mike Day wrote:
> > For the create subcommand the backing file (-b) option is documented
> > on-line but not in the binary. Add it.
> > 
> > Signed-off-by: Mike Day <ncmike@ncultra.org>
> > ---
> >  qemu-img-cmds.hx | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> > index d029609..7724709 100644
> > --- a/qemu-img-cmds.hx
> > +++ b/qemu-img-cmds.hx
> > @@ -16,9 +16,9 @@ STEXI
> >  ETEXI
> >  
> >  DEF("create", img_create,
> > -    "create [-q] [-f fmt] [-o options] filename [size]")
> > +    "create [-q] [-f fmt] [-b backing-file] [-o options] filename [size]")
> >  STEXI
> > -@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
> > +@item create [-q] [-f @var{fmt}] [-b @var{backing_file}] [-o @var{options}] @var{filename} [@var{size}]
> 
> There are two spellings for this:
> 
> -b file
> -o backing_file=file
> 
> is -o considered to be preferred over -b, as an explanation for why it
> is not documented?

Yes, it is. -b was documented before -o backing_file existed. Commit
8063d0fe removed it.

Kevin

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

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Eric Blake]

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

On 05/12/2014 09:13 AM, Kevin Wolf wrote:
> Am 12.05.2014 um 17:05 hat Eric Blake geschrieben:
>> On 05/12/2014 06:35 AM, Mike Day wrote:
>>> For the create subcommand the backing file (-b) option is documented
>>> on-line but not in the binary. Add it.

Online where?  In 'qemu-img --help', or on some web page (at what URL)?

>> is -o considered to be preferred over -b, as an explanation for why it
>> is not documented?
> 
> Yes, it is. -b was documented before -o backing_file existed. Commit
> 8063d0fe removed it.

If anything, that says we need to fix whatever "documented on-line"
source was documenting something that we want to be obsolete; and maybe
tweak qemu-img to be verbose about warning on the use of deprecated
options.  But this patch would go in the wrong direction of reviving
intentionally undocumented code.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Mike Day]

On Mon, May 12, 2014 at 11:20 AM, Eric Blake <eblake@redhat.com> wrote:
> Online where?  In 'qemu-img --help', or on some web page (at what URL)?

http://qemu.weilnetz.de/qemu-doc.html#vm_005fsnapshots

-o backing file is not documented in the help command.

Mike

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Eric Blake]

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

On 05/12/2014 09:36 AM, Mike Day wrote:
> On Mon, May 12, 2014 at 11:20 AM, Eric Blake <eblake@redhat.com> wrote:
>> Online where?  In 'qemu-img --help', or on some web page (at what URL)?
> 
> http://qemu.weilnetz.de/qemu-doc.html#vm_005fsnapshots

Not an official qemu reference; nothing we can do about out-of-date
third-party references.

> 
> -o backing file is not documented in the help command.

Ah, but it is:

$ qemu-img create -f qcow2 -o help
Supported options:
size             Virtual disk size
compat           Compatibility level (0.10 or 1.1)
backing_file     File name of a base image
backing_fmt      Image format of the base image
encryption       Encrypt the image
cluster_size     qcow2 cluster size
preallocation    Preallocation mode (allowed values: off, metadata)
lazy_refcounts   Postpone refcount updates

and more importantly, it only appears in the help output of -f modes
that actually support a backing file.  Contrast:

$ qemu-img create -f raw -o help
Supported options:
size             Virtual disk size


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Mike Day]

On Mon, May 12, 2014 at 11:53 AM, Eric Blake <eblake@redhat.com> wrote:
> Ah, but it is:
>
> $ qemu-img create -f qcow2 -o help
> Supported options:
> size             Virtual disk size
> compat           Compatibility level (0.10 or 1.1)
> backing_file     File name of a base image
> backing_fmt      Image format of the base image
> encryption       Encrypt the image
> cluster_size     qcow2 cluster size
> preallocation    Preallocation mode (allowed values: off, metadata)
> lazy_refcounts   Postpone refcount updates
>
> and more importantly, it only appears in the help output of -f modes
> that actually support a backing file.  Contrast:
>
> $ qemu-img create -f raw -o help
> Supported options:
> size             Virtual disk size

This is much more prominent in the top-level help:

rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F
backing_fmt] filename

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Eric Blake]

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

On 05/12/2014 10:05 AM, Mike Day wrote:
> On Mon, May 12, 2014 at 11:53 AM, Eric Blake <eblake@redhat.com> wrote:
>> Ah, but it is:
>>
>> $ qemu-img create -f qcow2 -o help
>> Supported options:
>> size             Virtual disk size
>> compat           Compatibility level (0.10 or 1.1)
>> backing_file     File name of a base image
>> backing_fmt      Image format of the base image
>> encryption       Encrypt the image
>> cluster_size     qcow2 cluster size
>> preallocation    Preallocation mode (allowed values: off, metadata)
>> lazy_refcounts   Postpone refcount updates
>>
>> and more importantly, it only appears in the help output of -f modes
>> that actually support a backing file.  Contrast:
>>
>> $ qemu-img create -f raw -o help
>> Supported options:
>> size             Virtual disk size
> 
> This is much more prominent in the top-level help:
> 
> rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F
> backing_fmt] filename

'rebase' lacks -o; so it must continue to document -b.  This thread was
started in context to the 'create' subcommand, not the 'rebase'
subcommand.  (Arguably, we may need to add -o to rebase someday, but
that's not for this patch)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Kevin Wolf]

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

Am 12.05.2014 um 18:32 hat Eric Blake geschrieben:
> On 05/12/2014 10:05 AM, Mike Day wrote:
> > On Mon, May 12, 2014 at 11:53 AM, Eric Blake <eblake@redhat.com> wrote:
> >> Ah, but it is:
> >>
> >> $ qemu-img create -f qcow2 -o help
> >> Supported options:
> >> size             Virtual disk size
> >> compat           Compatibility level (0.10 or 1.1)
> >> backing_file     File name of a base image
> >> backing_fmt      Image format of the base image
> >> encryption       Encrypt the image
> >> cluster_size     qcow2 cluster size
> >> preallocation    Preallocation mode (allowed values: off, metadata)
> >> lazy_refcounts   Postpone refcount updates
> >>
> >> and more importantly, it only appears in the help output of -f modes
> >> that actually support a backing file.  Contrast:
> >>
> >> $ qemu-img create -f raw -o help
> >> Supported options:
> >> size             Virtual disk size
> > 
> > This is much more prominent in the top-level help:
> > 
> > rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F
> > backing_fmt] filename
> 
> 'rebase' lacks -o; so it must continue to document -b.  This thread was
> started in context to the 'create' subcommand, not the 'rebase'
> subcommand.  (Arguably, we may need to add -o to rebase someday, but
> that's not for this patch)

What would qemu-img rebase do with -o? It is just for (safely) changing
the backing file, not for updating options. There is qemu-img amend for
that, and it does have an -o option.

Kevin

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

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Eric Blake]

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

On 05/12/2014 10:36 AM, Kevin Wolf wrote:

>>> rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F
>>> backing_fmt] filename
>>
>> 'rebase' lacks -o; so it must continue to document -b.  This thread was
>> started in context to the 'create' subcommand, not the 'rebase'
>> subcommand.  (Arguably, we may need to add -o to rebase someday, but
>> that's not for this patch)
> 
> What would qemu-img rebase do with -o? It is just for (safely) changing
> the backing file, not for updating options. There is qemu-img amend for
> that, and it does have an -o option.

It's a consistency argument.  Why can't we have:

rebase -o backing_file=file,backing_fmt=fmt

similar to create, instead of having to treat rebase as the oddball
command that still takes separate options for two highly related items?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Kevin Wolf]

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

Am 12.05.2014 um 19:02 hat Eric Blake geschrieben:
> On 05/12/2014 10:36 AM, Kevin Wolf wrote:
> 
> >>> rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F
> >>> backing_fmt] filename
> >>
> >> 'rebase' lacks -o; so it must continue to document -b.  This thread was
> >> started in context to the 'create' subcommand, not the 'rebase'
> >> subcommand.  (Arguably, we may need to add -o to rebase someday, but
> >> that's not for this patch)
> > 
> > What would qemu-img rebase do with -o? It is just for (safely) changing
> > the backing file, not for updating options. There is qemu-img amend for
> > that, and it does have an -o option.
> 
> It's a consistency argument.  Why can't we have:
> 
> rebase -o backing_file=file,backing_fmt=fmt
> 
> similar to create, instead of having to treat rebase as the oddball
> command that still takes separate options for two highly related items?

But that's inconsistent with other -o options if you can't say:

    rebase -o lazy_refcounts=on

rebase will always be the oddball because a backing file is not just an
option for it, but it is the whole purpose of the command. Perhaps we
shouldn't have used -b, but just a second non-option argument. But I
don't think changing this now is worth it.

Kevin

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

Re: [Qemu-devel] [PATCH] Add backing file option to qemu-img create help.

[Mike Day]

On Mon, May 12, 2014 at 12:36 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> What would qemu-img rebase do with -o? It is just for (safely) changing
> the backing file, not for updating options. There is qemu-img amend for
> that, and it does have an -o option.

A little background ... I'm writing a 4-day  KVM training course for
the Linux foundation. From time to time I come across undocumented,
buggy, or confusing options and if I can I try to fix them. In the
case of qemu-img I looked in a lot of places for documentation and
ended up at the unofficial web page that documented the -b backing
file option which works just fine for create and rebase. If an
official page had the information I needed in one place instead of
bits and pieces scattered everywhere I would have used it, naturally.

Here's why the help is confusing. On the top level help, all the
options except for -b are documented. A couple subcommands are even
listed in the top level help with their options. I expected that -b
would be documented on the top level with the others. The fact that -b
<basefile> is documented for the rebase subcommand reenforced my
expectation. I was assuming there is some consistency in using options
with backing files. (Especially if the option is called
"backingfile.")

I'm not advocating anything here, and I think its fine to leave things
as they are. I just don't want to be the guy who complains and never
fixes anything.

Mike