All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open()
@ 2014-02-15  3:30 Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

bdrv_file_open() is now nearly a subset of bdrv_open(), except for the
fact that bdrv_file_open() is for protocols and bdrv_open() for block
drivers. It is possible to use bdrv_file_open() with a block driver, but
in that case that block driver must be explicitly specified.

Due to these great similarities, bdrv_file_open() can be integrated and
made a special case of bdrv_open(). If the flag BDRV_O_PROTOCOL is
specified, bdrv_open() will now do what bdrv_file_open() used to do:
Auto-detecting a protocol instead of a block driver.

This series implements this and changes all calls to bdrv_file_open() to
bdrv_open() calls with BDRV_O_PROTOCOL specified.

Note that this flag cannot be discerned automatically since it is
impossible for bdrv_open() to know by itself whether a given file should
be opened with or without the format layer involved: Both are valid
alternatives. Therefore, it still has to be specified by the user.


v3:
 - patch 1:
   - Kevin: add assert(pbs) and assert(*pbs == NULL) in
     bdrv_open_image()
   - Benoît: remove spurious new line removal
 - patch 2:
   - Benoît: document reference parameter of bdrv_open()
 - patch 3:
   - add the same assertions before every bdrv_file_open() translated to
     bdrv_open() and try not to initialize BDS pointers to NULL but
     rather set them right in front of bdrv_open() as in v2 for patch 1
   - remove addition of *pbs = NULL in bdrv_open_image() in front of the
     bdrv_file_open() being translated to bdrv_open()
 - patch 7:
   - Kevin: remove wrongly added
     "else if (drv->bdrv_needs_filename && !filename)" block
 - patch 8:
   - since patch 3 now does not add the *pbs = NULL anymore, this patch
     does not need to remove it either (I added the Reviewed-bys anyway,
     since this is just about temporary code introduced by this series)

v2:
 - patch 1:
   - added appropriate assertions before every bdrv_open() and
     bdrv_open_image() call where the BDS should be NULL (i.e., where a
     new BDS should be created) and it is not obvious
   - don't initialize BDS pointers to NULL if that is necessary for a
     bdrv_open(); rather, set them to NULL directly before calling
     bdrv_open() in order to make the expected behavior of bdrv_open()
     (create a new BDS) more obvious
   - make bdrv_open_image() behave the same way as bdrv_open() in regard
     to BDS creation/reuse (completed in patch 8)
   - keep "options == NULL" check in bdrv_open() together with the block
     for setting "bs->options"
 - patch 2:
   - due to options now possibly being NULL in the reference handling
     block of bdrv_open(), the condition for "options_non_empty" has to
     be adjusted
   - contextual changes due to patch 1
 - patch 3:
   - adjust comment for BDRV_O_PROTOCOL
   - contextual changes due to the previous patches
 - patch 4 (previously 5):
   - contextual changes due to the previous patches
 - patch 5 (previously 6):
   - bdrv_close() is unnecessary if the BDS could not be opened
 - patch 6:
   - melded together from previous patches 4, 7 and 8
 - patch 7 (previously 9):
   - make the "options" parameter of bdrv_file_open() an indirect
     pointer; if bdrv_file_open() takes the reference, it will be set to
     NULL; otherwise, the caller is responsible for freeing the QDict
 - patch 8 (previously 10):
   - make bdrv_open_image() behave the same way as bdrv_open() in regard
     to BDS creation/reuse
   - contextual changes due to the previous patches


Max Reitz (8):
  block: Change BDS parameter of bdrv_open() to **
  block: Add reference parameter to bdrv_open()
  block: Make bdrv_file_open() static
  block: Reuse reference handling from bdrv_open()
  block: Remove bdrv_new() from bdrv_file_open()
  block: Handle bs->options in bdrv_open() only
  block: Reuse success path from bdrv_open()
  block: Remove bdrv_open_image()'s force_raw option

 block.c               | 228 +++++++++++++++++++++++++-------------------------
 block/blkdebug.c      |   3 +-
 block/blkverify.c     |   6 +-
 block/cow.c           |   5 +-
 block/qcow.c          |   5 +-
 block/qcow2.c         |  18 ++--
 block/qed.c           |   8 +-
 block/sheepdog.c      |   7 +-
 block/vhdx.c          |   4 +-
 block/vmdk.c          |  19 +++--
 block/vvfat.c         |   6 +-
 blockdev.c            |  22 +++--
 hw/block/xen_disk.c   |   4 +-
 include/block/block.h |  13 +--
 qemu-img.c            |  12 ++-
 qemu-io.c             |   8 +-
 qemu-nbd.c            |   2 +-
 17 files changed, 198 insertions(+), 172 deletions(-)

-- 
1.8.5.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v3 1/8] block: Change BDS parameter of bdrv_open() to **
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
@ 2014-02-15  3:30 ` Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 2/8] block: Add reference parameter to bdrv_open() Max Reitz
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

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>
---
 block.c               | 64 ++++++++++++++++++++++++++++++++-------------------
 block/blkdebug.c      |  1 +
 block/blkverify.c     |  2 ++
 block/qcow2.c         | 14 +++++++----
 block/vmdk.c          |  5 ++--
 block/vvfat.c         |  6 ++---
 blockdev.c            | 20 ++++++++--------
 hw/block/xen_disk.c   |  2 +-
 include/block/block.h |  2 +-
 qemu-img.c            | 10 ++++----
 qemu-io.c             |  2 +-
 qemu-nbd.c            |  2 +-
 12 files changed, 73 insertions(+), 57 deletions(-)

diff --git a/block.c b/block.c
index 636aa11..0a9fc68 100644
--- a/block.c
+++ b/block.c
@@ -1040,7 +1040,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);
@@ -1109,8 +1109,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);
     }
@@ -1119,11 +1117,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",
@@ -1160,6 +1158,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,
@@ -1170,6 +1170,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);
@@ -1190,8 +1193,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");
@@ -1199,13 +1200,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
             goto done;
         }
 
-        bs = bdrv_new("");
-        ret = bdrv_open(bs, filename, image_options, flags, NULL, errp);
-        if (ret < 0) {
-            bdrv_unref(bs);
-        } else {
-            *pbs = bs;
-        }
+        ret = bdrv_open(pbs, filename, image_options, flags, NULL, errp);
     } else {
         ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
                              errp);
@@ -1223,17 +1218,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();
@@ -1254,12 +1260,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;
@@ -1316,6 +1321,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);
@@ -1387,6 +1393,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:
@@ -1400,13 +1407,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 (error_is_set(&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 (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
@@ -5288,9 +5306,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("");
-
-            ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
+            bs = NULL;
+            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",
@@ -5298,7 +5315,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);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 8eb0db0..053fa4c 100644
--- a/block/blkdebug.c
+++ b/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) {
diff --git a/block/blkverify.c b/block/blkverify.c
index cfcbcf4..86585e7 100644
--- a/block/blkverify.c
+++ b/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) {
diff --git a/block/qcow2.c b/block/qcow2.c
index 0b4335c..c8e8ba7 100644
--- a/block/qcow2.c
+++ b/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 (error_is_set(&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;
 }
 
diff --git a/block/vmdk.c b/block/vmdk.c
index e809e2e..52cd12a 100644
--- a/block/vmdk.c
+++ b/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("");
-        ret = bdrv_open(bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp);
+        BlockDriverState *bs = NULL;
+        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")) {
diff --git a/block/vvfat.c b/block/vvfat.c
index 664941c..ae7bc6f 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2936,15 +2936,13 @@ static int enable_write_target(BDRVVVFATState *s)
         goto err;
     }
 
-    s->qcow = bdrv_new("");
-
-    ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
+    s->qcow = 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;
     }
 
diff --git a/blockdev.c b/blockdev.c
index 36ceece..6dd351c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -510,7 +510,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",
@@ -1301,12 +1301,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);
     }
@@ -1555,7 +1555,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;
@@ -1991,10 +1991,9 @@ void qmp_drive_backup(const char *device, const char *target,
         return;
     }
 
-    target_bs = bdrv_new("");
-    ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err);
+    target_bs = NULL;
+    ret = bdrv_open(&target_bs, target, NULL, flags, drv, &local_err);
     if (ret < 0) {
-        bdrv_unref(target_bs);
         error_propagate(errp, local_err);
         return;
     }
@@ -2135,11 +2134,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("");
-    ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
+    target_bs = NULL;
+    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;
     }
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 098f6c6..14e6d0b 100644
--- a/hw/block/xen_disk.c
+++ b/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",
diff --git a/include/block/block.h b/include/block/block.h
index 963a61f..980869d 100644
--- a/include/block/block.h
+++ b/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);
diff --git a/qemu-img.c b/qemu-img.c
index c989850..897aa56 100644
--- a/qemu-img.c
+++ b/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",
diff --git a/qemu-io.c b/qemu-io.c
index 7f459d8..8da8f6e 100644
--- a/qemu-io.c
+++ b/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);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 136e8c9..0cf123c 100644
--- a/qemu-nbd.c
+++ b/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],
-- 
1.8.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v3 2/8] block: Add reference parameter to bdrv_open()
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
@ 2014-02-15  3:30 ` Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static Max Reitz
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

