[Qemu-devel] [PATCH 0/5] block: Drop drv parameter from bdrv_open()

[Qemu-devel] [PATCH 0/5] block: Drop drv parameter from bdrv_open()

[Max Reitz]

We don't really need that parameter, so let's drop it. Doing so may even
fix some bugs, see
http://lists.nongnu.org/archive/html/qemu-block/2015-08/msg00171.html.

In the course of writing this series, I had to decide whether the make
sure all callers of bdrv_find_whitelisted_format() would still only
accept whitelisted formats, which you'd think would be a good idea; but
the only caller left was qmp_change_blockdev(), so I guess noone really
cared about it anymore, instead relying on use_bdrv_whitelist alone.

So I decided dropped bdrv_find_whitelisted_format() completely. If you
feel this is a bad decision, feel free to argue but then I guess we'll
have to reevaluate all bdrv_find_format() calls whether they should
actually be bdrv_find_whitelisted_format() calls.


Max Reitz (5):
  block: Always pass NULL as drv for bdrv_open()
  block: Drop drv parameter from bdrv_open()
  block: Drop drv parameter from bdrv_open_inherit()
  block: Drop drv parameter from bdrv_fill_options()
  block: Drop bdrv_find_whitelisted_format()

 block.c               | 108 +++++++++++++++++++-------------------------------
 block/block-backend.c |   2 +-
 block/parallels.c     |   2 +-
 block/qcow.c          |   2 +-
 block/qcow2.c         |  18 ++++++---
 block/qed.c           |   2 +-
 block/sheepdog.c      |   5 +--
 block/vdi.c           |   2 +-
 block/vhdx.c          |   2 +-
 block/vmdk.c          |   7 ++--
 block/vpc.c           |   2 +-
 block/vvfat.c         |   8 +++-
 blockdev.c            |  72 +++++++++++++--------------------
 include/block/block.h |   5 +--
 14 files changed, 100 insertions(+), 137 deletions(-)

-- 
2.4.6

[Qemu-devel] [PATCH 1/5] block: Always pass NULL as drv for bdrv_open()

[Max Reitz]

Change all callers of bdrv_open() to pass the driver name in the options
QDict instead of passing its BlockDriver pointer.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c       | 24 ++++++++++----------
 block/qcow2.c | 16 ++++++++-----
 block/vvfat.c |  8 +++++--
 blockdev.c    | 72 +++++++++++++++++++++++------------------------------------
 4 files changed, 57 insertions(+), 63 deletions(-)

diff --git a/block.c b/block.c
index d088ee0..193daf9 100644
--- a/block.c
+++ b/block.c
@@ -1385,11 +1385,13 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
               qstring_from_str("file"));
     qdict_put(snapshot_options, "file.filename",
               qstring_from_str(tmp_filename));
+    qdict_put(snapshot_options, "driver",
+              qstring_from_str("qcow2"));
 
     bs_snapshot = bdrv_new();
 
     ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
