From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35390) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VUFej-0003Qy-9n for qemu-devel@nongnu.org; Thu, 10 Oct 2013 08:47:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VUFed-0005ZY-Aq for qemu-devel@nongnu.org; Thu, 10 Oct 2013 08:47:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4228) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VUFec-0005ZH-OJ for qemu-devel@nongnu.org; Thu, 10 Oct 2013 08:47:47 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r9ACljFt024011 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 10 Oct 2013 08:47:46 -0400 Date: Thu, 10 Oct 2013 20:47:43 +0800 From: Fam Zheng Message-ID: <20131010124743.GA21583@T430s.nay.redhat.com> References: <1381399079-17826-1-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1381399079-17826-1-git-send-email-kwolf@redhat.com> Subject: Re: [Qemu-devel] [PATCH] block: Improve driver whitelist checks Reply-To: famz@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com On Thu, 10/10 11:57, Kevin Wolf wrote: > The main intent of this patch is to consolidate the whitelist checks to > a single point in the code instead of spreading it everywhere. This adds > a nicer error message for read-only whitelisting, too, in places where > it was still missing. > > The patch also contains a bonus bug fix: By finding the format first in > bdrv_open() and then independently checking against the whitelist only > later, we avoid the case that use of a non-whitelisted format results in > probing rather than an error message. Previously, this could happen when > using the driver=... option. > > Signed-off-by: Kevin Wolf > --- > block.c | 10 +++++++--- > blockdev.c | 8 ++------ > 2 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/block.c b/block.c > index beea027..84c0eac 100644 > --- a/block.c > +++ b/block.c > @@ -769,7 +769,11 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, > bs->read_only = !(open_flags & BDRV_O_RDWR); > > if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) { > - error_setg(errp, "Driver '%s' is not whitelisted", drv->format_name); > + error_setg(errp, > + !bs->read_only && bdrv_is_whitelisted(drv, true) > + ? "Driver '%s' can only be used for read-only devices" > + : "Driver '%s' is not whitelisted", > + drv->format_name); > return -ENOTSUP; > } > > @@ -881,7 +885,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, > /* Find the right block driver */ > drvname = qdict_get_try_str(options, "driver"); > if (drvname) { > - drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR)); > + drv = bdrv_find_format(drvname); > if (!drv) { > error_setg(errp, "Unknown driver '%s'", drvname); > } > @@ -1123,7 +1127,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, > /* Find the right image format driver */ > drvname = qdict_get_try_str(options, "driver"); > if (drvname) { > - drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR)); > + drv = bdrv_find_format(drvname); > qdict_del(options, "driver"); > } > > diff --git a/blockdev.c b/blockdev.c > index 92029d8..5f3cece 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -468,13 +468,9 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, > return NULL; > } > > - drv = bdrv_find_whitelisted_format(buf, ro); > + drv = bdrv_find_format(buf); > if (!drv) { > - if (!ro && bdrv_find_whitelisted_format(buf, !ro)) { > - error_report("'%s' can be only used as read-only device.", buf); > - } else { > - error_report("'%s' invalid format", buf); > - } > + error_report("'%s' invalid format", buf); > return NULL; > } > } This is much cleaner now. Thanks. Reviewed-by: Fam Zheng