commit 4e4bf5c42c8b2847a90367936a6df6c277f4a76a
author Kevin Wolf <kwolf@redhat.com> Fri Dec 16 18:52:37 2016 +0100
committer Kevin Wolf <kwolf@redhat.com> Fri Feb 24 16:09:23 2017 +0100
tree 2b2003718fbd7652d35daf6bb209db90792ce88f
parent 52cdbc5869a3fbbe4d91c83e97dffb212af28ce3

block: Attach bs->file only during .bdrv_open()

The way that attaching bs->file worked was a bit unusual in that it was
the only child that would be attached to a node which is not opened yet.
Because of this, the block layer couldn't know yet which permissions the
driver would eventually need.

This patch moves the point where bs->file is attached to the beginning
of the individual .bdrv_open() implementations, so drivers already know
what they are going to do with the child. This is also more consistent
with how driver-specific children work.

For a moment, bdrv_open() gets its own BdrvChild to perform image
probing, but instead of directly assigning this BdrvChild to the BDS, it
becomes a temporary one and the node name is passed as an option to the
drivers, so that they can simply use bdrv_open_child() to create another
reference for their own use.

This duplicated child for (the not opened yet) bs is not the final
state, a follow-up patch will change the image probing code to use a
BlockBackend, which is completely independent of bs.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>

block.c

diff --git a/block.c b/block.c
index d951b5d..40c4dee 100644
--- a/block.c
+++ b/block.c
@@ -1103,13 +1103,6 @@
         assert(!drv->bdrv_needs_filename || filename != NULL);
         ret = drv->bdrv_file_open(bs, options, open_flags, &local_err);
     } else {
-        if (file == NULL) {
-            error_setg(errp, "Can't use '%s' as a block driver for the "
-                       "protocol level", drv->format_name);
-            ret = -EINVAL;
-            goto free_and_fail;
-        }
-        bs->file = file;
         ret = drv->bdrv_open(bs, options, open_flags, &local_err);
     }
 
@@ -1145,7 +1138,6 @@
     return 0;
 
 free_and_fail:
-    bs->file = NULL;
     g_free(bs->opaque);
     bs->opaque = NULL;
     bs->drv = NULL;
@@ -1368,7 +1360,18 @@
     }
 
     if (child->bs->inherits_from == parent) {
-        child->bs->inherits_from = NULL;
+        BdrvChild *c;
+
+        /* Remove inherits_from only when the last reference between parent and
+         * child->bs goes away. */
+        QLIST_FOREACH(c, &parent->children, next) {
+            if (c != child && c->bs == child->bs) {
+                break;
+            }
+        }
+        if (c == NULL) {
+            child->bs->inherits_from = NULL;
+        }
     }
 
     bdrv_root_unref_child(child);
@@ -1789,13 +1792,20 @@
         qdict_del(options, "backing");
     }
 
-    /* Open image file without format layer */
+    /* Open image file without format layer. This BdrvChild is only used for
+     * probing, the block drivers will do their own bdrv_open_child() for the
+     * same BDS, which is why we put the node name back into options. */
     if ((flags & BDRV_O_PROTOCOL) == 0) {
+        /* FIXME Shouldn't attach a child to a node that isn't opened yet. */
         file = bdrv_open_child(filename, options, "file", bs,
                                &child_file, true, &local_err);
         if (local_err) {
             goto fail;
         }
+        if (file != NULL) {
+            qdict_put(options, "file",
+                      qstring_from_str(bdrv_get_node_name(file->bs)));
+        }
     }
 
     /* Image format probing */
@@ -1835,7 +1845,7 @@
         goto fail;
     }
 
-    if (file && (bs->file != file)) {
+    if (file) {
         bdrv_unref_child(bs, file);
         file = NULL;
     }
@@ -1901,6 +1911,9 @@
     if (file != NULL) {
         bdrv_unref_child(bs, file);
     }
+    if (bs->file != NULL) {
+        bdrv_unref_child(bs, bs->file);
+    }
     QDECREF(snapshot_options);
     QDECREF(bs->explicit_options);
     QDECREF(bs->options);