diff mbox

[v2,7/9] block: Remove second bdrv_open() recursion

Message ID 1403706959-6979-8-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf June 25, 2014, 2:35 p.m. UTC 
This recursion was introduced in commit 505d7583 in order to allow
nesting image formats. It only ever takes effect when the user
explicitly specifies a driver name and that driver isn't suitable for
the protocol level.

We can check this earlier in bdrv_open() and if the explicitly
requested driver is a format driver, clear BDRV_O_PROTOCOL so that
another bs->file layer is opened.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

Comments

Eric Blake June 25, 2014, 3:52 p.m. UTC | #1 
On 06/25/2014 08:35 AM, Kevin Wolf wrote:
> This recursion was introduced in commit 505d7583 in order to allow
> nesting image formats. It only ever takes effect when the user
> explicitly specifies a driver name and that driver isn't suitable for
> the protocol level.
> 
> We can check this earlier in bdrv_open() and if the explicitly
> requested driver is a format driver, clear BDRV_O_PROTOCOL so that
> another bs->file layer is opened.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c | 50 +++++++++++++++++++++++++-------------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox

Patch

diff --git a/block.c b/block.c
index 64e1176..b179dd6 100644
--- a/block.c
+++ b/block.c
@@ -1404,6 +1404,26 @@  int bdrv_open(BlockDriverState **pbs, const char *filename,
         goto fail;
     }
 
+    /* Find the right image format driver */
+    drv = NULL;
+    drvname = qdict_get_try_str(options, "driver");
+    if (drvname) {
+        drv = bdrv_find_format(drvname);
+        qdict_del(options, "driver");
+        if (!drv) {
+            error_setg(errp, "Unknown driver: '%s'", drvname);
+            ret = -EINVAL;
+            goto fail;
+        }
+    }
+
+    assert(drvname || !(flags & BDRV_O_PROTOCOL));
+    if (drv && !drv->bdrv_file_open) {
+        /* If the user explicitly wants a format driver here, we'll need to add
+         * another layer for the protocol in bs->file */
+        flags &= ~BDRV_O_PROTOCOL;
+    }
+
     bs->options = options;
     options = qdict_clone_shallow(options);
 
@@ -1426,25 +1446,13 @@  int bdrv_open(BlockDriverState **pbs, const char *filename,
         }
     }
 
-    /* Find the right image format driver */
-    drv = NULL;
-    drvname = qdict_get_try_str(options, "driver");
-    assert(drvname || !(flags & BDRV_O_PROTOCOL));
-
-    if (drvname) {
-        drv = bdrv_find_format(drvname);
-        qdict_del(options, "driver");
-        if (!drv) {
-            error_setg(errp, "Unknown driver: '%s'", drvname);
-            ret = -EINVAL;
-            goto fail;
-        }
-    } else if (file) {
+    /* Image format probing */
+    if (!drv && file) {
         ret = find_image_format(file, filename, &drv, &local_err);
         if (ret < 0) {
             goto fail;
         }
-    } else {
+    } else if (!drv) {
         error_setg(errp, "Must specify either driver or file");
         ret = -EINVAL;
         goto fail;
@@ -1452,16 +1460,8 @@  int bdrv_open(BlockDriverState **pbs, const char *filename,
 
     /* Open the image */
     if (flags & BDRV_O_PROTOCOL) {
-        if (!drv->bdrv_file_open) {
-            const char *filename;
-            filename = qdict_get_try_str(options, "filename");
-            ret = bdrv_open(&bs, filename, NULL, options,
-                            flags & ~BDRV_O_PROTOCOL, drv, &local_err);
-            options = NULL;
-        } else {
-            ret = bdrv_open_common(bs, NULL, options,
-                                   flags & ~BDRV_O_PROTOCOL, drv, &local_err);
-        }
+        ret = bdrv_open_common(bs, NULL, options,
+                               flags & ~BDRV_O_PROTOCOL, drv, &local_err);
         if (!ret) {
             bs->growable = 1;
             goto done;