-                    flags, &bdrv_qcow2, &local_err);
+                    flags, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
@@ -3739,7 +3741,6 @@ void bdrv_img_create(const char *filename, const char *fmt,
     const char *backing_fmt, *backing_file;
     int64_t size;
     BlockDriver *drv, *proto_drv;
-    BlockDriver *backing_drv = NULL;
     Error *local_err = NULL;
     int ret = 0;
 
@@ -3813,14 +3814,6 @@ void bdrv_img_create(const char *filename, const char *fmt,
     }
 
     backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
-    if (backing_fmt) {
-        backing_drv = bdrv_find_format(backing_fmt);
-        if (!backing_drv) {
-            error_setg(errp, "Unknown backing file format '%s'",
-                       backing_fmt);
-            goto out;
-        }
-    }
 
     // The size for the image must always be specified, with one exception:
     // If we are using a backing file, we can obtain the size from there
@@ -3831,6 +3824,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
             char *full_backing = g_new0(char, PATH_MAX);
             int64_t size;
             int back_flags;
+            QDict *backing_options = NULL;
 
             bdrv_get_full_backing_filename_from_filename(filename, backing_file,
                                                          full_backing, PATH_MAX,
@@ -3844,9 +3838,15 @@ void bdrv_img_create(const char *filename, const char *fmt,
             back_flags =
                 flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
 
+            if (backing_fmt) {
+                backing_options = qdict_new();
+                qdict_put(backing_options, "driver",
+                          qstring_from_str(backing_fmt));
+            }
+
             bs = NULL;
-            ret = bdrv_open(&bs, full_backing, NULL, NULL, back_flags,
-                            backing_drv, &local_err);
+            ret = bdrv_open(&bs, full_backing, NULL, backing_options,
+                            back_flags, NULL, &local_err);
             g_free(full_backing);
             if (ret < 0) {
                 goto out;
diff --git a/block/qcow2.c b/block/qcow2.c
index ea34ae2..867b43b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1873,8 +1873,10 @@ static int qcow2_create2(const char *filename, int64_t total_size,
                          QemuOpts *opts, int version, int refcount_order,
                          Error **errp)
 {
-    /* Calculate cluster_bits */
     int cluster_bits;
+    QDict *options;
+
+    /* Calculate cluster_bits */
     cluster_bits = ctz32(cluster_size);
     if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits > MAX_CLUSTER_BITS ||
         (1 << cluster_bits) != cluster_size)
@@ -2032,9 +2034,11 @@ static int qcow2_create2(const char *filename, int64_t total_size,
      * refcount of the cluster that is occupied by the header and the refcount
      * table)
      */
-    ret = bdrv_open(&bs, filename, NULL, NULL,
+    options = qdict_new();
+    qdict_put(options, "driver", qstring_from_str("qcow2"));
+    ret = bdrv_open(&bs, filename, NULL, options,
                     BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
-                    &bdrv_qcow2, &local_err);
+                    NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
@@ -2084,9 +2088,11 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     bs = NULL;
 
     /* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */
-    ret = bdrv_open(&bs, filename, NULL, NULL,
+    options = qdict_new();
+    qdict_put(options, "driver", qstring_from_str("qcow2"));
+    ret = bdrv_open(&bs, filename, NULL, options,
                     BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
-                    &bdrv_qcow2, &local_err);
+                    NULL, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto out;
diff --git a/block/vvfat.c b/block/vvfat.c
index 2068697..bffe8ad 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2926,6 +2926,8 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)
     QemuOpts *opts = NULL;
     int ret;
     int size = sector2cluster(s, s->sector_count);
+    QDict *options;
+
     s->used_clusters = calloc(size, 1);
 
     array_init(&(s->commits), sizeof(commit_t));
@@ -2956,9 +2958,11 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)
     }
 
     s->qcow = NULL;
-    ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, NULL,
+    options = qdict_new();
+    qdict_put(options, "driver", qstring_from_str("qcow"));
+    ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, options,
                     BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
-                    bdrv_qcow, errp);
+                    NULL, errp);
     if (ret < 0) {
         goto err;
     }
diff --git a/blockdev.c b/blockdev.c
index 62a4586..4528fbe 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1416,9 +1416,8 @@ typedef struct ExternalSnapshotState {
 static void external_snapshot_prepare(BlkTransactionState *common,
                                       Error **errp)
 {
-    BlockDriver *drv;
     int flags, ret;
-    QDict *options = NULL;
+    QDict *options;
     Error *local_err = NULL;
     bool has_device = false;
     const char *device;
@@ -1453,12 +1452,6 @@ static void external_snapshot_prepare(BlkTransactionState *common,
     }
 
     /* start processing */
-    drv = bdrv_find_format(format);
-    if (!drv) {
-        error_setg(errp, QERR_INVALID_BLOCK_FORMAT, format);
-        return;
-    }
-
     state->old_bs = bdrv_lookup_bs(has_device ? device : NULL,
                                    has_node_name ? node_name : NULL,
                                    &local_err);
@@ -1517,17 +1510,18 @@ static void external_snapshot_prepare(BlkTransactionState *common,
         }
     }
 
+    options = qdict_new();
     if (has_snapshot_node_name) {
-        options = qdict_new();
         qdict_put(options, "node-name",
                   qstring_from_str(snapshot_node_name));
     }
+    qdict_put(options, "driver", qstring_from_str(format));
 
     /* TODO Inherit bs->options or only take explicit options with an
      * extended QMP command? */
     assert(state->new_bs == NULL);
     ret = bdrv_open(&state->new_bs, new_image_file, NULL, options,
-                    flags | BDRV_O_NO_BACKING, drv, &local_err);
+                    flags | BDRV_O_NO_BACKING, NULL, &local_err);
     /* We will manually add the backing_hd field to the bs later */
     if (ret != 0) {
         error_propagate(errp, local_err);
@@ -1889,13 +1883,19 @@ void qmp_block_passwd(bool has_device, const char *device,
 
 /* Assumes AioContext is held */
 static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
-                                    int bdrv_flags, BlockDriver *drv,
+                                    int bdrv_flags, const char *format,
                                     const char *password, Error **errp)
 {
     Error *local_err = NULL;
+    QDict *options = NULL;
     int ret;
 
-    ret = bdrv_open(&bs, filename, NULL, NULL, bdrv_flags, drv, &local_err);
+    if (format) {
+        options = qdict_new();
+        qdict_put(options, "driver", qstring_from_str(format));
+    }
+
+    ret = bdrv_open(&bs, filename, NULL, options, bdrv_flags, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return;
@@ -1910,7 +1910,6 @@ void qmp_change_blockdev(const char *device, const char *filename,
     BlockBackend *blk;
     BlockDriverState *bs;
     AioContext *aio_context;
-    BlockDriver *drv = NULL;
     int bdrv_flags;
     Error *err = NULL;
 
@@ -1925,14 +1924,6 @@ void qmp_change_blockdev(const char *device, const char *filename,
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
 
-    if (format) {
-        drv = bdrv_find_whitelisted_format(format, bs->read_only);
-        if (!drv) {
-            error_setg(errp, QERR_INVALID_BLOCK_FORMAT, format);
-            goto out;
-        }
-    }
-
     eject_device(blk, 0, &err);
     if (err) {
         error_propagate(errp, err);
@@ -1942,7 +1933,7 @@ void qmp_change_blockdev(const char *device, const char *filename,
     bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
     bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
 
-    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
+    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, format, NULL, errp);
 
 out:
     aio_context_release(aio_context);
@@ -2460,7 +2451,7 @@ void qmp_drive_backup(const char *device, const char *target,
     BlockDriverState *source = NULL;
     BdrvDirtyBitmap *bmap = NULL;
     AioContext *aio_context;
-    BlockDriver *drv = NULL;
+    QDict *options = NULL;
     Error *local_err = NULL;
     int flags;
     int64_t size;
@@ -2500,13 +2491,6 @@ void qmp_drive_backup(const char *device, const char *target,
     if (!has_format) {
         format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
     }
-    if (format) {
-        drv = bdrv_find_format(format);
-        if (!drv) {
-            error_setg(errp, QERR_INVALID_BLOCK_FORMAT, format);
-            goto out;
-        }
-    }
 
     /* Early check to avoid creating target */
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
@@ -2534,7 +2518,7 @@ void qmp_drive_backup(const char *device, const char *target,
     }
 
     if (mode != NEW_IMAGE_MODE_EXISTING) {
-        assert(format && drv);
+        assert(format);
         if (source) {
             bdrv_img_create(target, format, source->filename,
                             source->drv->format_name, NULL,
@@ -2550,8 +2534,13 @@ void qmp_drive_backup(const char *device, const char *target,
         goto out;
     }
 
+    if (format) {
+        options = qdict_new();
+        qdict_put(options, "driver", qstring_from_str(format));
+    }
+
     target_bs = NULL;
-    ret = bdrv_open(&target_bs, target, NULL, NULL, flags, drv, &local_err);
+    ret = bdrv_open(&target_bs, target, NULL, options, flags, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
@@ -2657,9 +2646,8 @@ void qmp_drive_mirror(const char *device, const char *target,
     BlockDriverState *bs;
     BlockDriverState *source, *target_bs;
     AioContext *aio_context;
-    BlockDriver *drv = NULL;
     Error *local_err = NULL;
-    QDict *options = NULL;
+    QDict *options;
     int flags;
     int64_t size;
     int ret;
@@ -2716,13 +2704,6 @@ void qmp_drive_mirror(const char *device, const char *target,
     if (!has_format) {
         format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
     }
-    if (format) {
-        drv = bdrv_find_format(format);
-        if (!drv) {
-            error_setg(errp, QERR_INVALID_BLOCK_FORMAT, format);
-            goto out;
-        }
-    }
 
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) {
         goto out;
@@ -2777,7 +2758,7 @@ void qmp_drive_mirror(const char *device, const char *target,
         && mode != NEW_IMAGE_MODE_EXISTING)
     {
         /* create new image w/o backing file */
-        assert(format && drv);
+        assert(format);
         bdrv_img_create(target, format,
                         NULL, NULL, NULL, size, flags, &local_err, false);
     } else {
@@ -2801,17 +2782,20 @@ void qmp_drive_mirror(const char *device, const char *target,
         goto out;
     }
 
+    options = qdict_new();
     if (has_node_name) {
-        options = qdict_new();
         qdict_put(options, "node-name", qstring_from_str(node_name));
     }
+    if (format) {
+        qdict_put(options, "driver", qstring_from_str(format));
+    }
 
     /* Mirroring takes care of copy-on-write using the source's backing
      * file.
      */
     target_bs = NULL;
     ret = bdrv_open(&target_bs, target, NULL, options,
-                    flags | BDRV_O_NO_BACKING, drv, &local_err);
+                    flags | BDRV_O_NO_BACKING, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
-- 
2.4.6

[Qemu-devel] [PATCH 2/5] block: Drop drv parameter from bdrv_open()

[Max Reitz]

Now that this parameter is effectively unused, we can drop it and just
pass NULL on to bdrv_open_inherit().

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 9 ++++-----
 block/block-backend.c | 2 +-
 block/parallels.c     | 2 +-
 block/qcow.c          | 2 +-
 block/qcow2.c         | 6 +++---
 block/qed.c           | 2 +-
 block/sheepdog.c      | 5 ++---
 block/vdi.c           | 2 +-
 block/vhdx.c          | 2 +-
 block/vmdk.c          | 7 +++----
 block/vpc.c           | 2 +-
 block/vvfat.c         | 2 +-
 blockdev.c            | 8 ++++----
 include/block/block.h | 3 +--
 14 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/block.c b/block.c
index 193daf9..ac89487 100644
--- a/block.c
+++ b/block.c
@@ -1391,7 +1391,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
     bs_snapshot = bdrv_new();
 
     ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
-                    flags, NULL, &local_err);
+                    flags, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
@@ -1637,11 +1637,10 @@ close_and_fail:
 }
 
 int bdrv_open(BlockDriverState **pbs, const char *filename,
-              const char *reference, QDict *options, int flags,
-              BlockDriver *drv, Error **errp)
+              const char *reference, QDict *options, int flags, Error **errp)
 {
     return bdrv_open_inherit(pbs, filename, reference, options, flags, NULL,
-                             NULL, drv, errp);
+                             NULL, NULL, errp);
 }
 
 typedef struct BlockReopenQueueEntry {
@@ -3846,7 +3845,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
 
             bs = NULL;
             ret = bdrv_open(&bs, full_backing, NULL, backing_options,
-                            back_flags, NULL, &local_err);
+                            back_flags, &local_err);
             g_free(full_backing);
             if (ret < 0) {
                 goto out;
diff --git a/block/block-backend.c b/block/block-backend.c
index aee8a12..c2e8732 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -126,7 +126,7 @@ BlockBackend *blk_new_open(const char *name, const char *filename,
         return NULL;
     }
 
-    ret = bdrv_open(&blk->bs, filename, reference, options, flags, NULL, errp);
+    ret = bdrv_open(&blk->bs, filename, reference, options, flags, errp);
     if (ret < 0) {
         blk_unref(blk);
         return NULL;
diff --git a/block/parallels.c b/block/parallels.c
index 046b568..5cd6ec3 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -476,7 +476,7 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
 
     file = NULL;
     ret = bdrv_open(&file, filename, NULL, NULL,
-                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
+                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
diff --git a/block/qcow.c b/block/qcow.c
index 01fba54..6e35db1 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -793,7 +793,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
 
     qcow_bs = NULL;
     ret = bdrv_open(&qcow_bs, filename, NULL, NULL,
-                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
+                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto cleanup;
diff --git a/block/qcow2.c b/block/qcow2.c
index 867b43b..a707d8d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1975,7 +1975,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
 
     bs = NULL;
     ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
-                    NULL, &local_err);
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
@@ -2038,7 +2038,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     qdict_put(options, "driver", qstring_from_str("qcow2"));
     ret = bdrv_open(&bs, filename, NULL, options,
                     BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
-                    NULL, &local_err);
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
@@ -2092,7 +2092,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     qdict_put(options, "driver", qstring_from_str("qcow2"));
     ret = bdrv_open(&bs, filename, NULL, options,
                     BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
-                    NULL, &local_err);
+                    &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto out;
diff --git a/block/qed.c b/block/qed.c
index 954ed00..a7ff1d9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -583,7 +583,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
 
     bs = NULL;
     ret = bdrv_open(&bs, filename, NULL, NULL,
-                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL, NULL,
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
                     &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 9585beb..67ca788 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1554,7 +1554,7 @@ static int sd_prealloc(const char *filename, Error **errp)
     int ret;
 
     ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
-                    NULL, errp);
+                    errp);
     if (ret < 0) {
         goto out_with_err_set;
     }
@@ -1746,8 +1746,7 @@ static int sd_create(const char *filename, QemuOpts *opts,
         }
 
         bs = NULL;
-        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, NULL,
-                        errp);
+        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, errp);
         if (ret < 0) {
             goto out;
         }
diff --git a/block/vdi.c b/block/vdi.c
index 7642ef3..062a654 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -764,7 +764,7 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
         goto exit;
     }
     ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
-                    NULL, &local_err);
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
diff --git a/block/vhdx.c b/block/vhdx.c
index 0776de7..58f0aae 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1842,7 +1842,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
 
     bs = NULL;
     ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
-                    NULL, &local_err);
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
diff --git a/block/vmdk.c b/block/vmdk.c
index fbaab67..7bdc3d0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1632,7 +1632,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
 
     assert(bs == NULL);
     ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
-                    NULL, &local_err);
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
@@ -1905,8 +1905,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
             ret = -ENOENT;
             goto exit;
         }
-        ret = bdrv_open(&bs, full_backing, NULL, NULL, BDRV_O_NO_BACKING, NULL,
-                        errp);
+        ret = bdrv_open(&bs, full_backing, NULL, NULL, BDRV_O_NO_BACKING, errp);
         g_free(full_backing);
         if (ret != 0) {
             goto exit;
@@ -1977,7 +1976,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
     }
     assert(new_bs == NULL);
     ret = bdrv_open(&new_bs, filename, NULL, NULL,
-                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
+                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
diff --git a/block/vpc.c b/block/vpc.c
index 3e385d9..2b3b518 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -794,7 +794,7 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
         goto out;
     }
     ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
-                    NULL, &local_err);
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
diff --git a/block/vvfat.c b/block/vvfat.c
index bffe8ad..7ddc962 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2962,7 +2962,7 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)
     qdict_put(options, "driver", qstring_from_str("qcow"));
     ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, options,
                     BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
-                    NULL, errp);
+                    errp);
     if (ret < 0) {
         goto err;
     }
diff --git a/blockdev.c b/blockdev.c
index 4528fbe..801858e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1521,7 +1521,7 @@ static void external_snapshot_prepare(BlkTransactionState *common,
      * extended QMP command? */
     assert(state->new_bs == NULL);
     ret = bdrv_open(&state->new_bs, new_image_file, NULL, options,
-                    flags | BDRV_O_NO_BACKING, NULL, &local_err);
+                    flags | BDRV_O_NO_BACKING, &local_err);
     /* We will manually add the backing_hd field to the bs later */
     if (ret != 0) {
         error_propagate(errp, local_err);
@@ -1895,7 +1895,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
         qdict_put(options, "driver", qstring_from_str(format));
     }
 
-    ret = bdrv_open(&bs, filename, NULL, options, bdrv_flags, NULL, &local_err);
+    ret = bdrv_open(&bs, filename, NULL, options, bdrv_flags, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return;
@@ -2540,7 +2540,7 @@ void qmp_drive_backup(const char *device, const char *target,
     }
 
     target_bs = NULL;
-    ret = bdrv_open(&target_bs, target, NULL, options, flags, NULL, &local_err);
+    ret = bdrv_open(&target_bs, target, NULL, options, flags, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
@@ -2795,7 +2795,7 @@ void qmp_drive_mirror(const char *device, const char *target,
      */
     target_bs = NULL;
     ret = bdrv_open(&target_bs, target, NULL, options,
-                    flags | BDRV_O_NO_BACKING, NULL, &local_err);
+                    flags | BDRV_O_NO_BACKING, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
diff --git a/include/block/block.h b/include/block/block.h
index 37916f7..67556c4 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -218,8 +218,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd);
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
 int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp);
 int bdrv_open(BlockDriverState **pbs, const char *filename,
-              const char *reference, QDict *options, int flags,
-              BlockDriver *drv, Error **errp);
+              const char *reference, QDict *options, int flags, Error **errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
                                     BlockDriverState *bs, int flags);
 int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
-- 
2.4.6

[Qemu-devel] [PATCH 3/5] block: Drop drv parameter from bdrv_open_inherit()

[Max Reitz]

Now that this parameter is effectively unused, we can drop it and just
pass NULL to bdrv_fill_options().

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index ac89487..8aa5f25 100644
--- a/block.c
+++ b/block.c
@@ -85,8 +85,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers =
 static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
                              const char *reference, QDict *options, int flags,
                              BlockDriverState *parent,
-                             const BdrvChildRole *child_role,
-                             BlockDriver *drv, Error **errp);
+                             const BdrvChildRole *child_role, Error **errp);
 
 static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
 /* If non-zero, use only whitelisted block drivers */
@@ -1227,8 +1226,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
     assert(bs->backing_hd == NULL);
     ret = bdrv_open_inherit(&backing_hd,
                             *backing_filename ? backing_filename : NULL,
-                            NULL, options, 0, bs, &child_backing,
-                            NULL, &local_err);
+                            NULL, options, 0, bs, &child_backing, &local_err);
     if (ret < 0) {
         bdrv_unref(backing_hd);
         backing_hd = NULL;
@@ -1291,7 +1289,7 @@ BdrvChild *bdrv_open_child(const char *filename,
 
     bs = NULL;
     ret = bdrv_open_inherit(&bs, filename, reference, image_options, 0,
-                            parent, child_role, NULL, errp);
+                            parent, child_role, errp);
     if (ret < 0) {
         goto done;
     }
@@ -1422,11 +1420,11 @@ out:
 static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
                              const char *reference, QDict *options, int flags,
                              BlockDriverState *parent,
-                             const BdrvChildRole *child_role,
-                             BlockDriver *drv, Error **errp)
+                             const BdrvChildRole *child_role, Error **errp)
 {
     int ret;
     BlockDriverState *file = NULL, *bs;
+    BlockDriver *drv = NULL;
     const char *drvname;
     Error *local_err = NULL;
     int snapshot_flags = 0;
@@ -1476,13 +1474,12 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         flags = child_role->inherit_flags(parent->open_flags);
     }
 
