Am 26.01.2016 um 14:34 hat Daniel P. Berrange geschrieben:
> 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 <berra...@redhat.com>
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 9567774..5bb1de7 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -10,68 +10,68 @@ STEXI
> ETEXI
>
> DEF("check", img_check,
> - "check [-q] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache]
> filename")
> + "check [-q] [--object objectdef] [-f fmt] [--output=ofmt] [-r [leaks |
> all]] [-T src_cache] filename")
> STEXI
> -@item check [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T
> @var{src_cache}] @var{filename}
> +@item check [--object objectdef] [-q] [-f @var{fmt}] [--output=@var{ofmt}]
> [-r [leaks | all]] [-T @var{src_cache}] @var{filename}
> ETEXI
s/objectdef/@var{objectdef}/ (in each command)
> @@ -94,6 +98,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"
> + " manual page for a description of the object properties. The
> common object\n"
> + " type that it makes sense to define is the @code{secret}
> object, which is used\n"
> + " to supply passwords and/or encryption keys.\n"
> " 'fmt' is the disk image format. It is guessed automatically in
> most cases\n"
> " 'cache' is the cache mode used to write the output disk image,
> the valid\n"
> " options are: 'none', 'writeback' (default, except for
> convert), 'writethrough',\n"
This one in contrast is printed literally on stdout, so using @code{} is
not a great idea.
> @@ -154,6 +162,34 @@ static void QEMU_NORETURN help(void)
> exit(EXIT_SUCCESS);
> }
>
> +static QemuOptsList qemu_object_opts = {
> + .name = "object",
> + .implied_opt_name = "qom-type",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
> + .desc = {
> + { }
> + },
> +};
> +
> +static int object_create(void *opaque, QemuOpts *opts, Error **errp)
> +{
> + Error *err = NULL;
> + OptsVisitor *ov;
> + QDict *pdict;
> +
> + ov = opts_visitor_new(opts);
> + pdict = qemu_opts_to_qdict(opts, NULL);
> +
> + user_creatable_add(pdict, opts_get_visitor(ov), &err);
> + opts_visitor_cleanup(ov);
> + QDECREF(pdict);
> + if (err) {
> + error_propagate(errp, err);
> + return -1;
> + }
> + return 0;
> +}
> +
> static int GCC_FMT_ATTR(2, 3) qprintf(bool quiet, const char *fmt, ...)
> {
> int ret = 0;
> @@ -273,9 +309,17 @@ static int img_create(int argc, char **argv)
> char *options = NULL;
> Error *local_err = NULL;
> bool quiet = false;
> + QemuOpts *opts;
There are places where we declare variables only used by a specific
option locally with a new block after the case label. This could be
another one for which it is appropriate - it's not used after the option
parsing any more (and it can't be used there because it may still be
uninitialised).
> 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);
> if (c == -1) {
> break;
> }
> @@ -317,6 +361,13 @@ static int img_create(int argc, char **argv)
> case 'q':
> quiet = true;
> break;
> + case OPTION_OBJECT:
> + opts = qemu_opts_parse_noisily(qemu_find_opts("object"),
> + optarg, true);
Any reason for using qemu_add_opts() to register the opts list and then
finding it again by name instead of just using &qemu_object_opts here?
> + if (!opts) {
> + exit(1);
> + }
You seem to introduce a lot of exit(1) calls even where the surrounding
code prefers return 1.
Also, for other patches Eric has been asking to use EXIT_FAILURE instead
of 1 in new code, and I think that makes sense, too.
> + break;
> }
> }
Kevin
