From patchwork Tue Feb 18 17:33:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 321601 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48BF92C00D5 for ; Wed, 19 Feb 2014 05:01:13 +1100 (EST) Received: from localhost ([::1]:52501 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFoyk-0001G9-Vq for incoming@patchwork.ozlabs.org; Tue, 18 Feb 2014 13:01:10 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43158) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFoYN-0000ZD-Cb for qemu-devel@nongnu.org; Tue, 18 Feb 2014 12:34:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFoY0-00051p-0J for qemu-devel@nongnu.org; Tue, 18 Feb 2014 12:33:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:24965) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFoXz-00051C-PM for qemu-devel@nongnu.org; Tue, 18 Feb 2014 12:33:31 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1IHVS5N018845 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 18 Feb 2014 12:31:28 -0500 Received: from localhost (ovpn-116-56.ams2.redhat.com [10.36.116.56]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s1IHVNGQ020808 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Tue, 18 Feb 2014 12:31:26 -0500 From: Max Reitz To: qemu-devel@nongnu.org Date: Tue, 18 Feb 2014 18:33:11 +0100 Message-Id: <1392744792-9512-8-git-send-email-mreitz@redhat.com> In-Reply-To: <1392744792-9512-1-git-send-email-mreitz@redhat.com> References: <1392744792-9512-1-git-send-email-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , =?UTF-8?q?Beno=C3=AEt=20Canet?= , Stefan Hajnoczi , Max Reitz Subject: [Qemu-devel] [PATCH v4 7/8] block: Reuse success path from bdrv_open() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 Reviewed-by: Kevin Wolf Reviewed-by: Benoit Canet --- block.c | 63 +++++++++++++++++++++++++++++---------------------------------- 1 file changed, 29 insertions(+), 34 deletions(-) diff --git a/block.c b/block.c index 7e50e7a..71ed977 100644 --- a/block.c +++ b/block.c @@ -955,13 +955,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; @@ -971,9 +973,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 " @@ -983,13 +985,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) { @@ -1008,41 +1010,30 @@ 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"); } 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; } @@ -1253,12 +1244,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 { @@ -1395,12 +1384,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;