Allow bdrv_open() to handle references to existing block devices just as
bdrv_file_open() is already capable of.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c               | 45 ++++++++++++++++++++++++++++++++++++++-------
 block/qcow2.c         |  4 ++--
 block/vmdk.c          |  3 ++-
 block/vvfat.c         |  2 +-
 blockdev.c            | 12 ++++++------
 hw/block/xen_disk.c   |  4 ++--
 include/block/block.h |  5 +++--
 qemu-img.c            |  8 ++++----
 qemu-io.c             |  4 +++-
 qemu-nbd.c            |  2 +-
 10 files changed, 62 insertions(+), 27 deletions(-)

diff --git a/block.c b/block.c
index 0a9fc68..2483fb9 100644
--- a/block.c
+++ b/block.c
@@ -1040,7 +1040,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, NULL, options, flags, drv, &local_err);
         options = NULL;
     } else {
         ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
@@ -1119,7 +1119,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
 
     assert(bs->backing_hd == NULL);
     ret = bdrv_open(&bs->backing_hd,
-                    *backing_filename ? backing_filename : NULL, options,
+                    *backing_filename ? backing_filename : NULL, NULL, options,
                     back_flags, back_drv, &local_err);
     if (ret < 0) {
         bs->backing_hd = NULL;
@@ -1200,7 +1200,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
             goto done;
         }
 
-        ret = bdrv_open(pbs, filename, image_options, flags, NULL, errp);
+        ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
     } else {
         ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
                              errp);
@@ -1221,9 +1221,14 @@ done:
  *
  * 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.
+ *
+ * The reference parameter may be used to specify an existing block device which
+ * should be opened. If specified, neither options nor a filename may be given,
+ * nor can an existing BDS be reused (that is, *pbs has to be NULL).
  */
