From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33069) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCBqp-0003no-PF for qemu-devel@nongnu.org; Sat, 08 Feb 2014 12:38:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WCBqi-0005Uo-GQ for qemu-devel@nongnu.org; Sat, 08 Feb 2014 12:37:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:26193) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCBqi-0005Ua-7w for qemu-devel@nongnu.org; Sat, 08 Feb 2014 12:37:52 -0500 From: Max Reitz Date: Sat, 8 Feb 2014 18:39:18 +0100 Message-Id: <1391881159-13585-8-git-send-email-mreitz@redhat.com> In-Reply-To: <1391881159-13585-1-git-send-email-mreitz@redhat.com> References: <1391881159-13585-1-git-send-email-mreitz@redhat.com> Subject: [Qemu-devel] [PATCH v2 7/8] block: Reuse success path from bdrv_open() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Kevin Wolf , Jeff Cody , =?UTF-8?q?Beno=C3=AEt=20Canet?= , Stefan Hajnoczi , Max Reitz The fail and success paths of bdrv_file_open() may be further shortened by reusing code already existent in bdrv_open(). This includes bdrv_file_open() not taking the reference to options which allows the removal of QDECREF(options) in that function. Signed-off-by: Max Reitz --- block.c | 68 ++++++++++++++++++++++++++++++++--------------------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/block.c b/block.c index 1fcd7e0..22e5a44 100644 --- a/block.c +++ b/block.c @@ -948,13 +948,15 @@ free_and_fail: /* * Opens a file using a protocol (file, host_device, nbd, ...) * - * options is a QDict of options to pass to the block drivers, or NULL for an - * empty set of options. The reference to the QDict belongs to the block layer - * after the call (even on failure), so if the caller intends to reuse the - * dictionary, it needs to use QINCREF() before calling bdrv_file_open. + * options is an indirect pointer to a QDict of options to pass to the block + * drivers, or pointer to NULL for an empty set of options. If this function + * takes ownership of the QDict reference, it will set *options to NULL; + * otherwise, it will contain unused/unrecognized options after this function + * returns. Then, the caller is responsible for freeing it. If it intends to + * reuse the QDict, QINCREF() should be called beforehand. */ static int bdrv_file_open(BlockDriverState *bs, const char *filename, - QDict *options, int flags, Error **errp) + QDict **options, int flags, Error **errp) { BlockDriver *drv; const char *drvname; @@ -964,9 +966,9 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename, /* Fetch the file name from the options QDict if necessary */ if (!filename) { - filename = qdict_get_try_str(options, "filename"); - } else if (filename && !qdict_haskey(options, "filename")) { - qdict_put(options, "filename", qstring_from_str(filename)); + filename = qdict_get_try_str(*options, "filename"); + } else if (filename && !qdict_haskey(*options, "filename")) { + qdict_put(*options, "filename", qstring_from_str(filename)); allow_protocol_prefix = true; } else { error_setg(errp, "Can't specify 'file' and 'filename' options at the " @@ -976,13 +978,13 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename, } /* Find the right block driver */ - drvname = qdict_get_try_str(options, "driver"); + drvname = qdict_get_try_str(*options, "driver"); if (drvname) { drv = bdrv_find_format(drvname); if (!drv) { error_setg(errp, "Unknown driver '%s'", drvname); } - qdict_del(options, "driver"); + qdict_del(*options, "driver"); } else if (filename) { drv = bdrv_find_protocol(filename, allow_protocol_prefix); if (!drv) { @@ -1001,41 +1003,35 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename, /* Parse the filename and open it */ if (drv->bdrv_parse_filename && filename) { - drv->bdrv_parse_filename(filename, options, &local_err); + drv->bdrv_parse_filename(filename, *options, &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); ret = -EINVAL; goto fail; } - qdict_del(options, "filename"); + qdict_del(*options, "filename"); + } else if (drv->bdrv_needs_filename && !filename) { + error_setg(errp, "The '%s' block driver requires a file name", + drv->format_name); + ret = -EINVAL; + goto fail; } if (!drv->bdrv_file_open) { - ret = bdrv_open(&bs, filename, NULL, options, flags, drv, &local_err); - options = NULL; + ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err); + *options = NULL; } else { - ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err); + ret = bdrv_open_common(bs, NULL, *options, flags, drv, &local_err); } if (ret < 0) { error_propagate(errp, local_err); goto fail; } - /* Check if any unknown options were used */ - if (options && (qdict_size(options) != 0)) { - const QDictEntry *entry = qdict_first(options); - error_setg(errp, "Block protocol '%s' doesn't support the option '%s'", - drv->format_name, entry->key); - ret = -EINVAL; - goto fail; - } - QDECREF(options); - bs->growable = 1; return 0; fail: - QDECREF(options); return ret; } @@ -1242,12 +1238,10 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, if (flags & BDRV_O_PROTOCOL) { assert(!drv); - ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL, + ret = bdrv_file_open(bs, filename, &options, flags & ~BDRV_O_PROTOCOL, &local_err); - options = NULL; if (!ret) { - *pbs = bs; - return 0; + goto done; } else if (bs->drv) { goto close_and_fail; } else { @@ -1384,12 +1378,18 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } } +done: /* Check if any unknown options were used */ - if (qdict_size(options) != 0) { + if (options && (qdict_size(options) != 0)) { const QDictEntry *entry = qdict_first(options); - error_setg(errp, "Block format '%s' used by device '%s' doesn't " - "support the option '%s'", drv->format_name, bs->device_name, - entry->key); + if (flags & BDRV_O_PROTOCOL) { + error_setg(errp, "Block protocol '%s' doesn't support the option " + "'%s'", drv->format_name, entry->key); + } else { + error_setg(errp, "Block format '%s' used by device '%s' doesn't " + "support the option '%s'", drv->format_name, + bs->device_name, entry->key); + } ret = -EINVAL; goto close_and_fail; -- 1.8.5.4