The Wednesday 11 Jun 2014 à 16:04:57 (+0200), Kevin Wolf wrote :
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
> block.c | 88
> +++++++++++++++++++++++++++++++++--------------------------------
> 1 file changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/block.c b/block.c
> index effb3e6..b9c2b41 100644
> --- a/block.c
> +++ b/block.c
> @@ -1006,19 +1006,62 @@ free_and_fail:
> return ret;
> }
>
> +static QDict *parse_json_filename(const char *filename, Error **errp)
> +{
> + QObject *options_obj;
> + QDict *options;
> + int ret;
> +
> + ret = strstart(filename, "json:", &filename);
> + assert(ret);
> +
> + options_obj = qobject_from_json(filename);
> + if (!options_obj) {
> + error_setg(errp, "Could not parse the JSON options");
> + return NULL;
> + }
> +
> + if (qobject_type(options_obj) != QTYPE_QDICT) {
> + qobject_decref(options_obj);
> + error_setg(errp, "Invalid JSON object given");
> + return NULL;
> + }
> +
> + options = qobject_to_qdict(options_obj);
> + qdict_flatten(options);
> +
> + return options;
> +}
I am under the impression that this code move could be avoided by using a
function
prototype: the patch would be less cluttered.
> +
> /*
> * Fills in default options for opening images and converts the legacy
> * filename/flags pair to option QDict entries.
> */
> -static int bdrv_fill_options(QDict **options, const char *filename, int
> flags,
> +static int bdrv_fill_options(QDict **options, const char **pfilename, int
> flags,
> Error **errp)
> {
> + const char *filename = *pfilename;
> const char *drvname;
> bool protocol = flags & BDRV_O_PROTOCOL;
> bool parse_filename = false;
> Error *local_err = NULL;
> BlockDriver *drv;
>
> + /* Parse json: pseudo-protocol */
> + if (filename && g_str_has_prefix(filename, "json:")) {
> + QDict *json_options = parse_json_filename(filename, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return -EINVAL;
> + }
> +
> + /* Options given in the filename have lower priority than options
> + * specified directly */
> + qdict_join(*options, json_options, false);
> + QDECREF(json_options);
> + *pfilename = filename = NULL;
> + }
> +
> if (!protocol) {
> return 0;
> }
> @@ -1339,33 +1382,6 @@ out:
> g_free(tmp_filename);
> }
>
> -static QDict *parse_json_filename(const char *filename, Error **errp)
> -{
> - QObject *options_obj;
> - QDict *options;
> - int ret;
> -
> - ret = strstart(filename, "json:", &filename);
> - assert(ret);
> -
> - options_obj = qobject_from_json(filename);
> - if (!options_obj) {
> - error_setg(errp, "Could not parse the JSON options");
> - return NULL;
> - }
> -
> - if (qobject_type(options_obj) != QTYPE_QDICT) {
> - qobject_decref(options_obj);
> - error_setg(errp, "Invalid JSON object given");
> - return NULL;
> - }
> -
> - options = qobject_to_qdict(options_obj);
> - qdict_flatten(options);
> -
> - return options;
> -}
Second part of the uneeded move.
Maybe you could declare bdrv_fill_options below parse_json_filename in the first
patch to avoid the ulterior move.
> -
> /*
> * Opens a disk image (raw, qcow2, vmdk, ...)
> *
> @@ -1429,21 +1445,7 @@ int bdrv_open(BlockDriverState **pbs, const char
> *filename,
> options = qdict_new();
> }
>
> - if (filename && g_str_has_prefix(filename, "json:")) {
> - QDict *json_options = parse_json_filename(filename, &local_err);
> - if (local_err) {
> - ret = -EINVAL;
> - goto fail;
> - }
> -
> - /* Options given in the filename have lower priority than options
> - * specified directly */
> - qdict_join(options, json_options, false);
> - QDECREF(json_options);
> - filename = NULL;
> - }
> -
> - ret = bdrv_fill_options(&options, filename, flags, &local_err);
> + ret = bdrv_fill_options(&options, &filename, flags, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return ret;
> --
> 1.8.3.1
>
>