-    ret = bdrv_fill_options(&options, &filename, &flags, drv, &local_err);
+    ret = bdrv_fill_options(&options, &filename, &flags, NULL, &local_err);
     if (local_err) {
         goto fail;
     }
 
     /* Find the right image format driver */
-    drv = NULL;
     drvname = qdict_get_try_str(options, "driver");
     if (drvname) {
         drv = bdrv_find_format(drvname);
@@ -1640,7 +1637,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
               const char *reference, QDict *options, int flags, Error **errp)
 {
     return bdrv_open_inherit(pbs, filename, reference, options, flags, NULL,
-                             NULL, NULL, errp);
+                             NULL, errp);
 }
 
 typedef struct BlockReopenQueueEntry {
-- 
2.4.6

[Qemu-devel] [PATCH 4/5] block: Drop drv parameter from bdrv_fill_options()

[Max Reitz]

Now that this parameter is effectively unused, we can drop it and change
the function accordingly.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 59 ++++++++++++++++++++++-------------------------------------
 1 file changed, 22 insertions(+), 37 deletions(-)

diff --git a/block.c b/block.c
index 8aa5f25..434f43c 100644
--- a/block.c
+++ b/block.c
@@ -996,13 +996,13 @@ static QDict *parse_json_filename(const char *filename, Error **errp)
  * block driver has been specified explicitly.
  */
 static int bdrv_fill_options(QDict **options, const char **pfilename,
-                             int *flags, BlockDriver *drv, Error **errp)
+                             int *flags, Error **errp)
 {
     const char *filename = *pfilename;
     const char *drvname;
     bool protocol = *flags & BDRV_O_PROTOCOL;
     bool parse_filename = false;
-    BlockDriver *tmp_drv;
+    BlockDriver *drv = NULL;
     Error *local_err = NULL;
 
     /* Parse json: pseudo-protocol */
@@ -1021,15 +1021,15 @@ static int bdrv_fill_options(QDict **options, const char **pfilename,
     }
 
     drvname = qdict_get_try_str(*options, "driver");
-
-    /* If the user has explicitly specified the driver, this choice should
-     * override the BDRV_O_PROTOCOL flag */
-    tmp_drv = drv;
-    if (!tmp_drv && drvname) {
-        tmp_drv = bdrv_find_format(drvname);
-    }
-    if (tmp_drv) {
-        protocol = tmp_drv->bdrv_file_open;
+    if (drvname) {
+        drv = bdrv_find_format(drvname);
+        if (!drv) {
+            error_setg(errp, "Unknown driver '%s'", drvname);
+            return -ENOENT;
+        }
+        /* If the user has explicitly specified the driver, this choice should
+         * override the BDRV_O_PROTOCOL flag */
+        protocol = drv->bdrv_file_open;
     }
 
     if (protocol) {
@@ -1053,33 +1053,18 @@ static int bdrv_fill_options(QDict **options, const char **pfilename,
     /* Find the right block driver */
     filename = qdict_get_try_str(*options, "filename");
 
-    if (drv) {
-        if (drvname) {
-            error_setg(errp, "Driver specified twice");
-            return -EINVAL;
-        }
-        drvname = drv->format_name;
-        qdict_put(*options, "driver", qstring_from_str(drvname));
-    } else {
-        if (!drvname && protocol) {
-            if (filename) {
-                drv = bdrv_find_protocol(filename, parse_filename, errp);
-                if (!drv) {
-                    return -EINVAL;
-                }
-
-                drvname = drv->format_name;
-                qdict_put(*options, "driver", qstring_from_str(drvname));
-            } else {
-                error_setg(errp, "Must specify either driver or file");
-                return -EINVAL;
-            }
-        } else if (drvname) {
-            drv = bdrv_find_format(drvname);
+    if (!drvname && protocol) {
+        if (filename) {
+            drv = bdrv_find_protocol(filename, parse_filename, errp);
             if (!drv) {
-                error_setg(errp, "Unknown driver '%s'", drvname);
-                return -ENOENT;
+                return -EINVAL;
             }
+
+            drvname = drv->format_name;
+            qdict_put(*options, "driver", qstring_from_str(drvname));
+        } else {
+            error_setg(errp, "Must specify either driver or file");
+            return -EINVAL;
         }
     }
 
@@ -1474,7 +1459,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         flags = child_role->inherit_flags(parent->open_flags);
     }
 
-    ret = bdrv_fill_options(&options, &filename, &flags, NULL, &local_err);
+    ret = bdrv_fill_options(&options, &filename, &flags, &local_err);
     if (local_err) {
         goto fail;
     }
-- 
2.4.6

[Qemu-devel] [PATCH 5/5] block: Drop bdrv_find_whitelisted_format()

[Max Reitz]

It is unused by now, so we can drop it.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 7 -------
 include/block/block.h | 2 --
 2 files changed, 9 deletions(-)

diff --git a/block.c b/block.c
index 434f43c..461eb94 100644
--- a/block.c
+++ b/block.c
@@ -313,13 +313,6 @@ static int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
     return 0;
 }
 
-BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
-                                          bool read_only)
-{
-    BlockDriver *drv = bdrv_find_format(format_name);
-    return drv && bdrv_is_whitelisted(drv, read_only) ? drv : NULL;
-}
-
 typedef struct CreateCo {
     BlockDriver *drv;
     char *filename;
diff --git a/include/block/block.h b/include/block/block.h
index 67556c4..b075f67 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -193,8 +193,6 @@ BlockDriver *bdrv_find_protocol(const char *filename,
                                 bool allow_protocol_prefix,
                                 Error **errp);
 BlockDriver *bdrv_find_format(const char *format_name);
-BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
-                                          bool readonly);
 int bdrv_create(BlockDriver *drv, const char* filename,
                 QemuOpts *opts, Error **errp);
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
-- 
2.4.6

Re: [Qemu-devel] [PATCH 1/5] block: Always pass NULL as drv for bdrv_open()

[Alberto Garcia]

On Wed 26 Aug 2015 07:47:48 PM CEST, Max Reitz <mreitz@redhat.com> wrote:
> Change all callers of bdrv_open() to pass the driver name in the options
> QDict instead of passing its BlockDriver pointer.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH 2/5] block: Drop drv parameter from bdrv_open()

[Alberto Garcia]

On Wed 26 Aug 2015 07:47:49 PM CEST, Max Reitz wrote:

> Now that this parameter is effectively unused, we can drop it and just
> pass NULL on to bdrv_open_inherit().
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH 3/5] block: Drop drv parameter from bdrv_open_inherit()

[Alberto Garcia]

On Wed 26 Aug 2015 07:47:50 PM CEST, Max Reitz wrote:

> Now that this parameter is effectively unused, we can drop it and just
> pass NULL to bdrv_fill_options().
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH 4/5] block: Drop drv parameter from bdrv_fill_options()

