[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 18/24] block: Initial implementation of -blockde
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 18/24] block: Initial implementation of -blockdev |
Date: |
Tue, 28 Feb 2017 20:57:22 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 27.02.2017 um 12:20 hat Markus Armbruster geschrieben:
> The new command line option -blockdev works like QMP command
> blockdev-add.
>
> The option argument may be given in JSON syntax, exactly as in QMP.
> Example usage:
>
> -blockdev '{"node-name": "foo", "driver": "raw", "file": {"driver":
> "file", "filename": "foo.img"} }'
>
> The JSON argument doesn't exactly blend into the existing option
> syntax, so the traditional KEY=VALUE,... syntax is also supported,
> using dotted keys to do the nesting:
>
> -blockdev node-name=foo,driver=raw,file.driver=file,file.filename=foo.img
>
> This does not yet support lists or downstream extensions, i.e. keys
> with __RFQDN_ prefix, but the next few patches will take care of that.
>
> Note that calling qmp_blockdev_add() (say via qmp_marshal_block_add())
> right away would crash. We need to stash the configuration for later
> instead. This is crudely done, and bypasses QemuOpts, even though
> storing configuration is what QemuOpts is for. Need to revamp option
> infrastructure to support QAPI types like BlockdevOptions.
>
> Signed-off-by: Markus Armbruster <address@hidden>
Apparently I forgot to actually reply here after going back to the
previous patch...
> @@ -532,6 +532,13 @@ Use @var{file} as CD-ROM image (you cannot use
> @option{-hdc} and
> using @file{/dev/cdrom} as filename (@pxref{host_drives}).
> ETEXI
>
> +DEF("blockdev", HAS_ARG, QEMU_OPTION_blockdev,
> + "-blockdev driver[,node-name=N][,discard=ignore|unmap]\n"
I was going to suggest the same thing as Eric here, simply 'driver'
doesn't look right.
> + " [,cache.direct=on|off][,cache.no-flush=on|off]\n"
> + " [,read-only=on|off][,detect-zeroes=on|off|unmap]\n"
> + " [,driver specific parameters...]\n"
> + " configure a block backend\n", QEMU_ARCH_ALL)
> +
> DEF("drive", HAS_ARG, QEMU_OPTION_drive,
> "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
> " [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
> @@ -4454,6 +4483,16 @@ int main(int argc, char **argv, char **envp)
> qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
> NULL, NULL);
> }
> + while (!QSIMPLEQ_EMPTY(&bdo_queue)) {
> + BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(&bdo_queue);
> +
> + QSIMPLEQ_REMOVE_HEAD(&bdo_queue, entry);
> + loc_push_restore(&bdo->loc);
> + qmp_blockdev_add(bdo->bdo, &error_fatal);
> + loc_pop(&bdo->loc);
> + qapi_free_BlockdevOptions(bdo->bdo);
> + g_free(bdo);
> + }
> if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
> &machine_class->block_default_type, NULL)) {
> exit(1);
And here I found the placement of the new code odd, it's in the middle
of -drive handling. The current placement means that -drive can
reference -blockdev nodes, but not the other way round. I like this and
wouldn't recommand changing it. So my suggestion for a better placement
would be to move the -blockdev code up a bit.
Kevin
- Re: [Qemu-devel] [PATCH 14/24] check-qjson: Test errors from qobject_from_json(), (continued)
- [Qemu-devel] [PATCH 09/24] libqtest: Fix qmp() & friends to abort on JSON parse errors, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 21/24] test-qapi-util: New, covering qapi/qapi-util.c, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 18/24] block: Initial implementation of -blockdev, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 17/24] qapi: New qobject_input_visitor_new_str() for convenience, Markus Armbruster, 2017/02/27
- Re: [Qemu-devel] [PATCH 17/24] qapi: New qobject_input_visitor_new_str() for convenience, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH 02/24] tests: Fix gcov-files-test-qemu-opts-y, gcov-files-test-logging-y, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 08/24] qobject: Propagate parse errors through qobject_from_jsonv(), Markus Armbruster, 2017/02/27