[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Improve driver whitelist checks
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH] block: Improve driver whitelist checks |
Date: |
Thu, 10 Oct 2013 20:47:43 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> ---
> 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 <address@hidden>