block: Change BDS parameter of bdrv_open() to **

Make bdrv_open() take a pointer to a BDS pointer, similarly to
bdrv_file_open(). If a pointer to a NULL pointer is given, bdrv_open()
will create a new BDS with an empty name; if the BDS pointer is not
NULL, that existing BDS will be reused (in the same way as bdrv_open()
already did).

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

block.c

@@ -1046,7 +1046,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
     }
 
     if (!drv->bdrv_file_open) {
-        ret = bdrv_open(bs, filename, options, flags, drv, &local_err);
+        ret = bdrv_open(&bs, filename, options, flags, drv, &local_err);
         options = NULL;
     } else {
         ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
@@ -1115,8 +1115,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
                                        sizeof(backing_filename));
     }
 
-    bs->backing_hd = bdrv_new("");
-
     if (bs->backing_format[0] != '\0') {
         back_drv = bdrv_find_format(bs->backing_format);
     }
@@ -1125,11 +1123,11 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
     back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT |
                                     BDRV_O_COPY_ON_READ);
 
-    ret = bdrv_open(bs->backing_hd,
+    assert(bs->backing_hd == NULL);
+    ret = bdrv_open(&bs->backing_hd,
                     *backing_filename ? backing_filename : NULL, options,
                     back_flags, back_drv, &local_err);
     if (ret < 0) {
-        bdrv_unref(bs->backing_hd);
         bs->backing_hd = NULL;
         bs->open_flags |= BDRV_O_NO_BACKING;
         error_setg(errp, "Could not open backing file: %s",
@@ -1166,6 +1164,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
  * BlockdevRef.
  *
  * The BlockdevRef will be removed from the options QDict.
+ *
+ * To conform with the behavior of bdrv_open(), *pbs has to be NULL.
  */
 int bdrv_open_image(BlockDriverState **pbs, const char *filename,
                     QDict *options, const char *bdref_key, int flags,
@@ -1176,6 +1176,9 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
     char *bdref_key_dot;
     const char *reference;
 
+    assert(pbs);
+    assert(*pbs == NULL);
+
     bdref_key_dot = g_strdup_printf("%s.", bdref_key);
     qdict_extract_subqdict(options, &image_options, bdref_key_dot);
     g_free(bdref_key_dot);
@@ -1196,8 +1199,6 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
         /* If a filename is given and the block driver should be detected
            automatically (instead of using none), use bdrv_open() in order to do
            that auto-detection. */
-        BlockDriverState *bs;
-
         if (reference) {
             error_setg(errp, "Cannot reference an existing block device while "
                        "giving a filename");
@@ -1205,13 +1206,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
             goto done;
         }
 
-        bs = bdrv_new("");
+        ret = bdrv_open(pbs, filename, image_options, flags, NULL, errp);
-        ret = bdrv_open(bs, filename, image_options, flags, NULL, errp);
-        if (ret < 0) {
-            bdrv_unref(bs);
-        } else {
-            *pbs = bs;
-        }
     } else {
         ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
                              errp);
@@ -1229,17 +1224,28 @@ done:
  * empty set of options. The reference to the QDict belongs to the block layer
  * after the call (even on failure), so if the caller intends to reuse the
  * dictionary, it needs to use QINCREF() before calling bdrv_open.
+ *
+ * If *pbs is NULL, a new BDS will be created with a pointer to it stored there.
+ * If it is not NULL, the referenced BDS will be reused.
  */
-int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
+int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
               int flags, BlockDriver *drv, Error **errp)
 {
     int ret;
     /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
     char tmp_filename[PATH_MAX + 1];
-    BlockDriverState *file = NULL;
+    BlockDriverState *file = NULL, *bs;
     const char *drvname;
     Error *local_err = NULL;
 
+    assert(pbs);
+
+    if (*pbs) {
+        bs = *pbs;
+    } else {
+        bs = bdrv_new("");
+    }
+
     /* NULL means an empty set of options */
     if (options == NULL) {
         options = qdict_new();
@@ -1260,12 +1266,11 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
            instead of opening 'filename' directly */
 
         /* Get the required size from the image */
-        bs1 = bdrv_new("");
         QINCREF(options);
-        ret = bdrv_open(bs1, filename, options, BDRV_O_NO_BACKING,
+        bs1 = NULL;
+        ret = bdrv_open(&bs1, filename, options, BDRV_O_NO_BACKING,
                         drv, &local_err);
         if (ret < 0) {
-            bdrv_unref(bs1);
             goto fail;
         }
         total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK;
@@ -1322,6 +1327,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         flags |= BDRV_O_ALLOW_RDWR;
     }
 
+    assert(file == NULL);
     ret = bdrv_open_image(&file, filename, options, "file",
                           bdrv_open_flags(bs, flags | BDRV_O_UNMAP), true, true,
                           &local_err);
@@ -1393,6 +1399,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         bdrv_dev_change_media_cb(bs, true);
     }
 
+    *pbs = bs;
     return 0;
 
 unlink_and_fail:
@@ -1406,13 +1413,24 @@ fail:
     QDECREF(bs->options);
     QDECREF(options);
     bs->options = NULL;
+    if (!*pbs) {
+        /* If *pbs is NULL, a new BDS has been created in this function and
+           needs to be freed now. Otherwise, it does not need to be closed,
+           since it has not really been opened yet. */
+        bdrv_unref(bs);
+    }
     if (local_err) {
         error_propagate(errp, local_err);
     }
     return ret;
 
 close_and_fail:
-    bdrv_close(bs);
+    /* See fail path, but now the BDS has to be always closed */
+    if (*pbs) {
+        bdrv_close(bs);
+    } else {
+        bdrv_unref(bs);
+    }
     QDECREF(options);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -5290,9 +5308,8 @@ void bdrv_img_create(const char *filename, const char *fmt,
             back_flags =
                 flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
 
-            bs = bdrv_new("");
+            bs = NULL;
-
+            ret = bdrv_open(&bs, backing_file->value.s, NULL, back_flags,
-            ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
                             backing_drv, &local_err);
             if (ret < 0) {
                 error_setg_errno(errp, -ret, "Could not open '%s': %s",
@@ -5300,7 +5317,6 @@ void bdrv_img_create(const char *filename, const char *fmt,
                                  error_get_pretty(local_err));
                 error_free(local_err);
                 local_err = NULL;
-                bdrv_unref(bs);
                 goto out;
             }
             bdrv_get_geometry(bs, &size);

block/blkdebug.c

@@ -410,6 +410,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
     s->state = 1;
 
     /* Open the backing file */
+    assert(bs->file == NULL);
     ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-image"), options, "image",
                           flags, true, false, &local_err);
     if (ret < 0) {

block/blkverify.c

@@ -135,6 +135,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Open the raw file */
+    assert(bs->file == NULL);
     ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options,
                           "raw", flags, true, false, &local_err);
     if (ret < 0) {
@@ -143,6 +144,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Open the test file */
+    assert(s->test_file == NULL);
     ret = bdrv_open_image(&s->test_file, qemu_opt_get(opts, "x-image"), options,
                           "test", flags, false, false, &local_err);
     if (ret < 0) {

block/qcow2.c

@@ -1543,7 +1543,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
         goto out;
     }
 
-    bdrv_close(bs);
+    bdrv_unref(bs);
+    bs = NULL;
 
     /*
      * And now open the image and make it consistent first (i.e. increase the
@@ -1552,7 +1553,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
      */
     BlockDriver* drv = bdrv_find_format("qcow2");
     assert(drv != NULL);
-    ret = bdrv_open(bs, filename, NULL,
+    ret = bdrv_open(&bs, filename, NULL,
         BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
@@ -1599,10 +1600,11 @@ static int qcow2_create2(const char *filename, int64_t total_size,
         }
     }
 
-    bdrv_close(bs);
+    bdrv_unref(bs);
+    bs = NULL;
 
     /* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */
-    ret = bdrv_open(bs, filename, NULL,
+    ret = bdrv_open(&bs, filename, NULL,
                     BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
                     drv, &local_err);
     if (local_err) {
@@ -1612,7 +1614,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
 
     ret = 0;
 out:
-    bdrv_unref(bs);
+    if (bs) {
+        bdrv_unref(bs);
+    }
     return ret;
 }
 

block/vmdk.c

@@ -1755,10 +1755,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
         goto exit;
     }
     if (backing_file) {
-        BlockDriverState *bs = bdrv_new("");
+        BlockDriverState *bs = NULL;
-        ret = bdrv_open(bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp);
+        ret = bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp);
         if (ret != 0) {
-            bdrv_unref(bs);
             goto exit;
         }
         if (strcmp(bs->drv->format_name, "vmdk")) {

block/vvfat.c

@@ -2936,15 +2936,13 @@ static int enable_write_target(BDRVVVFATState *s)
         goto err;
     }
 
-    s->qcow = bdrv_new("");
+    s->qcow = NULL;
-
+    ret = bdrv_open(&s->qcow, s->qcow_filename, NULL,
-    ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
             BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
             &local_err);
     if (ret < 0) {
         qerror_report_err(local_err);
         error_free(local_err);
-        bdrv_unref(s->qcow);
         goto err;
     }
 

blockdev.c

@@ -504,7 +504,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
     bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
 
     QINCREF(bs_opts);
-    ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
+    ret = bdrv_open(&dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
 
     if (ret < 0) {
         error_setg(errp, "could not open disk image %s: %s",
@@ -1330,12 +1330,12 @@ static void external_snapshot_prepare(BlkTransactionState *common,
                   qstring_from_str(snapshot_node_name));
     }
 
-    /* We will manually add the backing_hd field to the bs later */
-    state->new_bs = bdrv_new("");
     /* TODO Inherit bs->options or only take explicit options with an
      * extended QMP command? */
-    ret = bdrv_open(state->new_bs, new_image_file, options,
+    assert(state->new_bs == NULL);
+    ret = bdrv_open(&state->new_bs, new_image_file, options,
                     flags | BDRV_O_NO_BACKING, drv, &local_err);
+    /* We will manually add the backing_hd field to the bs later */
     if (ret != 0) {
         error_propagate(errp, local_err);
     }
@@ -1582,7 +1582,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv, &local_err);
+    ret = bdrv_open(&bs, filename, NULL, bdrv_flags, drv, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return;
@@ -2018,10 +2018,9 @@ void qmp_drive_backup(const char *device, const char *target,
         return;
     }
 
-    target_bs = bdrv_new("");
+    target_bs = NULL;
-    ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err);
+    ret = bdrv_open(&target_bs, target, NULL, flags, drv, &local_err);
     if (ret < 0) {
-        bdrv_unref(target_bs);
         error_propagate(errp, local_err);
         return;
     }
@@ -2162,11 +2161,10 @@ void qmp_drive_mirror(const char *device, const char *target,
     /* Mirroring takes care of copy-on-write using the source's backing
      * file.
      */
-    target_bs = bdrv_new("");
+    target_bs = NULL;
-    ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
+    ret = bdrv_open(&target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
                     &local_err);
     if (ret < 0) {
-        bdrv_unref(target_bs);
         error_propagate(errp, local_err);
         return;
     }

hw/block/xen_disk.c

@@ -813,7 +813,7 @@ static int blk_connect(struct XenDevice *xendev)
             Error *local_err = NULL;
             BlockDriver *drv = bdrv_find_whitelisted_format(blkdev->fileproto,
                                                            readonly);
-            if (bdrv_open(blkdev->bs,
+            if (bdrv_open(&blkdev->bs,
                           blkdev->filename, NULL, qflags, drv, &local_err) != 0)
             {
                 xen_be_printf(&blkdev->xendev, 0, "error: %s\n",

include/block/block.h

@@ -190,7 +190,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
                     QDict *options, const char *bdref_key, int flags,
                     bool force_raw, bool allow_none, Error **errp);
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
-int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
+int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
               int flags, BlockDriver *drv, Error **errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
                                     BlockDriverState *bs, int flags);

qemu-img.c

@@ -289,7 +289,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
         drv = NULL;
     }
 
-    ret = bdrv_open(bs, filename, NULL, flags, drv, &local_err);
+    ret = bdrv_open(&bs, filename, NULL, flags, drv, &local_err);
     if (ret < 0) {
         error_report("Could not open '%s': %s", filename,
                      error_get_pretty(local_err));
@@ -310,9 +310,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
     }
     return bs;
 fail:
-    if (bs) {
+    bdrv_unref(bs);
-        bdrv_unref(bs);
-    }
     return NULL;
 }
 
@@ -2314,7 +2312,7 @@ static int img_rebase(int argc, char **argv)
 
         bs_old_backing = bdrv_new("old_backing");
         bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
-        ret = bdrv_open(bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
+        ret = bdrv_open(&bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
                         old_backing_drv, &local_err);
         if (ret) {
             error_report("Could not open old backing file '%s': %s",
@@ -2324,7 +2322,7 @@ static int img_rebase(int argc, char **argv)
         }
         if (out_baseimg[0]) {
             bs_new_backing = bdrv_new("new_backing");
-            ret = bdrv_open(bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
+            ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
                         new_backing_drv, &local_err);
             if (ret) {
                 error_report("Could not open new backing file '%s': %s",

qemu-io.c

@@ -68,7 +68,7 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
     } else {
         qemuio_bs = bdrv_new("hda");
 
-        if (bdrv_open(qemuio_bs, name, opts, flags, NULL, &local_err) < 0) {
+        if (bdrv_open(&qemuio_bs, name, opts, flags, NULL, &local_err) < 0) {
             fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
                     error_get_pretty(local_err));
             error_free(local_err);

qemu-nbd.c

@@ -597,7 +597,7 @@ int main(int argc, char **argv)
 
     bs = bdrv_new("hda");
     srcpath = argv[optind];
-    ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err);
+    ret = bdrv_open(&bs, srcpath, NULL, flags, drv, &local_err);
     if (ret < 0) {
         errno = -ret;
         err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind],