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

["Daniel P. Berrange" <berrange@redhat.com>]

On Tue, Feb 02, 2016 at 05:24:32PM -0700, Eric Blake wrote:
> 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 <berrange@redhat.com>
> > ---
> >  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?

Oh, I didn't realize that it allowed NULL for that parameter.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|