Re: [Qemu-devel] [PATCH v5 02/10] qemu-img: add support for --object command line arg

[Eric Blake]

On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> Allow creation of user creatable object types with qemu-img
> via a new --object command line arg. This will be used to supply
> passwords and/or encryption keys to the various block driver
> backends via the recently added 'secret' object type.
> 
>  # printf letmein > mypasswd.txt
>  # qemu-img info --object secret,id=sec0,file=mypasswd.txt \
>       ...other info args...
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  qemu-img-cmds.hx |  44 ++++-----
>  qemu-img.c       | 269 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  qemu-img.texi    |   8 ++
>  3 files changed, 291 insertions(+), 30 deletions(-)
> 

> +++ b/qemu-img.c
> @@ -94,6 +97,10 @@ static void QEMU_NORETURN help(void)
>             "\n"
>             "Command parameters:\n"
>             "  'filename' is a disk image filename\n"
> +           "  'objectdef' is a QEMU user creatable object definition. See 
> the @code{qemu(1)}\n"

Drop @code; this is the --help text.

> +           "    manual page for a description of the object properties. The 
> common object\n"
> +           "    type that it makes sense to define is 'secret' object, which 
> is used to\n"

s/is/is a/

or maybe go for something shorter:

The most common object type is a 'secret', which is used...

or match the text you put in the info:

The only object type that it makes sense to define is the 'secret'
object, which is used...

> @@ -275,7 +291,14 @@ static int img_create(int argc, char **argv)
>      bool quiet = false;
>  
>      for(;;) {
> -        c = getopt(argc, argv, "F:b:f:he6o:q");
> +        int option_index = 0;
> +        static const struct option long_options[] = {
> +            {"help", no_argument, 0, 'h'},
> +            {"object", required_argument, 0, OPTION_OBJECT},
> +            {0, 0, 0, 0}
> +        };
> +        c = getopt_long(argc, argv, "F:b:f:he6o:q",
> +                        long_options, &option_index);

Can't you pass NULL for the last parameter, if you aren't going to use
option_index in your error reporting?

> @@ -675,7 +730,14 @@ static int img_commit(int argc, char **argv)
>      cache = BDRV_DEFAULT_CACHE;
>      base = NULL;
>      for(;;) {
> -        c = getopt(argc, argv, "f:ht:b:dpq");
> +        int option_index = 0;
> +        static const struct option long_options[] = {
> +            {"help", no_argument, 0, 'h'},
> +            {"object", required_argument, 0, OPTION_OBJECT},
> +            {0, 0, 0, 0}
> +        };
> +        c = getopt_long(argc, argv, "f:ht:b:dpq",
> +                        long_options, &option_index);

more than once. I'll quit pointing it out.  Doesn't affect behavior
either way.

> +++ b/qemu-img.texi
> @@ -24,6 +24,14 @@ Command parameters:
>  @table @var
>  @item filename
>   is a disk image filename
> +
> address@hidden --object @var{objectdef}
> +
> +is a QEMU user creatable object definition. See the @code{qemu(1)} manual
> +page for a description of the object properties. The only object type that
> +it makes sense to define is the @code{secret} object, which is used to
> +supply passwords and/or encryption keys.

With the help text fixed,
Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature