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>

diff --git a/block.c b/block.c
index d951b5dc9e9ccbb9d0138fd1a2982cda65900faf..40c4dee2da72599d7f5030a4b5c720c0a4d9ae79 100644 (file)
--- a/block.c
+++ b/block.c
@@ -1103,13 +1103,6 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
         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 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     return 0;
 
 free_and_fail:
-    bs->file = NULL;
     g_free(bs->opaque);
     bs->opaque = NULL;
     bs->drv = NULL;
@@ -1368,7 +1360,18 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)
     }
 
     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 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
         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 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
         goto fail;
     }
 
-    if (file && (bs->file != file)) {
+    if (file) {
         bdrv_unref_child(bs, file);
         file = NULL;
     }
@@ -1901,6 +1911,9 @@ fail:
     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);

diff --git a/block/bochs.c b/block/bochs.c
index 8c9652ebeb5516a0afde651e54bd0f2dbc08bdd1..7dd2ac4f515f9bd7b630156737d98d230e9b8fc7 100644 (file)
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -104,6 +104,12 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
     struct bochs_header bochs;
     int ret;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     bs->read_only = true; /* no write support yet */
 
     ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs));

diff --git a/block/cloop.c b/block/cloop.c
index 7b75f7ef7b63db60adb186c5ffe9b2e7c5ca01cc..877c9b0d1b2b3c913c5399399d2ccf34d3b5d2c6 100644 (file)
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -66,6 +66,12 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
     uint32_t offsets_size, max_compressed_block_size = 1, i;
     int ret;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     bs->read_only = true;
 
     /* read header */

diff --git a/block/crypto.c b/block/crypto.c
index e05e4dda4719633e63c5d7815a025a1a2f3a73fb..7cb2ff2946e49f0fc829dae6f379834d474c000a 100644 (file)
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -300,6 +300,12 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
     QCryptoBlockOpenOptions *open_opts = NULL;
     unsigned int cflags = 0;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     opts = qemu_opts_create(opts_spec, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (local_err) {

diff --git a/block/dmg.c b/block/dmg.c
index 58a3ae86c1da77e8825efc0548fb3c44bc8ab07e..8e387cdfe5d08790efaf001b4e47b33e74e826f4 100644 (file)
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -413,6 +413,12 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
     int64_t offset;
     int ret;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     block_module_load_one("dmg-bz2");
     bs->read_only = true;
 

diff --git a/block/parallels.c b/block/parallels.c
index ac94dfb497322a5a7217304f249feac7f74f0279..b2ec09f7e66e0847772efb92582a90d0dbcaf561 100644 (file)
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -581,6 +581,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     Error *local_err = NULL;
     char *buf;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     ret = bdrv_pread(bs->file, 0, &ph, sizeof(ph));
     if (ret < 0) {
         goto fail;

diff --git a/block/qcow.c b/block/qcow.c
index 4534515db99317af8a1d01bf131dd31da359b720..038b05ab1bcc5de99404d5d95760866883e22520 100644 (file)
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -106,6 +106,12 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     QCowHeader header;
     Error *local_err = NULL;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
     if (ret < 0) {
         goto fail;

diff --git a/block/qcow2.c b/block/qcow2.c
index 3e1172b32581912663ca2ac26c17d4c1baff0609..21e61427eb33ead0a06de5edcbb82566ad692b3e 100644 (file)
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -814,8 +814,8 @@ static int qcow2_update_options(BlockDriverState *bs, QDict *options,
     return ret;
 }
 
-static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
-                      Error **errp)
+static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
+                         Error **errp)
 {
     BDRVQcow2State *s = bs->opaque;
     unsigned int len, i;
@@ -1205,6 +1205,18 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     return ret;
 }
 
+static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
+                      Error **errp)
+{
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
+    return qcow2_do_open(bs, options, flags, errp);
+}
+
 static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -1785,7 +1797,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
     options = qdict_clone_shallow(bs->options);
 
     flags &= ~BDRV_O_INACTIVE;
-    ret = qcow2_open(bs, options, flags, &local_err);
+    ret = qcow2_do_open(bs, options, flags, &local_err);
     QDECREF(options);
     if (local_err) {
         error_propagate(errp, local_err);

diff --git a/block/qed.c b/block/qed.c
index 0b62c7799e2a8ddbd5d6f19073eae7c327caaec4..62a0a09326f91fa44c919c128f84f712f23e7eab 100644 (file)
--- a/block/qed.c
+++ b/block/qed.c
@@ -415,8 +415,8 @@ static void bdrv_qed_drain(BlockDriverState *bs)
     }
 }
 
-static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
-                         Error **errp)
+static int bdrv_qed_do_open(BlockDriverState *bs, QDict *options, int flags,
+                            Error **errp)
 {
     BDRVQEDState *s = bs->opaque;
     QEDHeader le_header;
@@ -550,6 +550,18 @@ out:
     return ret;
 }
 