[Alberto Garcia]

On Wed 26 Aug 2015 07:47:51 PM CEST, Max Reitz wrote:

> Now that this parameter is effectively unused, we can drop it and
> change the function accordingly.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH 5/5] block: Drop bdrv_find_whitelisted_format()

[Alberto Garcia]

On Wed 26 Aug 2015 07:47:52 PM CEST, Max Reitz wrote:

> It is unused by now, so we can drop it.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH 0/5] block: Drop drv parameter from bdrv_open()

[Kevin Wolf]

Am 26.08.2015 um 19:47 hat Max Reitz geschrieben:
> We don't really need that parameter, so let's drop it. Doing so may even
> fix some bugs, see
> http://lists.nongnu.org/archive/html/qemu-block/2015-08/msg00171.html.
> 
> In the course of writing this series, I had to decide whether the make
> sure all callers of bdrv_find_whitelisted_format() would still only
> accept whitelisted formats, which you'd think would be a good idea; but
> the only caller left was qmp_change_blockdev(), so I guess noone really
> cared about it anymore, instead relying on use_bdrv_whitelist alone.
> 
> So I decided dropped bdrv_find_whitelisted_format() completely. If you
> feel this is a bad decision, feel free to argue but then I guess we'll
> have to reevaluate all bdrv_find_format() calls whether they should
> actually be bdrv_find_whitelisted_format() calls.

Thanks, applied to the block branch.

Kevin