Re: [Qemu-devel] [PATCH 13/34] qemu-io: Add command 'reopen'

[Max Reitz <mreitz@redhat.com>]

On 08.05.2015 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qemu-io-cmds.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 71 insertions(+)
>
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 1afcfc0..ef8f3fd 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1978,6 +1978,76 @@ static const cmdinfo_t map_cmd = {
>          .oneline        = "prints the allocated areas of a file",
>   };
>   
> +static int reopen_f(BlockBackend *blk, int argc, char **argv);
> +
> +static QemuOptsList reopen_opts = {
> +    .name = "reopen",
> +    .merge_lists = true,
> +    .head = QTAILQ_HEAD_INITIALIZER(reopen_opts.head),
> +    .desc = {
> +        /* no elements => accept any params */
> +        { /* end of list */ }
> +    },
> +};
> +
> +static const cmdinfo_t reopen_cmd = {
> +       .name           = "reopen",
> +       .argmin         = 0,
> +       .argmax         = -1,
> +       .cfunc          = reopen_f,
> +       .args           = "",

Why none? "[-c cache] [-o options] [-r]" would be pretty helpful, I reckon.

> +       .oneline        = "reopens an image with new options",

No .help? Awww. (Fine, as long as .args isn't empty)

> +};
> +
> +static int reopen_f(BlockBackend *blk, int argc, char **argv)
> +{
> +    BlockDriverState *bs = blk_bs(blk);
> +    QemuOpts *qopts;
> +    QDict *opts;
> +    int c;
> +    int flags = bs->open_flags;
> +
> +    BlockReopenQueue *brq;
> +    Error *local_err = NULL;
> +
> +    while ((c = getopt(argc, argv, "c:o:r")) != EOF) {
> +        switch (c) {
> +        case 'c':
> +            if (bdrv_parse_cache_flags(optarg, &flags) < 0) {
> +                error_report("Invalid cache option: %s", optarg);
> +                return 0;
> +            }
> +            break;
> +        case 'o':
> +            if (!qemu_opts_parse(&reopen_opts, optarg, 0)) {
> +                printf("could not parse option list -- %s\n", optarg);
> +                qemu_opts_reset(&reopen_opts);
> +                return 0;
> +            }
> +            break;
> +        case 'r':
> +            flags &= ~BDRV_O_RDWR;
> +            break;
> +        default:
> +            qemu_opts_reset(&reopen_opts);
> +            return qemuio_command_usage(&reopen_cmd);
> +        }
> +    }

Should I be getting an error for "reopen foo", that is, excess arguments?

> +
> +    qopts = qemu_opts_find(&reopen_opts, NULL);
> +    opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
> +    qemu_opts_reset(&reopen_opts);
> +
> +    brq = bdrv_reopen_queue(NULL, bs, opts, flags);
> +    bdrv_reopen_multiple(brq, &local_err);
> +    if (local_err) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);

How about error_report_err() instead?

Max

> +    }
> +
> +    return 0;
> +}
> +
>   static int break_f(BlockBackend *blk, int argc, char **argv)
>   {
>       int ret;
> @@ -2265,6 +2335,7 @@ static void __attribute((constructor)) init_qemuio_commands(void)
>       qemuio_add_command(&discard_cmd);
>       qemuio_add_command(&alloc_cmd);
>       qemuio_add_command(&map_cmd);
> +    qemuio_add_command(&reopen_cmd);
>       qemuio_add_command(&break_cmd);
>       qemuio_add_command(&remove_break_cmd);
>       qemuio_add_command(&resume_cmd);