+static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
+                         Error **errp)
+{
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
+    return bdrv_qed_do_open(bs, options, flags, errp);
+}
+
 static void bdrv_qed_refresh_limits(BlockDriverState *bs, Error **errp)
 {
     BDRVQEDState *s = bs->opaque;
@@ -1629,7 +1641,7 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
     bdrv_qed_close(bs);
 
     memset(s, 0, sizeof(BDRVQEDState));
-    ret = bdrv_qed_open(bs, NULL, bs->open_flags, &local_err);
+    ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         error_prepend(errp, "Could not reopen qed layer: ");

diff --git a/block/raw-format.c b/block/raw-format.c
index 0ddffbd0cd4e1097a0bf499f9187e63855fd142f..ce34d1b1cdee4842b33563d99d8a8dc60f2ec3bb 100644 (file)
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -384,6 +384,12 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
     BDRVRawState *s = bs->opaque;
     int ret;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     bs->sg = bs->file->bs->sg;
     bs->supported_write_flags = BDRV_REQ_FUA &
         bs->file->bs->supported_write_flags;

diff --git a/block/replication.c b/block/replication.c
index 729dd124999ffcc8988f78bb3919d96799825f36..eff85c77baf6d0299cb1241672ece3700e8721d9 100644 (file)
--- a/block/replication.c
+++ b/block/replication.c
@@ -86,6 +86,12 @@ static int replication_open(BlockDriverState *bs, QDict *options,
     const char *mode;
     const char *top_id;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     ret = -EINVAL;
     opts = qemu_opts_create(&replication_runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);

diff --git a/block/vdi.c b/block/vdi.c
index 0aeb940aa8e0b99311470fc30ad16f47a4bc56e5..18b4773aaced90dcf8f41028024f8c623cc1f16d 100644 (file)
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -363,6 +363,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     int ret;
     Error *local_err = NULL;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     logout("\n");
 
     ret = bdrv_read(bs->file, 0, (uint8_t *)&header, 1);

diff --git a/block/vhdx.c b/block/vhdx.c
index c67772e2f706940765a9408d679e6f9fb38b8417..9918ee98ffa4126abe2bab877cc5a6254fac6e0f 100644 (file)
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -898,6 +898,12 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
     uint64_t signature;
     Error *local_err = NULL;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     s->bat = NULL;
     s->first_visible_write = true;
 

diff --git a/block/vmdk.c b/block/vmdk.c
index 393c84d8b125b7a4560f073ba09802c0dfaed1a3..9d68ec5a4ecae98008970a4e7bc504637d3026ae 100644 (file)
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -943,6 +943,12 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
     uint32_t magic;
     Error *local_err = NULL;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     buf = vmdk_read_desc(bs->file, 0, errp);
     if (!buf) {
         return -EINVAL;

diff --git a/block/vpc.c b/block/vpc.c
index ed6353dbd410e5cdf0f68b88d820bc15335dee62..d0df2a1c544056d3a1ab7ed057d6b8e73b18f5f6 100644 (file)
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -220,6 +220,12 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     int disk_type = VHD_DYNAMIC;
     int ret;
 
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
+                               false, errp);
+    if (!bs->file) {
+        return -EINVAL;
+    }
+
     opts = qemu_opts_create(&vpc_runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (local_err) {

diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 42bf4164ca0b6a7522575777277cf31032e907c2..7524c62025ad7471c5d11e40de90819958b589e5 100644 (file)
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -225,7 +225,7 @@ Testing: -drive driver=nbd
 QEMU_PROG: -drive driver=nbd: NBD server address missing
 
 Testing: -drive driver=raw
-QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the protocol level
+QEMU_PROG: -drive driver=raw: A block device must be specified for "file"
 
 Testing: -drive file.driver=file
 QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file name
@@ -234,7 +234,7 @@ Testing: -drive file.driver=nbd
 QEMU_PROG: -drive file.driver=nbd: NBD server address missing
 
 Testing: -drive file.driver=raw
-QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the protocol level
+QEMU_PROG: -drive file.driver=raw: A block device must be specified for "file"
 
 Testing: -drive foo=bar
 QEMU_PROG: -drive foo=bar: Must specify either driver or file

diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index f8047a2e4582cac4947142ae742e7c9e50fa732f..e206ad6c29f7767e853be4d6877e68014a608ef7 100644 (file)
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -323,7 +323,7 @@ Testing: -drive driver=nbd
 QEMU_PROG: -drive driver=nbd: NBD server address missing
 
 Testing: -drive driver=raw
-QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the protocol level
+QEMU_PROG: -drive driver=raw: A block device must be specified for "file"
 
 Testing: -drive file.driver=file
 QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file name
@@ -332,7 +332,7 @@ Testing: -drive file.driver=nbd
 QEMU_PROG: -drive file.driver=nbd: NBD server address missing
 
 Testing: -drive file.driver=raw
-QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the protocol level
+QEMU_PROG: -drive file.driver=raw: A block device must be specified for "file"
 
 Testing: -drive foo=bar
 QEMU_PROG: -drive foo=bar: Must specify either driver or file