-int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
-              int flags, BlockDriver *drv, Error **errp)
+int bdrv_open(BlockDriverState **pbs, const char *filename,
+              const char *reference, QDict *options, int flags,
+              BlockDriver *drv, Error **errp)
 {
     int ret;
     /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
@@ -1234,6 +1239,32 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
 
     assert(pbs);
 
+    if (reference) {
+        bool options_non_empty = options ? qdict_size(options) : false;
+        QDECREF(options);
+
+        if (*pbs) {
+            error_setg(errp, "Cannot reuse an existing BDS when referencing "
+                       "another block device");
+            return -EINVAL;
+        }
+
+        if (filename || options_non_empty) {
+            error_setg(errp, "Cannot reference an existing block device with "
+                       "additional options or a new filename");
+            return -EINVAL;
+        }
+
+        bs = bdrv_find(reference);
+        if (!bs) {
+            error_setg(errp, "Cannot find block device '%s'", reference);
+            return -ENODEV;
+        }
+        bdrv_ref(bs);
+        *pbs = bs;
+        return 0;
+    }
+
     if (*pbs) {
         bs = *pbs;
     } else {
@@ -1262,7 +1293,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
         /* Get the required size from the image */
         QINCREF(options);
         bs1 = NULL;
-        ret = bdrv_open(&bs1, filename, options, BDRV_O_NO_BACKING,
+        ret = bdrv_open(&bs1, filename, NULL, options, BDRV_O_NO_BACKING,
                         drv, &local_err);
         if (ret < 0) {
             goto fail;
@@ -5307,7 +5338,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
                 flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
 
             bs = NULL;
-            ret = bdrv_open(&bs, backing_file->value.s, NULL, back_flags,
+            ret = bdrv_open(&bs, backing_file->value.s, NULL, NULL, back_flags,
                             backing_drv, &local_err);
             if (ret < 0) {
                 error_setg_errno(errp, -ret, "Could not open '%s': %s",
diff --git a/block/qcow2.c b/block/qcow2.c
index c8e8ba7..6996276 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1553,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, NULL,
         BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
@@ -1604,7 +1604,7 @@ 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,
+    ret = bdrv_open(&bs, filename, NULL, NULL,
                     BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
                     drv, &local_err);
     if (error_is_set(&local_err)) {
diff --git a/block/vmdk.c b/block/vmdk.c
index 52cd12a..e007793 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1756,7 +1756,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
     }
     if (backing_file) {
         BlockDriverState *bs = NULL;
-        ret = bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp);
+        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_NO_BACKING, NULL,
+                        errp);
         if (ret != 0) {
             goto exit;
         }
diff --git a/block/vvfat.c b/block/vvfat.c
index ae7bc6f..d7a830e 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2937,7 +2937,7 @@ static int enable_write_target(BDRVVVFATState *s)
     }
 
     s->qcow = NULL;
-    ret = bdrv_open(&s->qcow, s->qcow_filename, NULL,
+    ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, NULL,
             BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
             &local_err);
     if (ret < 0) {
diff --git a/blockdev.c b/blockdev.c
index 6dd351c..7b7e349 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -510,7 +510,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, NULL, bs_opts, bdrv_flags, drv, &error);
 
     if (ret < 0) {
         error_setg(errp, "could not open disk image %s: %s",
@@ -1304,7 +1304,7 @@ static void external_snapshot_prepare(BlkTransactionState *common,
     /* 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, options,
+    ret = bdrv_open(&state->new_bs, new_image_file, NULL, options,
                     flags | BDRV_O_NO_BACKING, drv, &local_err);
     /* We will manually add the backing_hd field to the bs later */
     if (ret != 0) {
@@ -1555,7 +1555,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, NULL, bdrv_flags, drv, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return;
@@ -1992,7 +1992,7 @@ void qmp_drive_backup(const char *device, const char *target,
     }
 
     target_bs = NULL;
-    ret = bdrv_open(&target_bs, target, NULL, flags, drv, &local_err);
+    ret = bdrv_open(&target_bs, target, NULL, NULL, flags, drv, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return;
@@ -2135,8 +2135,8 @@ void qmp_drive_mirror(const char *device, const char *target,
      * file.
      */
     target_bs = NULL;
-    ret = bdrv_open(&target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
-                    &local_err);
+    ret = bdrv_open(&target_bs, target, NULL, NULL, flags | BDRV_O_NO_BACKING,
+                    drv, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return;
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 14e6d0b..61e6ff3 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -813,8 +813,8 @@ 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,
-                          blkdev->filename, NULL, qflags, drv, &local_err) != 0)
+            if (bdrv_open(&blkdev->bs, blkdev->filename, NULL, NULL, qflags,
+                          drv, &local_err) != 0)
             {
                 xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
                               error_get_pretty(local_err));
diff --git a/include/block/block.h b/include/block/block.h
index 980869d..a421041 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -190,8 +190,9 @@ 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 **pbs, const char *filename, QDict *options,
-              int flags, BlockDriver *drv, Error **errp);
+int bdrv_open(BlockDriverState **pbs, const char *filename,
+              const char *reference, QDict *options, int flags,
+              BlockDriver *drv, Error **errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
                                     BlockDriverState *bs, int flags);
 int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
diff --git a/qemu-img.c b/qemu-img.c
index 897aa56..b834d52 100644
--- a/qemu-img.c
+++ b/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, NULL, flags, drv, &local_err);
     if (ret < 0) {
         error_report("Could not open '%s': %s", filename,
                      error_get_pretty(local_err));
@@ -2312,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, NULL, BDRV_O_FLAGS,
                         old_backing_drv, &local_err);
         if (ret) {
             error_report("Could not open old backing file '%s': %s",
@@ -2322,8 +2322,8 @@ 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,
-                        new_backing_drv, &local_err);
+            ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL,
+                            BDRV_O_FLAGS, new_backing_drv, &local_err);
             if (ret) {
                 error_report("Could not open new backing file '%s': %s",
                              out_baseimg, error_get_pretty(local_err));
diff --git a/qemu-io.c b/qemu-io.c
index 8da8f6e..61d54c0 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -68,7 +68,9 @@ 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, NULL, 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);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 0cf123c..711162c 100644
--- a/qemu-nbd.c
+++ b/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, NULL, flags, drv, &local_err);
     if (ret < 0) {
         errno = -ret;
         err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind],
-- 
1.8.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 2/8] block: Add reference parameter to bdrv_open() Max Reitz
@ 2014-02-15  3:30 ` Max Reitz
  2014-02-17 13:26   ` Kevin Wolf
  2014-02-18 13:48   ` Benoît Canet
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 4/8] block: Reuse reference handling from bdrv_open() Max Reitz
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

Add the bdrv_open() option BDRV_O_PROTOCOL which results in passing the
call to bdrv_file_open(). Additionally, make bdrv_file_open() static and
therefore bdrv_open() the only way to call it.

Consequently, all existing calls to bdrv_file_open() have to be adjusted
to use bdrv_open() with the BDRV_O_PROTOCOL flag instead.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 16 +++++++++++-----
 block/cow.c           |  5 +++--
 block/qcow.c          |  5 +++--
 block/qcow2.c         |  4 +++-
 block/qed.c           |  8 +++++---
 block/sheepdog.c      |  7 +++++--
 block/vhdx.c          |  4 +++-
 block/vmdk.c          | 13 +++++++++----
 include/block/block.h |  6 +++---
 qemu-io.c             |  4 +++-
 10 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/block.c b/block.c
index 2483fb9..a697bce 100644
--- a/block.c
+++ b/block.c
@@ -953,9 +953,9 @@ free_and_fail:
  * after the call (even on failure), so if the caller intends to reuse the
  * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
  */
-int bdrv_file_open(BlockDriverState **pbs, const char *filename,
-                   const char *reference, QDict *options, int flags,
-                   Error **errp)
+static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+                          const char *reference, QDict *options, int flags,
+                          Error **errp)
 {
     BlockDriverState *bs = NULL;
     BlockDriver *drv;
@@ -1202,8 +1202,8 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
 
         ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
     } else {
-        ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
-                             errp);
+        ret = bdrv_open(pbs, filename, reference, image_options,
+                        flags | BDRV_O_PROTOCOL, NULL, errp);
     }
 
 done:
@@ -1239,6 +1239,12 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
 
     assert(pbs);
 
+    if (flags & BDRV_O_PROTOCOL) {
+        assert(!drv);
+        return bdrv_file_open(pbs, filename, reference, options,
+                              flags & ~BDRV_O_PROTOCOL, errp);
+    }
+
     if (reference) {
         bool options_non_empty = options ? qdict_size(options) : false;
         QDECREF(options);
diff --git a/block/cow.c b/block/cow.c
index 7fc0b12..d0385be 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -351,8 +351,9 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
         return ret;
     }
 
-    ret = bdrv_file_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
-                         &local_err);
+    cow_bs = NULL;
+    ret = bdrv_open(&cow_bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
     if (ret < 0) {
         qerror_report_err(local_err);
         error_free(local_err);
diff --git a/block/qcow.c b/block/qcow.c
index 948b0c5..8d00853 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -691,8 +691,9 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
         return ret;
     }
 
-    ret = bdrv_file_open(&qcow_bs, filename, NULL, NULL, BDRV_O_RDWR,
-                         &local_err);
+    qcow_bs = NULL;
+    ret = bdrv_open(&qcow_bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
     if (ret < 0) {
         qerror_report_err(local_err);
         error_free(local_err);
diff --git a/block/qcow2.c b/block/qcow2.c
index 6996276..395194b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1493,7 +1493,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
         return ret;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+    bs = NULL;
+    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+                    NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
diff --git a/block/qed.c b/block/qed.c
index b9ca7ac..9bc181f 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -562,7 +562,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
     size_t l1_size = header.cluster_size * header.table_size;
     Error *local_err = NULL;
     int ret = 0;
-    BlockDriverState *bs = NULL;
+    BlockDriverState *bs;
 
     ret = bdrv_create_file(filename, NULL, &local_err);
     if (ret < 0) {
@@ -571,8 +571,10 @@ static int qed_create(const char *filename, uint32_t cluster_size,
         return ret;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, NULL,
-                         BDRV_O_RDWR | BDRV_O_CACHE_WB, &local_err);
+    bs = NULL;
+    ret = bdrv_open(&bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL, NULL,
+                    &local_err);
     if (ret < 0) {
         qerror_report_err(local_err);
         error_free(local_err);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 672b9c9..c767aad 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1534,7 +1534,8 @@ static int sd_prealloc(const char *filename)
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+                    NULL, &local_err);
     if (ret < 0) {
         qerror_report_err(local_err);
         error_free(local_err);
@@ -1695,7 +1696,9 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
             goto out;
         }
 
-        ret = bdrv_file_open(&bs, backing_file, NULL, NULL, 0, &local_err);
+        bs = NULL;
+        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, NULL,
+                        &local_err);
         if (ret < 0) {
             qerror_report_err(local_err);
             error_free(local_err);
diff --git a/block/vhdx.c b/block/vhdx.c
index 55689cf..366ff2e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1797,7 +1797,9 @@ static int vhdx_create(const char *filename, QEMUOptionParameter *options,
         goto exit;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+    bs = NULL;
+    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+                    NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
diff --git a/block/vmdk.c b/block/vmdk.c
index e007793..60ed0f2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -776,8 +776,9 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
 
         path_combine(extent_path, sizeof(extent_path),
                 desc_file_path, fname);
-        ret = bdrv_file_open(&extent_file, extent_path, NULL, NULL,
-                             bs->open_flags, errp);
+        extent_file = NULL;
+        ret = bdrv_open(&extent_file, extent_path, NULL, NULL,
+                        bs->open_flags | BDRV_O_PROTOCOL, NULL, errp);
         if (ret) {
             return ret;
         }
@@ -1493,7 +1494,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
         goto exit;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+    assert(bs == NULL);
+    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+                    NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
@@ -1831,7 +1834,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
             goto exit;
         }
     }
-    ret = bdrv_file_open(&new_bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
+    assert(new_bs == NULL);
+    ret = bdrv_open(&new_bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not write description");
         goto exit;
diff --git a/include/block/block.h b/include/block/block.h
index a421041..bf78db5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -102,6 +102,9 @@ typedef enum {
 #define BDRV_O_CHECK       0x1000  /* open solely for consistency check */
 #define BDRV_O_ALLOW_RDWR  0x2000  /* allow reopen to change from r/o to r/w */
 #define BDRV_O_UNMAP       0x4000  /* execute guest UNMAP/TRIM operations */
+#define BDRV_O_PROTOCOL    0x8000  /* if no block driver is explicitly given:
+                                      select an appropriate protocol driver,
+                                      ignoring the format layer */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 
@@ -183,9 +186,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 int bdrv_parse_cache_flags(const char *mode, int *flags);
 int bdrv_parse_discard_flags(const char *mode, int *flags);
-int bdrv_file_open(BlockDriverState **pbs, const char *filename,
-                   const char *reference, QDict *options, int flags,
-                   Error **errp);
 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);
diff --git a/qemu-io.c b/qemu-io.c
index 61d54c0..71d7806 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -59,7 +59,9 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
     }
 
     if (growable) {
-        if (bdrv_file_open(&qemuio_bs, name, NULL, opts, flags, &local_err)) {
+        if (bdrv_open(&qemuio_bs, name, NULL, opts, flags | BDRV_O_PROTOCOL,
+                      NULL, &local_err))
+        {
             fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
                     error_get_pretty(local_err));
             error_free(local_err);
-- 
1.8.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v3 4/8] block: Reuse reference handling from bdrv_open()
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
                   ` (2 preceding siblings ...)
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static Max Reitz
@ 2014-02-15  3:30 ` Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 5/8] block: Remove bdrv_new() from bdrv_file_open() Max Reitz
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

Remove the reference parameter and the related handling code from
bdrv_file_open(), since it exists in bdrv_open() now as well.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

diff --git a/block.c b/block.c
index a697bce..a1adf93 100644
--- a/block.c
+++ b/block.c
@@ -954,8 +954,7 @@ free_and_fail:
  * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
  */
 static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
-                          const char *reference, QDict *options, int flags,
-                          Error **errp)
+                          QDict *options, int flags, Error **errp)
 {
     BlockDriverState *bs = NULL;
     BlockDriver *drv;
@@ -969,24 +968,6 @@ static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
         options = qdict_new();
     }
 
-    if (reference) {
-        if (filename || qdict_size(options)) {
-            error_setg(errp, "Cannot reference an existing block device with "
-                       "additional options or a new filename");
-            return -EINVAL;
-        }
-        QDECREF(options);
-
-        bs = bdrv_find(reference);
-        if (!bs) {
-            error_setg(errp, "Cannot find block device '%s'", reference);
-            return -ENODEV;
-        }
-        bdrv_ref(bs);
-        *pbs = bs;
-        return 0;
-    }
-
     bs = bdrv_new("");
     bs->options = options;
     options = qdict_clone_shallow(options);
@@ -1239,12 +1220,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
 
     assert(pbs);
 
-    if (flags & BDRV_O_PROTOCOL) {
-        assert(!drv);
-        return bdrv_file_open(pbs, filename, reference, options,
-                              flags & ~BDRV_O_PROTOCOL, errp);
-    }
-
     if (reference) {
         bool options_non_empty = options ? qdict_size(options) : false;
         QDECREF(options);
@@ -1271,6 +1246,12 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
         return 0;
     }
 
+    if (flags & BDRV_O_PROTOCOL) {
+        assert(!drv);
+        return bdrv_file_open(pbs, filename, options, flags & ~BDRV_O_PROTOCOL,
+                              errp);
+    }
+
     if (*pbs) {
         bs = *pbs;
     } else {
-- 
1.8.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v3 5/8] block: Remove bdrv_new() from bdrv_file_open()
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
                   ` (3 preceding siblings ...)
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 4/8] block: Reuse reference handling from bdrv_open() Max Reitz
@ 2014-02-15  3:30 ` Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 6/8] block: Handle bs->options in bdrv_open() only Max Reitz
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

Change bdrv_file_open() to take a simple pointer to an already existing
BDS instead of an indirect one. The BDS will be created in bdrv_open()
if necessary.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index a1adf93..7243630 100644
--- a/block.c
+++ b/block.c
@@ -953,10 +953,9 @@ free_and_fail:
  * after the call (even on failure), so if the caller intends to reuse the
  * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
  */
-static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+static int bdrv_file_open(BlockDriverState *bs, const char *filename,
                           QDict *options, int flags, Error **errp)
 {
-    BlockDriverState *bs = NULL;
     BlockDriver *drv;
     const char *drvname;
     bool allow_protocol_prefix = false;
@@ -968,7 +967,6 @@ static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
         options = qdict_new();
     }
 
-    bs = bdrv_new("");
     bs->options = options;
     options = qdict_clone_shallow(options);
 
@@ -1042,7 +1040,6 @@ static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
     QDECREF(options);
 
     bs->growable = 1;
-    *pbs = bs;
     return 0;
 
 fail:
@@ -1050,7 +1047,6 @@ fail:
     if (!bs->drv) {
         QDECREF(bs->options);
     }
-    bdrv_unref(bs);
     return ret;
 }
 
@@ -1246,18 +1242,24 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
         return 0;
     }
 
-    if (flags & BDRV_O_PROTOCOL) {
-        assert(!drv);
-        return bdrv_file_open(pbs, filename, options, flags & ~BDRV_O_PROTOCOL,
-                              errp);
-    }
-
     if (*pbs) {
         bs = *pbs;
     } else {
         bs = bdrv_new("");
     }
 
+    if (flags & BDRV_O_PROTOCOL) {
+        assert(!drv);
+        ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
+                             errp);
+        if (ret && !*pbs) {
+            bdrv_unref(bs);
+        } else if (!ret) {
+            *pbs = bs;
+        }
+        return ret;
+    }
+
     /* NULL means an empty set of options */
     if (options == NULL) {
         options = qdict_new();
-- 
1.8.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v3 6/8] block: Handle bs->options in bdrv_open() only
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
                   ` (4 preceding siblings ...)
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 5/8] block: Remove bdrv_new() from bdrv_file_open() Max Reitz
@ 2014-02-15  3:30 ` Max Reitz
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open() Max Reitz
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

The fail paths of bdrv_file_open() and bdrv_open() naturally exhibit
similarities, thus it is possible to reuse the one from bdrv_open() and
shorten the one in bdrv_file_open() accordingly.

Also, setting bs->options in bdrv_file_open() is not necessary if it is
already done in bdrv_open().

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/block.c b/block.c
index 7243630..c11d4d0 100644
--- a/block.c
+++ b/block.c
@@ -962,14 +962,6 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
     Error *local_err = NULL;
     int ret;
 
-    /* NULL means an empty set of options */
-    if (options == NULL) {
-        options = qdict_new();
-    }
-
-    bs->options = options;
-    options = qdict_clone_shallow(options);
-
     /* Fetch the file name from the options QDict if necessary */
     if (!filename) {
         filename = qdict_get_try_str(options, "filename");
@@ -1044,9 +1036,6 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
 
 fail:
     QDECREF(options);
-    if (!bs->drv) {
-        QDECREF(bs->options);
-    }
     return ret;
 }
 
@@ -1248,18 +1237,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
         bs = bdrv_new("");
     }
 
-    if (flags & BDRV_O_PROTOCOL) {
-        assert(!drv);
-        ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
-                             errp);
-        if (ret && !*pbs) {
-            bdrv_unref(bs);
-        } else if (!ret) {
-            *pbs = bs;
-        }
-        return ret;
-    }
-
     /* NULL means an empty set of options */
     if (options == NULL) {
         options = qdict_new();
@@ -1268,6 +1245,21 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
     bs->options = options;
     options = qdict_clone_shallow(options);
 
+    if (flags & BDRV_O_PROTOCOL) {
+        assert(!drv);
+        ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
+                             &local_err);
+        options = NULL;
+        if (!ret) {
+            *pbs = bs;
+            return 0;
+        } else if (bs->drv) {
+            goto close_and_fail;
+        } else {
+            goto fail;
+        }
+    }
+
     /* For snapshot=on, create a temporary qcow2 overlay */
     if (flags & BDRV_O_SNAPSHOT) {
         BlockDriverState *bs1;
-- 
1.8.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open()
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
                   ` (5 preceding siblings ...)
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 6/8] block: Handle bs->options in bdrv_open() only Max Reitz
@ 2014-02-15  3:30 ` Max Reitz
  2014-02-17 13:34   ` Kevin Wolf
  2014-02-18 13:44   ` Benoît Canet
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 8/8] block: Remove bdrv_open_image()'s force_raw option Max Reitz
  2014-02-17 12:42 ` [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Kevin Wolf
  8 siblings, 2 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

The fail and success paths of bdrv_file_open() may be further shortened
by reusing code already existent in bdrv_open(). This includes
bdrv_file_open() not taking the reference to options which allows the
removal of QDECREF(options) in that function.

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

diff --git a/block.c b/block.c
index c11d4d0..7e2dc81 100644
--- a/block.c
+++ b/block.c
@@ -948,13 +948,15 @@ free_and_fail:
 /*
  * Opens a file using a protocol (file, host_device, nbd, ...)
  *
- * options is a QDict of options to pass to the block drivers, or NULL for an
- * 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_file_open.
+ * options is an indirect pointer to a QDict of options to pass to the block
+ * drivers, or pointer to NULL for an empty set of options. If this function
+ * takes ownership of the QDict reference, it will set *options to NULL;
+ * otherwise, it will contain unused/unrecognized options after this function
+ * returns. Then, the caller is responsible for freeing it. If it intends to
+ * reuse the QDict, QINCREF() should be called beforehand.
  */
 static int bdrv_file_open(BlockDriverState *bs, const char *filename,
-                          QDict *options, int flags, Error **errp)
+                          QDict **options, int flags, Error **errp)
 {
     BlockDriver *drv;
     const char *drvname;
@@ -964,9 +966,9 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
 
     /* Fetch the file name from the options QDict if necessary */
     if (!filename) {
-        filename = qdict_get_try_str(options, "filename");
-    } else if (filename && !qdict_haskey(options, "filename")) {
-        qdict_put(options, "filename", qstring_from_str(filename));
+        filename = qdict_get_try_str(*options, "filename");
+    } else if (filename && !qdict_haskey(*options, "filename")) {
+        qdict_put(*options, "filename", qstring_from_str(filename));
         allow_protocol_prefix = true;
     } else {
         error_setg(errp, "Can't specify 'file' and 'filename' options at the "
@@ -976,13 +978,13 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
     }
 
     /* Find the right block driver */
-    drvname = qdict_get_try_str(options, "driver");
+    drvname = qdict_get_try_str(*options, "driver");
     if (drvname) {
         drv = bdrv_find_format(drvname);
         if (!drv) {
             error_setg(errp, "Unknown driver '%s'", drvname);
         }
-        qdict_del(options, "driver");
+        qdict_del(*options, "driver");
     } else if (filename) {
         drv = bdrv_find_protocol(filename, allow_protocol_prefix);
         if (!drv) {
@@ -1001,41 +1003,30 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
 
     /* Parse the filename and open it */
     if (drv->bdrv_parse_filename && filename) {
-        drv->bdrv_parse_filename(filename, options, &local_err);
+        drv->bdrv_parse_filename(filename, *options, &local_err);
         if (error_is_set(&local_err)) {
             error_propagate(errp, local_err);
             ret = -EINVAL;
             goto fail;
         }
-        qdict_del(options, "filename");
+        qdict_del(*options, "filename");
     }
 
     if (!drv->bdrv_file_open) {
-        ret = bdrv_open(&bs, filename, NULL, options, flags, drv, &local_err);
-        options = NULL;
+        ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err);
+        *options = NULL;
     } else {
-        ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
+        ret = bdrv_open_common(bs, NULL, *options, flags, drv, &local_err);
     }
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto fail;
     }
 
-    /* Check if any unknown options were used */
-    if (options && (qdict_size(options) != 0)) {
-        const QDictEntry *entry = qdict_first(options);
-        error_setg(errp, "Block protocol '%s' doesn't support the option '%s'",
-                   drv->format_name, entry->key);
-        ret = -EINVAL;
-        goto fail;
-    }
-    QDECREF(options);
-
     bs->growable = 1;
     return 0;
 
 fail:
-    QDECREF(options);
     return ret;
 }
 
@@ -1247,12 +1238,10 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
 
     if (flags & BDRV_O_PROTOCOL) {
         assert(!drv);
-        ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
+        ret = bdrv_file_open(bs, filename, &options, flags & ~BDRV_O_PROTOCOL,
                              &local_err);
-        options = NULL;
         if (!ret) {
-            *pbs = bs;
-            return 0;
+            goto done;
         } else if (bs->drv) {
             goto close_and_fail;
         } else {
@@ -1389,12 +1378,18 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
         }
     }
 
+done:
     /* Check if any unknown options were used */
-    if (qdict_size(options) != 0) {
+    if (options && (qdict_size(options) != 0)) {
         const QDictEntry *entry = qdict_first(options);
-        error_setg(errp, "Block format '%s' used by device '%s' doesn't "
-                   "support the option '%s'", drv->format_name, bs->device_name,
-                   entry->key);
+        if (flags & BDRV_O_PROTOCOL) {
+            error_setg(errp, "Block protocol '%s' doesn't support the option "
+                       "'%s'", drv->format_name, entry->key);
+        } else {
+            error_setg(errp, "Block format '%s' used by device '%s' doesn't "
+                       "support the option '%s'", drv->format_name,
+                       bs->device_name, entry->key);
+        }
 
         ret = -EINVAL;
         goto close_and_fail;
-- 
1.8.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH v3 8/8] block: Remove bdrv_open_image()'s force_raw option
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
                   ` (6 preceding siblings ...)
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open() Max Reitz
@ 2014-02-15  3:30 ` Max Reitz
  2014-02-17 12:42 ` [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Kevin Wolf
  8 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2014-02-15  3:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Jeff Cody, Benoît Canet, Stefan Hajnoczi, Max Reitz

This option is now unnecessary since specifying BDRV_O_PROTOCOL as flag
will do exactly the same.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c               | 27 ++++-----------------------
 block/blkdebug.c      |  2 +-
 block/blkverify.c     |  4 ++--
 include/block/block.h |  2 +-
 4 files changed, 8 insertions(+), 27 deletions(-)

diff --git a/block.c b/block.c
index 7e2dc81..62161bd 100644
--- a/block.c
+++ b/block.c
@@ -1102,10 +1102,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
  * Opens a disk image whose options are given as BlockdevRef in another block
  * device's options.
  *
- * If force_raw is true, bdrv_file_open() will be used, thereby preventing any
- * image format auto-detection. If it is false and a filename is given,
- * bdrv_open() will be used for auto-detection.
- *
  * If allow_none is true, no image will be opened if filename is false and no
  * BlockdevRef is given. *pbs will remain unchanged and 0 will be returned.
  *
@@ -1120,7 +1116,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
  */
 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)
+                    bool allow_none, Error **errp)
 {
     QDict *image_options;
     int ret;
@@ -1146,22 +1142,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
         goto done;
     }
 
-    if (filename && !force_raw) {
-        /* 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. */
-        if (reference) {
-            error_setg(errp, "Cannot reference an existing block device while "
-                       "giving a filename");
-            ret = -EINVAL;
-            goto done;
-        }
-
-        ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
-    } else {
-        ret = bdrv_open(pbs, filename, reference, image_options,
-                        flags | BDRV_O_PROTOCOL, NULL, errp);
-    }
+    ret = bdrv_open(pbs, filename, reference, image_options, flags, NULL, errp);
 
 done:
     qdict_del(options, bdref_key);
@@ -1324,8 +1305,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
 
     assert(file == NULL);
     ret = bdrv_open_image(&file, filename, options, "file",
-                          bdrv_open_flags(bs, flags | BDRV_O_UNMAP), true, true,
-                          &local_err);
+                          bdrv_open_flags(bs, flags | BDRV_O_UNMAP) |
+                          BDRV_O_PROTOCOL, true, &local_err);
     if (ret < 0) {
         goto fail;
     }
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 053fa4c..6707f49 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -412,7 +412,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
     /* 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);
+                          flags | BDRV_O_PROTOCOL, false, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto out;
diff --git a/block/blkverify.c b/block/blkverify.c
index 86585e7..b57da59 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -137,7 +137,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);
+                          "raw", flags | BDRV_O_PROTOCOL, false, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto fail;
@@ -146,7 +146,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);
+                          "test", flags, false, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         s->test_file = NULL;
diff --git a/include/block/block.h b/include/block/block.h
index bf78db5..780f48b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -188,7 +188,7 @@ int bdrv_parse_cache_flags(const char *mode, int *flags);
 int bdrv_parse_discard_flags(const char *mode, int *flags);
 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);
+                    bool allow_none, Error **errp);
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
 int bdrv_open(BlockDriverState **pbs, const char *filename,
               const char *reference, QDict *options, int flags,
-- 
1.8.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open()
  2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
                   ` (7 preceding siblings ...)
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 8/8] block: Remove bdrv_open_image()'s force_raw option Max Reitz
@ 2014-02-17 12:42 ` Kevin Wolf
  2014-02-17 13:35   ` Benoît Canet
  8 siblings, 1 reply; 15+ messages in thread
From: Kevin Wolf @ 2014-02-17 12:42 UTC (permalink / raw)
  To: Max Reitz; +Cc: Jeff Cody, qemu-devel, Stefan Hajnoczi, Benoît Canet

Am 15.02.2014 um 04:30 hat Max Reitz geschrieben:
> bdrv_file_open() is now nearly a subset of bdrv_open(), except for the
> fact that bdrv_file_open() is for protocols and bdrv_open() for block
> drivers. It is possible to use bdrv_file_open() with a block driver, but
> in that case that block driver must be explicitly specified.
> 
> Due to these great similarities, bdrv_file_open() can be integrated and
> made a special case of bdrv_open(). If the flag BDRV_O_PROTOCOL is
> specified, bdrv_open() will now do what bdrv_file_open() used to do:
> Auto-detecting a protocol instead of a block driver.
> 
> This series implements this and changes all calls to bdrv_file_open() to
> bdrv_open() calls with BDRV_O_PROTOCOL specified.
> 
> Note that this flag cannot be discerned automatically since it is
> impossible for bdrv_open() to know by itself whether a given file should
> be opened with or without the format layer involved: Both are valid
> alternatives. Therefore, it still has to be specified by the user.

This series conflicts with Benoît's patches that have been merged into
master. When rebasing, please be careful with the code motion patch so
that you don't accidentally revert Benoît's changes. (It's an easy
conflict to resolve, but not trivial enough for me to do it while
applying the patch, with no additional review.)

Kevin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static Max Reitz
@ 2014-02-17 13:26   ` Kevin Wolf
  2014-02-18 13:48   ` Benoît Canet
  1 sibling, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2014-02-17 13:26 UTC (permalink / raw)
  To: Max Reitz; +Cc: Jeff Cody, qemu-devel, Stefan Hajnoczi, Benoît Canet

Am 15.02.2014 um 04:30 hat Max Reitz geschrieben:
> Add the bdrv_open() option BDRV_O_PROTOCOL which results in passing the
> call to bdrv_file_open(). Additionally, make bdrv_file_open() static and
> therefore bdrv_open() the only way to call it.
> 
> Consequently, all existing calls to bdrv_file_open() have to be adjusted
> to use bdrv_open() with the BDRV_O_PROTOCOL flag instead.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open()
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open() Max Reitz
@ 2014-02-17 13:34   ` Kevin Wolf
  2014-02-18 13:44   ` Benoît Canet
  1 sibling, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2014-02-17 13:34 UTC (permalink / raw)
  To: Max Reitz; +Cc: Jeff Cody, qemu-devel, Stefan Hajnoczi, Benoît Canet

Am 15.02.2014 um 04:30 hat Max Reitz geschrieben:
> The fail and success paths of bdrv_file_open() may be further shortened
> by reusing code already existent in bdrv_open(). This includes
> bdrv_file_open() not taking the reference to options which allows the
> removal of QDECREF(options) in that function.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open()
  2014-02-17 12:42 ` [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Kevin Wolf
@ 2014-02-17 13:35   ` Benoît Canet
  0 siblings, 0 replies; 15+ messages in thread
From: Benoît Canet @ 2014-02-17 13:35 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: Jeff Cody, qemu-devel, Stefan Hajnoczi, Max Reitz

The Monday 17 Feb 2014 à 13:42:43 (+0100), Kevin Wolf wrote :
> Am 15.02.2014 um 04:30 hat Max Reitz geschrieben:
> > bdrv_file_open() is now nearly a subset of bdrv_open(), except for the
> > fact that bdrv_file_open() is for protocols and bdrv_open() for block
> > drivers. It is possible to use bdrv_file_open() with a block driver, but
> > in that case that block driver must be explicitly specified.
> > 
> > Due to these great similarities, bdrv_file_open() can be integrated and
> > made a special case of bdrv_open(). If the flag BDRV_O_PROTOCOL is
> > specified, bdrv_open() will now do what bdrv_file_open() used to do:
> > Auto-detecting a protocol instead of a block driver.
> > 
> > This series implements this and changes all calls to bdrv_file_open() to
> > bdrv_open() calls with BDRV_O_PROTOCOL specified.
> > 
> > Note that this flag cannot be discerned automatically since it is
> > impossible for bdrv_open() to know by itself whether a given file should
> > be opened with or without the format layer involved: Both are valid
> > alternatives. Therefore, it still has to be specified by the user.
> 
> This series conflicts with Benoît's patches that have been merged into
> master. When rebasing, please be careful with the code motion patch so
> that you don't accidentally revert Benoît's changes. (It's an easy
> conflict to resolve, but not trivial enough for me to do it while
> applying the patch, with no additional review.)
> 
> Kevin

I will do another review once Max has rebased and resent this series.

Best regards

Benoît

> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open()
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open() Max Reitz
  2014-02-17 13:34   ` Kevin Wolf
@ 2014-02-18 13:44   ` Benoît Canet
  1 sibling, 0 replies; 15+ messages in thread
From: Benoît Canet @ 2014-02-18 13:44 UTC (permalink / raw)
  To: Max Reitz; +Cc: Kevin Wolf, Jeff Cody, qemu-devel, Stefan Hajnoczi

The Saturday 15 Feb 2014 à 04:30:23 (+0100), Max Reitz wrote :
> The fail and success paths of bdrv_file_open() may be further shortened
> by reusing code already existent in bdrv_open(). This includes
> bdrv_file_open() not taking the reference to options which allows the
> removal of QDECREF(options) in that function.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c | 63 +++++++++++++++++++++++++++++----------------------------------
>  1 file changed, 29 insertions(+), 34 deletions(-)
> 
> diff --git a/block.c b/block.c
> index c11d4d0..7e2dc81 100644
> --- a/block.c
> +++ b/block.c
> @@ -948,13 +948,15 @@ free_and_fail:
>  /*
>   * Opens a file using a protocol (file, host_device, nbd, ...)
>   *
> - * options is a QDict of options to pass to the block drivers, or NULL for an
> - * 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_file_open.
> + * options is an indirect pointer to a QDict of options to pass to the block
> + * drivers, or pointer to NULL for an empty set of options. If this function
> + * takes ownership of the QDict reference, it will set *options to NULL;
> + * otherwise, it will contain unused/unrecognized options after this function
> + * returns. Then, the caller is responsible for freeing it. If it intends to
> + * reuse the QDict, QINCREF() should be called beforehand.
>   */
>  static int bdrv_file_open(BlockDriverState *bs, const char *filename,
> -                          QDict *options, int flags, Error **errp)
> +                          QDict **options, int flags, Error **errp)
>  {
>      BlockDriver *drv;
>      const char *drvname;
> @@ -964,9 +966,9 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
>  
>      /* Fetch the file name from the options QDict if necessary */
>      if (!filename) {
> -        filename = qdict_get_try_str(options, "filename");
> -    } else if (filename && !qdict_haskey(options, "filename")) {
> -        qdict_put(options, "filename", qstring_from_str(filename));
> +        filename = qdict_get_try_str(*options, "filename");
> +    } else if (filename && !qdict_haskey(*options, "filename")) {
> +        qdict_put(*options, "filename", qstring_from_str(filename));
>          allow_protocol_prefix = true;
>      } else {
>          error_setg(errp, "Can't specify 'file' and 'filename' options at the "
> @@ -976,13 +978,13 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
>      }
>  
>      /* Find the right block driver */
> -    drvname = qdict_get_try_str(options, "driver");
> +    drvname = qdict_get_try_str(*options, "driver");
>      if (drvname) {
>          drv = bdrv_find_format(drvname);
>          if (!drv) {
>              error_setg(errp, "Unknown driver '%s'", drvname);
>          }
> -        qdict_del(options, "driver");
> +        qdict_del(*options, "driver");
>      } else if (filename) {
>          drv = bdrv_find_protocol(filename, allow_protocol_prefix);
>          if (!drv) {
> @@ -1001,41 +1003,30 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
>  
>      /* Parse the filename and open it */
>      if (drv->bdrv_parse_filename && filename) {
> -        drv->bdrv_parse_filename(filename, options, &local_err);
> +        drv->bdrv_parse_filename(filename, *options, &local_err);
>          if (error_is_set(&local_err)) {
>              error_propagate(errp, local_err);
>              ret = -EINVAL;
>              goto fail;
>          }
> -        qdict_del(options, "filename");
> +        qdict_del(*options, "filename");
>      }
>  
>      if (!drv->bdrv_file_open) {
> -        ret = bdrv_open(&bs, filename, NULL, options, flags, drv, &local_err);
> -        options = NULL;
> +        ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err);
> +        *options = NULL;
>      } else {
> -        ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
> +        ret = bdrv_open_common(bs, NULL, *options, flags, drv, &local_err);
>      }
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          goto fail;
>      }
>  
> -    /* Check if any unknown options were used */
> -    if (options && (qdict_size(options) != 0)) {
> -        const QDictEntry *entry = qdict_first(options);
> -        error_setg(errp, "Block protocol '%s' doesn't support the option '%s'",
> -                   drv->format_name, entry->key);
> -        ret = -EINVAL;
> -        goto fail;
> -    }
> -    QDECREF(options);
> -
>      bs->growable = 1;
>      return 0;
>  
>  fail:
> -    QDECREF(options);
>      return ret;
>  }
>  
> @@ -1247,12 +1238,10 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
>  
>      if (flags & BDRV_O_PROTOCOL) {
>          assert(!drv);
> -        ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
> +        ret = bdrv_file_open(bs, filename, &options, flags & ~BDRV_O_PROTOCOL,
>                               &local_err);
> -        options = NULL;
>          if (!ret) {
> -            *pbs = bs;
> -            return 0;
> +            goto done;
>          } else if (bs->drv) {
>              goto close_and_fail;
>          } else {
> @@ -1389,12 +1378,18 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
>          }
>      }
>  
> +done:
>      /* Check if any unknown options were used */
> -    if (qdict_size(options) != 0) {
> +    if (options && (qdict_size(options) != 0)) {
>          const QDictEntry *entry = qdict_first(options);
> -        error_setg(errp, "Block format '%s' used by device '%s' doesn't "
> -                   "support the option '%s'", drv->format_name, bs->device_name,
> -                   entry->key);
> +        if (flags & BDRV_O_PROTOCOL) {
> +            error_setg(errp, "Block protocol '%s' doesn't support the option "
> +                       "'%s'", drv->format_name, entry->key);
> +        } else {
> +            error_setg(errp, "Block format '%s' used by device '%s' doesn't "
> +                       "support the option '%s'", drv->format_name,
> +                       bs->device_name, entry->key);
> +        }
>  
>          ret = -EINVAL;
>          goto close_and_fail;
> -- 
> 1.8.5.4
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static
  2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static Max Reitz
  2014-02-17 13:26   ` Kevin Wolf
@ 2014-02-18 13:48   ` Benoît Canet
  1 sibling, 0 replies; 15+ messages in thread
From: Benoît Canet @ 2014-02-18 13:48 UTC (permalink / raw)
  To: Max Reitz; +Cc: Kevin Wolf, Jeff Cody, qemu-devel, Stefan Hajnoczi

The Saturday 15 Feb 2014 à 04:30:19 (+0100), Max Reitz wrote :
> Add the bdrv_open() option BDRV_O_PROTOCOL which results in passing the
> call to bdrv_file_open(). Additionally, make bdrv_file_open() static and
> therefore bdrv_open() the only way to call it.
> 
> Consequently, all existing calls to bdrv_file_open() have to be adjusted
> to use bdrv_open() with the BDRV_O_PROTOCOL flag instead.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c               | 16 +++++++++++-----
>  block/cow.c           |  5 +++--
>  block/qcow.c          |  5 +++--
>  block/qcow2.c         |  4 +++-
>  block/qed.c           |  8 +++++---
>  block/sheepdog.c      |  7 +++++--
>  block/vhdx.c          |  4 +++-
>  block/vmdk.c          | 13 +++++++++----
>  include/block/block.h |  6 +++---
>  qemu-io.c             |  4 +++-
>  10 files changed, 48 insertions(+), 24 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 2483fb9..a697bce 100644
> --- a/block.c
> +++ b/block.c
> @@ -953,9 +953,9 @@ free_and_fail:
>   * after the call (even on failure), so if the caller intends to reuse the
>   * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
>   */
> -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> -                   const char *reference, QDict *options, int flags,
> -                   Error **errp)
> +static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> +                          const char *reference, QDict *options, int flags,
> +                          Error **errp)
>  {
>      BlockDriverState *bs = NULL;
>      BlockDriver *drv;
> @@ -1202,8 +1202,8 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
>  
>          ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
>      } else {
> -        ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
> -                             errp);
> +        ret = bdrv_open(pbs, filename, reference, image_options,
> +                        flags | BDRV_O_PROTOCOL, NULL, errp);
>      }
>  
>  done:
> @@ -1239,6 +1239,12 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
>  
>      assert(pbs);
>  
> +    if (flags & BDRV_O_PROTOCOL) {
> +        assert(!drv);
> +        return bdrv_file_open(pbs, filename, reference, options,
> +                              flags & ~BDRV_O_PROTOCOL, errp);
> +    }
> +
>      if (reference) {
>          bool options_non_empty = options ? qdict_size(options) : false;
>          QDECREF(options);
> diff --git a/block/cow.c b/block/cow.c
> index 7fc0b12..d0385be 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -351,8 +351,9 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
>          return ret;
>      }
>  
> -    ret = bdrv_file_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
> -                         &local_err);
> +    cow_bs = NULL;
> +    ret = bdrv_open(&cow_bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
>      if (ret < 0) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> diff --git a/block/qcow.c b/block/qcow.c
> index 948b0c5..8d00853 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -691,8 +691,9 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
>          return ret;
>      }
>  
> -    ret = bdrv_file_open(&qcow_bs, filename, NULL, NULL, BDRV_O_RDWR,
> -                         &local_err);
> +    qcow_bs = NULL;
> +    ret = bdrv_open(&qcow_bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
>      if (ret < 0) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 6996276..395194b 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1493,7 +1493,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>          return ret;
>      }
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    bs = NULL;
> +    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +                    NULL, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          return ret;
> diff --git a/block/qed.c b/block/qed.c
> index b9ca7ac..9bc181f 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -562,7 +562,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
>      size_t l1_size = header.cluster_size * header.table_size;
>      Error *local_err = NULL;
>      int ret = 0;
> -    BlockDriverState *bs = NULL;
> +    BlockDriverState *bs;
>  
>      ret = bdrv_create_file(filename, NULL, &local_err);
>      if (ret < 0) {
> @@ -571,8 +571,10 @@ static int qed_create(const char *filename, uint32_t cluster_size,
>          return ret;
>      }
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL,
> -                         BDRV_O_RDWR | BDRV_O_CACHE_WB, &local_err);
> +    bs = NULL;
> +    ret = bdrv_open(&bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL, NULL,
> +                    &local_err);
>      if (ret < 0) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 672b9c9..c767aad 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1534,7 +1534,8 @@ static int sd_prealloc(const char *filename)
>      Error *local_err = NULL;
>      int ret;
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +                    NULL, &local_err);
>      if (ret < 0) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> @@ -1695,7 +1696,9 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
>              goto out;
>          }
>  
> -        ret = bdrv_file_open(&bs, backing_file, NULL, NULL, 0, &local_err);
> +        bs = NULL;
> +        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, NULL,
> +                        &local_err);
>          if (ret < 0) {
>              qerror_report_err(local_err);
>              error_free(local_err);
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 55689cf..366ff2e 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1797,7 +1797,9 @@ static int vhdx_create(const char *filename, QEMUOptionParameter *options,
>          goto exit;
>      }
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    bs = NULL;
> +    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +                    NULL, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          goto exit;
> diff --git a/block/vmdk.c b/block/vmdk.c
> index e007793..60ed0f2 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -776,8 +776,9 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>  
>          path_combine(extent_path, sizeof(extent_path),
>                  desc_file_path, fname);
> -        ret = bdrv_file_open(&extent_file, extent_path, NULL, NULL,
> -                             bs->open_flags, errp);
> +        extent_file = NULL;
> +        ret = bdrv_open(&extent_file, extent_path, NULL, NULL,
> +                        bs->open_flags | BDRV_O_PROTOCOL, NULL, errp);
>          if (ret) {
>              return ret;
>          }
> @@ -1493,7 +1494,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
>          goto exit;
>      }
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    assert(bs == NULL);
> +    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +                    NULL, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          goto exit;
> @@ -1831,7 +1834,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
>              goto exit;
>          }
>      }
> -    ret = bdrv_file_open(&new_bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    assert(new_bs == NULL);
> +    ret = bdrv_open(&new_bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
>      if (ret < 0) {
>          error_setg_errno(errp, -ret, "Could not write description");
>          goto exit;
> diff --git a/include/block/block.h b/include/block/block.h
> index a421041..bf78db5 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -102,6 +102,9 @@ typedef enum {
>  #define BDRV_O_CHECK       0x1000  /* open solely for consistency check */
>  #define BDRV_O_ALLOW_RDWR  0x2000  /* allow reopen to change from r/o to r/w */
>  #define BDRV_O_UNMAP       0x4000  /* execute guest UNMAP/TRIM operations */
> +#define BDRV_O_PROTOCOL    0x8000  /* if no block driver is explicitly given:
> +                                      select an appropriate protocol driver,
> +                                      ignoring the format layer */
>  
>  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
>  
> @@ -183,9 +186,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
>  void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
>  int bdrv_parse_cache_flags(const char *mode, int *flags);
>  int bdrv_parse_discard_flags(const char *mode, int *flags);
> -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> -                   const char *reference, QDict *options, int flags,
> -                   Error **errp);
>  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);
> diff --git a/qemu-io.c b/qemu-io.c
> index 61d54c0..71d7806 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -59,7 +59,9 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
>      }
>  
>      if (growable) {
> -        if (bdrv_file_open(&qemuio_bs, name, NULL, opts, flags, &local_err)) {
> +        if (bdrv_open(&qemuio_bs, name, NULL, opts, flags | BDRV_O_PROTOCOL,
> +                      NULL, &local_err))
> +        {
>              fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
>                      error_get_pretty(local_err));
>              error_free(local_err);
> -- 
> 1.8.5.4
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-02-18 13:48 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-15  3:30 [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 2/8] block: Add reference parameter to bdrv_open() Max Reitz
2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 3/8] block: Make bdrv_file_open() static Max Reitz
2014-02-17 13:26   ` Kevin Wolf
2014-02-18 13:48   ` Benoît Canet
2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 4/8] block: Reuse reference handling from bdrv_open() Max Reitz
2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 5/8] block: Remove bdrv_new() from bdrv_file_open() Max Reitz
2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 6/8] block: Handle bs->options in bdrv_open() only Max Reitz
2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 7/8] block: Reuse success path from bdrv_open() Max Reitz
2014-02-17 13:34   ` Kevin Wolf
2014-02-18 13:44   ` Benoît Canet
2014-02-15  3:30 ` [Qemu-devel] [PATCH v3 8/8] block: Remove bdrv_open_image()'s force_raw option Max Reitz
2014-02-17 12:42 ` [Qemu-devel] [PATCH v3 0/8] block: Integrate bdrv_file_open() into bdrv_open() Kevin Wolf
2014-02-17 13:35   ` Benoît Canet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.