[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V4 2/7] block: Allow the user to define "node-na
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH V4 2/7] block: Allow the user to define "node-name" option. |
Date: |
Mon, 9 Dec 2013 17:15:26 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 05.12.2013 um 18:14 hat Benoît Canet geschrieben:
> Signed-off-by: Benoit Canet <address@hidden>
> ---
> block.c | 44 +++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/block.c b/block.c
> index 4f6b36a..61f5ba0 100644
> --- a/block.c
> +++ b/block.c
> @@ -873,6 +873,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char
> *filename,
> const char *drvname;
> bool allow_protocol_prefix = false;
> Error *local_err = NULL;
> + const char *node_name = NULL;
> int ret;
>
> /* NULL means an empty set of options */
> @@ -880,7 +881,15 @@ int bdrv_file_open(BlockDriverState **pbs, const char
> *filename,
> options = qdict_new();
> }
>
> - bs = bdrv_new("", "");
> + node_name = qdict_get_try_str(options, "node-name");
> + if (node_name && bdrv_find_node(node_name)) {
> + error_setg(errp, "Duplicate node name");
> + QDECREF(options);
> + return -EINVAL;
> + }
> + bs = bdrv_new("", node_name ? node_name : "");
> + qdict_del(options, "node-name");
> +
> bs->options = options;
> options = qdict_clone_shallow(options);
>
> @@ -980,6 +989,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
> *options, Error **errp)
> int back_flags, ret;
> BlockDriver *back_drv = NULL;
> Error *local_err = NULL;
> + const char *node_name = NULL;
>
> if (bs->backing_hd != NULL) {
> QDECREF(options);
> @@ -1002,7 +1012,14 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
> *options, Error **errp)
> sizeof(backing_filename));
> }
>
> - bs->backing_hd = bdrv_new("", "");
> + node_name = qdict_get_try_str(options, "node-name");
> + if (node_name && bdrv_find_node(node_name)) {
> + error_setg(errp, "Duplicate node name");
> + QDECREF(options);
> + return -EINVAL;
> + }
> + bs->backing_hd = bdrv_new("", node_name ? node_name : "");
> + qdict_del(options, "node-name");
>
> if (bs->backing_format[0] != '\0') {
> back_drv = bdrv_find_format(bs->backing_format);
> @@ -1046,6 +1063,7 @@ int bdrv_open(BlockDriverState *bs, const char
> *filename, QDict *options,
> BlockDriverState *file = NULL;
> QDict *file_options = NULL;
> const char *drvname;
> + const char *node_name = NULL;
> Error *local_err = NULL;
>
> /* NULL means an empty set of options */
> @@ -1053,6 +1071,22 @@ int bdrv_open(BlockDriverState *bs, const char
> *filename, QDict *options,
> options = qdict_new();
> }
>
> + node_name = qdict_get_try_str(options, "node-name");
> + if (node_name && bdrv_find_node(node_name)) {
> + error_setg(errp, "Duplicate node name");
> + QDECREF(options);
> + return -EINVAL;
> + }
> +
> + if (node_name) {
> + pstrcpy(bs->node_name, sizeof(bs->node_name), node_name);
> + if (node_name[0] != '\0') {
> + QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list);
> + }
> + }
> +
> + qdict_del(options, "node-name");
We duplicate some code all over the place. In general you seem to be
trying to let the caller of bdrv_new() already figure out what the node
name is by parsing the options QDict; here however, you do it after
bdrv_new().
Can't we consolidate this and only ever set the node name in
bdrv_open_common(), so that the option is parsed only once, there is
only once place adding BDSes to the list, and there is only one place
checking for duplicates?
Kevin