From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59651) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgEsX-0004Br-7y for qemu-devel@nongnu.org; Mon, 20 Oct 2014 11:28:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XgEsS-0007c5-9Y for qemu-devel@nongnu.org; Mon, 20 Oct 2014 11:28:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41214) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgEsS-0007c0-1z for qemu-devel@nongnu.org; Mon, 20 Oct 2014 11:28:08 -0400 Message-ID: <544529F8.7000305@redhat.com> Date: Mon, 20 Oct 2014 17:27:52 +0200 From: Max Reitz MIME-Version: 1.0 References: <1413815720-29976-1-git-send-email-pl@kamp.de> <1413815720-29976-3-git-send-email-pl@kamp.de> In-Reply-To: <1413815720-29976-3-git-send-email-pl@kamp.de> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/4] block: introduce bdrv_runtime_opts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven , qemu-devel@nongnu.org Cc: kwolf@redhat.com, jcody@redhat.com, famz@redhat.com, benoit@irqsave.net, stefanha@redhat.com On 20.10.2014 at 16:35, Peter Lieven wrote: > This patch (orginally by Kevin) adds a bdrv_runtime_opts QemuOptsList. > The list will absorb all options that belong to the BDS (and not the > BlockBackend) and will be parsed and handled in bdrv_open_common. > > Signed-off-by: Kevin Wolf > Signed-off-by: Peter Lieven > --- > block.c | 39 +++++++++++++++++++++++++++++++++------ > 1 file changed, 33 insertions(+), 6 deletions(-) > > diff --git a/block.c b/block.c > index ba5281d..3b4a59a 100644 > --- a/block.c > +++ b/block.c > @@ -27,6 +27,7 @@ > #include "block/block_int.h" > #include "block/blockjob.h" > #include "qemu/module.h" > +#include "qapi/qmp/qbool.h" > #include "qapi/qmp/qjson.h" > #include "sysemu/block-backend.h" > #include "sysemu/sysemu.h" > @@ -875,6 +876,19 @@ static void bdrv_assign_node_name(BlockDriverState *bs, > QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list); > } > > +static QemuOptsList bdrv_runtime_opts = { > + .name = "bdrv_common", > + .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head), > + .desc = { > + { > + .name = "node-name", > + .type = QEMU_OPT_STRING, > + .help = "Node name of the block device node", > + }, > + { /* end of list */ } > + }, > +}; > + > /* > * Common part for opening disk images and files > * > @@ -886,6 +900,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, > int ret, open_flags; > const char *filename; > const char *node_name = NULL; > + QemuOpts *opts; > Error *local_err = NULL; > > assert(drv != NULL); > @@ -906,19 +921,28 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, > > trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name); > > - node_name = qdict_get_try_str(options, "node-name"); > + opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort); > + qemu_opts_absorb_qdict(opts, options, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + ret = -EINVAL; > + goto fail_opts; > + } > + > + node_name = qemu_opt_get(opts, "node-name"); > bdrv_assign_node_name(bs, node_name, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return -EINVAL; > + ret = -EINVAL; > + goto fail_opts; > } > - qdict_del(options, "node-name"); > > /* bdrv_open() with directly using a protocol as drv. This layer is already > * opened, so assign it to bs (while file becomes a closed BlockDriverState) > * and return immediately. */ > if (file != NULL && drv->bdrv_file_open) { > bdrv_swap(file, bs); > + qemu_opts_del(opts); > return 0; > } > > @@ -936,7 +960,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, > ? "Driver '%s' can only be used for read-only devices" > : "Driver '%s' is not whitelisted", > drv->format_name); > - return -ENOTSUP; > + ret = -ENOTSUP; > + goto fail_opts; > } > > assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */ > @@ -945,7 +970,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, > bdrv_enable_copy_on_read(bs); > } else { > error_setg(errp, "Can't use copy-on-read on read-only device"); > - return -EINVAL; > + ret = -EINVAL; > + goto fail_opts; > } > } > > @@ -958,7 +984,6 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, > > bs->drv = drv; > bs->opaque = g_malloc0(drv->instance_size); > - > bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB); > > /* Open the image, either directly or using a protocol */ I know it's not even your patch originally, but I still don't like spurious empty line removals, whoever might be responsible for them. > @@ -1010,6 +1035,8 @@ free_and_fail: > g_free(bs->opaque); > bs->opaque = NULL; > bs->drv = NULL; > +fail_opts: > + qemu_opts_del(opts); > return ret; > } > With that empty line removal hunk removed: Reviewed-by: Max Reitz