The Wednesday 11 Jun 2014 à 16:04:59 (+0200), Kevin Wolf wrote :
> This moves the bdrv_open_file() call a bit down so that it can use the
> bdrv_open() code that selects the right block driver.
>
> The code between the old and the new call site is either common code
> (the error message for an unknown driver has been unified now) or
> doesn't run with cleared BDRV_O_PROTOCOL (added an if block in one
> place, whereas the right path was already asserted in another place)
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
> block.c | 71
> ++++++++++++++++++++++++++++-------------------------------------
> 1 file changed, 30 insertions(+), 41 deletions(-)
>
> diff --git a/block.c b/block.c
> index 011cfc4..2aa0a56 100644
> --- a/block.c
> +++ b/block.c
> @@ -1136,25 +1136,14 @@ static int bdrv_fill_options(QDict **options, const
> char **pfilename, int flags,
> * 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, QDict **options, int flags,
> - Error **errp)
> +static int bdrv_file_open(BlockDriverState *bs, BlockDriver *drv,
> + QDict **options, int flags, Error **errp)
> {
> - BlockDriver *drv;
> const char *filename;
> - const char *drvname;
> Error *local_err = NULL;
> int ret;
>
> filename = qdict_get_try_str(*options, "filename");
> - drvname = qdict_get_str(*options, "driver");
> -
> - drv = bdrv_find_format(drvname);
> - if (!drv) {
> - error_setg(errp, "Unknown driver '%s'", drvname);
> - ret = -ENOENT;
> - goto fail;
> - }
> - qdict_del(*options, "driver");
>
> /* Open the file */
> if (!drv->bdrv_file_open) {
> @@ -1460,35 +1449,23 @@ int bdrv_open(BlockDriverState **pbs, const char
> *filename,
> bs->options = options;
> options = qdict_clone_shallow(options);
>
> - if (flags & BDRV_O_PROTOCOL) {
maybe you could use a bool protocol = flags & BDRV_O_PROTOCOL at the begining of
the function.
> - assert(!drv);
> - ret = bdrv_file_open(bs, &options, flags & ~BDRV_O_PROTOCOL,
> - &local_err);
> - if (!ret) {
> - drv = bs->drv;
> - goto done;
> - } else if (bs->drv) {
> - goto close_and_fail;
> - } else {
> - goto fail;
> - }
> - }
> -
> /* Open image file without format layer */
> - if (flags & BDRV_O_RDWR) {
> - flags |= BDRV_O_ALLOW_RDWR;
> - }
> - if (flags & BDRV_O_SNAPSHOT) {
> - snapshot_flags = bdrv_temp_snapshot_flags(flags);
> - flags = bdrv_backing_flags(flags);
> - }
> + if ((flags & BDRV_O_PROTOCOL) == 0) {
> + if (flags & BDRV_O_RDWR) {
> + flags |= BDRV_O_ALLOW_RDWR;
> + }
> + if (flags & BDRV_O_SNAPSHOT) {
> + snapshot_flags = bdrv_temp_snapshot_flags(flags);
> + flags = bdrv_backing_flags(flags);
> + }
>
> - assert(file == NULL);
> - ret = bdrv_open_image(&file, filename, options, "file",
> - bdrv_inherited_flags(flags),
> - true, &local_err);
> - if (ret < 0) {
> - goto fail;
> + assert(file == NULL);
> + ret = bdrv_open_image(&file, filename, options, "file",
> + bdrv_inherited_flags(flags),
> + true, &local_err);
> + if (ret < 0) {
> + goto fail;
> + }
> }
>
> /* Find the right image format driver */
> @@ -1500,7 +1477,7 @@ int bdrv_open(BlockDriverState **pbs, const char
> *filename,
> drv = bdrv_find_format(drvname);
> qdict_del(options, "driver");
> if (!drv) {
> - error_setg(errp, "Invalid driver: '%s'", drvname);
> + error_setg(errp, "Unknown driver: '%s'", drvname);
> ret = -EINVAL;
> goto fail;
> }
> @@ -1516,6 +1493,18 @@ int bdrv_open(BlockDriverState **pbs, const char
> *filename,
> }
>
> /* Open the image */
> + if (flags & BDRV_O_PROTOCOL) {
> + ret = bdrv_file_open(bs, drv, &options, flags & ~BDRV_O_PROTOCOL,
> + &local_err);
> + if (!ret) {
> + goto done;
> + } else if (bs->drv) {
> + goto close_and_fail;
> + } else {
> + goto fail;
> + }
> + }
> +
> ret = bdrv_open_common(bs, file, options, flags, drv, &local_err);
> if (ret < 0) {
> goto fail;
> --
> 1.8.3.1
>
>
Reviewed-by: Benoit Canet <ben...@irqsave.net>
