958e1b
From bfec9b88263e72c7425df04601bfcae5c06dca23 Mon Sep 17 00:00:00 2001
958e1b
From: Kevin Wolf <kwolf@redhat.com>
958e1b
Date: Thu, 23 Oct 2014 10:10:06 +0200
958e1b
Subject: [PATCH 06/19] block: Add errp to bdrv_new()
958e1b
958e1b
Message-id: <1414059011-15516-4-git-send-email-kwolf@redhat.com>
958e1b
Patchwork-id: 61837
958e1b
O-Subject: [RHEL-7.1 qemu-kvm PATCH v2 3/8] block: Add errp to bdrv_new()
958e1b
Bugzilla: 1088176
958e1b
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
958e1b
RH-Acked-by: Markus Armbruster <armbru@redhat.com>
958e1b
RH-Acked-by: Max Reitz <mreitz@redhat.com>
958e1b
958e1b
This patch adds an errp parameter to bdrv_new() and updates all its
958e1b
callers. The next patches will make use of this in order to check for
958e1b
duplicate IDs. Most of the callers know that their ID is fine, so they
958e1b
can simply assert that there is no error.
958e1b
958e1b
Behaviour doesn't change with this patch yet as bdrv_new() doesn't
958e1b
actually assign errors to errp.
958e1b
958e1b
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
958e1b
Reviewed-by: Eric Blake <eblake@redhat.com>
958e1b
(cherry picked from commit 98522f63f40adaebc412481e1d2e9170160d4539)
958e1b
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
958e1b
958e1b
Conflicts:
958e1b
	block.c
958e1b
	qemu-img.c
958e1b
	qemu-io.c
958e1b
958e1b
In RHEL 7, we have a different set of bdrv_new() callers. Fortunately,
958e1b
the additional ones compared to upstream are trivial: Either we know
958e1b
for sure that their name is unique  (e.g. fixed names used in qemu-img)
958e1b
and therefore bdrv_new() will never fail even after the final patch of
958e1b
this series ('block: Catch duplicate IDs in bdrv_new()'), or they use an
958e1b
empty name "" which can't conflict either.
958e1b
958e1b
This patch leaks local_err in xen_disk, which was fixed in upstream
958e1b
commit cedccf13. Downstream, we don't care about Xen, so it will remain
958e1b
unfixed.
958e1b
958e1b
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
958e1b
---
958e1b
 block.c               | 10 +++++-----
958e1b
 block/blkverify.c     |  2 +-
958e1b
 block/iscsi.c         |  2 +-
958e1b
 block/vmdk.c          |  2 +-
958e1b
 block/vvfat.c         |  4 ++--
958e1b
 blockdev.c            | 13 +++++++++----
958e1b
 hw/block/xen_disk.c   |  7 +++++--
958e1b
 include/block/block.h |  2 +-
958e1b
 qemu-img.c            |  6 +++---
958e1b
 qemu-io.c             |  2 +-
958e1b
 qemu-nbd.c            |  3 ++-
958e1b
 11 files changed, 31 insertions(+), 22 deletions(-)
958e1b
958e1b
diff --git a/block.c b/block.c
958e1b
index 496eb72..eb0810e 100644
958e1b
--- a/block.c
958e1b
+++ b/block.c
958e1b
@@ -300,7 +300,7 @@ void bdrv_register(BlockDriver *bdrv)
958e1b
 }
958e1b
 
958e1b
 /* create a new block device (by default it is empty) */
958e1b
-BlockDriverState *bdrv_new(const char *device_name)
958e1b
+BlockDriverState *bdrv_new(const char *device_name, Error **errp)
958e1b
 {
958e1b
     BlockDriverState *bs;
958e1b
 
958e1b
@@ -892,7 +892,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
958e1b
         options = qdict_new();
958e1b
     }
958e1b
 
958e1b
-    bs = bdrv_new("");
958e1b
+    bs = bdrv_new("", &error_abort);
958e1b
     bs->options = options;
958e1b
     options = qdict_clone_shallow(options);
958e1b
 
958e1b
@@ -1014,7 +1014,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
958e1b
                                        sizeof(backing_filename));
958e1b
     }
958e1b
 
958e1b
-    bs->backing_hd = bdrv_new("");
958e1b
+    bs->backing_hd = bdrv_new("", &error_abort);
958e1b
 
958e1b
     if (bs->backing_format[0] != '\0') {
958e1b
         back_drv = bdrv_find_format(bs->backing_format);
958e1b
@@ -1111,7 +1111,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
958e1b
            instead of opening 'filename' directly */
958e1b
 
958e1b
         /* if there is a backing file, use it */
958e1b
-        bs1 = bdrv_new("");
958e1b
+        bs1 = bdrv_new("", &error_abort);
958e1b
         ret = bdrv_open(bs1, filename, NULL, 0, drv, &local_err);
958e1b
         if (ret < 0) {
958e1b
             bdrv_unref(bs1);
958e1b
@@ -5273,7 +5273,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
958e1b
             back_flags =
958e1b
                 flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
958e1b
 
958e1b
-            bs = bdrv_new("");
958e1b
+            bs = bdrv_new("", &error_abort);
958e1b
 
958e1b
             ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
958e1b
                             backing_drv, &local_err);
958e1b
diff --git a/block/blkverify.c b/block/blkverify.c
958e1b
index 4ff7688..e623f6e 100644
958e1b
--- a/block/blkverify.c
958e1b
+++ b/block/blkverify.c
958e1b
@@ -155,7 +155,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
958e1b
         goto fail;
958e1b
     }
958e1b
 
958e1b
-    s->test_file = bdrv_new("");
958e1b
+    s->test_file = bdrv_new("", &error_abort);
958e1b
     ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, &local_err);
958e1b
     if (ret < 0) {
958e1b
         error_propagate(errp, local_err);
958e1b
diff --git a/block/iscsi.c b/block/iscsi.c
958e1b
index 9fe3be8..4f42f29 100644
958e1b
--- a/block/iscsi.c
958e1b
+++ b/block/iscsi.c
958e1b
@@ -1576,7 +1576,7 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options,
958e1b
     IscsiLun *iscsilun = NULL;
958e1b
     QDict *bs_options;
958e1b
 
958e1b
-    bs = bdrv_new("");
958e1b
+    bs = bdrv_new("", &error_abort);
958e1b
 
958e1b
     /* Read out options */
958e1b
     while (options && options->name) {
958e1b
diff --git a/block/vmdk.c b/block/vmdk.c
958e1b
index eff0663..a5b1f1c 100644
958e1b
--- a/block/vmdk.c
958e1b
+++ b/block/vmdk.c
958e1b
@@ -1745,7 +1745,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
958e1b
         goto exit;
958e1b
     }
958e1b
     if (backing_file) {
958e1b
-        BlockDriverState *bs = bdrv_new("");
958e1b
+        BlockDriverState *bs = bdrv_new("", &error_abort);
958e1b
         ret = bdrv_open(bs, backing_file, NULL, 0, NULL, errp);
958e1b
         if (ret != 0) {
958e1b
             bdrv_unref(bs);
958e1b
diff --git a/block/vvfat.c b/block/vvfat.c
958e1b
index 3ddaa0b..6fd52df 100644
958e1b
--- a/block/vvfat.c
958e1b
+++ b/block/vvfat.c
958e1b
@@ -2935,7 +2935,7 @@ static int enable_write_target(BDRVVVFATState *s)
958e1b
         goto err;
958e1b
     }
958e1b
 
958e1b
-    s->qcow = bdrv_new("");
958e1b
+    s->qcow = bdrv_new("", &error_abort);
958e1b
 
958e1b
     ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
958e1b
             BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
958e1b
@@ -2951,7 +2951,7 @@ static int enable_write_target(BDRVVVFATState *s)
958e1b
     unlink(s->qcow_filename);
958e1b
 #endif
958e1b
 
958e1b
-    s->bs->backing_hd = bdrv_new("");
958e1b
+    s->bs->backing_hd = bdrv_new("", &error_abort);
958e1b
     s->bs->backing_hd->drv = &vvfat_write_target;
958e1b
     s->bs->backing_hd->opaque = g_malloc(sizeof(void*));
958e1b
     *(void**)s->bs->backing_hd->opaque = s;
958e1b
diff --git a/blockdev.c b/blockdev.c
958e1b
index 1ac8804..b5792a2 100644
958e1b
--- a/blockdev.c
958e1b
+++ b/blockdev.c
958e1b
@@ -471,7 +471,11 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
958e1b
     /* init */
958e1b
     dinfo = g_malloc0(sizeof(*dinfo));
958e1b
     dinfo->id = g_strdup(qemu_opts_id(opts));
958e1b
-    dinfo->bdrv = bdrv_new(dinfo->id);
958e1b
+    dinfo->bdrv = bdrv_new(dinfo->id, &error);
958e1b
+    if (error) {
958e1b
+        error_propagate(errp, error);
958e1b
+        goto bdrv_new_err;
958e1b
+    }
958e1b
     dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
958e1b
     dinfo->bdrv->read_only = ro;
958e1b
     dinfo->type = type;
958e1b
@@ -531,8 +535,9 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
958e1b
 
958e1b
 err:
958e1b
     bdrv_unref(dinfo->bdrv);
958e1b
-    g_free(dinfo->id);
958e1b
     QTAILQ_REMOVE(&drives, dinfo, next);
958e1b
+bdrv_new_err:
958e1b
+    g_free(dinfo->id);
958e1b
     g_free(dinfo);
958e1b
 early_err:
958e1b
     QDECREF(bs_opts);
958e1b
@@ -1056,7 +1061,7 @@ static void external_snapshot_prepare(BlkTransactionStates *common,
958e1b
     }
958e1b
 
958e1b
     /* We will manually add the backing_hd field to the bs later */
958e1b
-    states->new_bs = bdrv_new("");
958e1b
+    states->new_bs = bdrv_new("", &error_abort);
958e1b
     /* TODO Inherit bs->options or only take explicit options with an
958e1b
      * extended QMP command? */
958e1b
     ret = bdrv_open(states->new_bs, new_image_file, NULL,
958e1b
@@ -1678,7 +1683,7 @@ void qmp_drive_mirror(const char *device, const char *target,
958e1b
     /* Mirroring takes care of copy-on-write using the source's backing
958e1b
      * file.
958e1b
      */
958e1b
-    target_bs = bdrv_new("");
958e1b
+    target_bs = bdrv_new("", &error_abort);
958e1b
     ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
958e1b
                     &local_err);
958e1b
     if (ret < 0) {
958e1b
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
958e1b
index a2653b2..fc45f36 100644
958e1b
--- a/hw/block/xen_disk.c
958e1b
+++ b/hw/block/xen_disk.c
958e1b
@@ -793,11 +793,14 @@ static int blk_connect(struct XenDevice *xendev)
958e1b
     index = (blkdev->xendev.dev - 202 * 256) / 16;
958e1b
     blkdev->dinfo = drive_get(IF_XEN, 0, index);
958e1b
     if (!blkdev->dinfo) {
958e1b
+        Error *local_err = NULL;
958e1b
         /* setup via xenbus -> create new block driver instance */
958e1b
         xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
958e1b
-        blkdev->bs = bdrv_new(blkdev->dev);
958e1b
+        blkdev->bs = bdrv_new(blkdev->dev, &local_err);
958e1b
+        if (local_err) {
958e1b
+            blkdev->bs = NULL;
958e1b
+        }
958e1b
         if (blkdev->bs) {
958e1b
-            Error *local_err = NULL;
958e1b
             BlockDriver *drv = bdrv_find_whitelisted_format(blkdev->fileproto,
958e1b
                                                            readonly);
958e1b
             if (bdrv_open(blkdev->bs,
958e1b
diff --git a/include/block/block.h b/include/block/block.h
958e1b
index 03b7960..3651bd9 100644
958e1b
--- a/include/block/block.h
958e1b
+++ b/include/block/block.h
958e1b
@@ -178,7 +178,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
958e1b
     QEMUOptionParameter *options, Error **errp);
958e1b
 int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
958e1b
                      Error **errp);
958e1b
-BlockDriverState *bdrv_new(const char *device_name);
958e1b
+BlockDriverState *bdrv_new(const char *device_name, Error **errp);
958e1b
 void bdrv_make_anon(BlockDriverState *bs);
958e1b
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
958e1b
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
958e1b
diff --git a/qemu-img.c b/qemu-img.c
958e1b
index ed1799c..5c2f36a 100644
958e1b
--- a/qemu-img.c
958e1b
+++ b/qemu-img.c
958e1b
@@ -274,7 +274,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
958e1b
     Error *local_err = NULL;
958e1b
     int ret;
958e1b
 
958e1b
-    bs = bdrv_new("image");
958e1b
+    bs = bdrv_new("image", &error_abort);
958e1b
 
958e1b
     if (fmt) {
958e1b
         drv = bdrv_find_format(fmt);
958e1b
@@ -2299,7 +2299,7 @@ static int img_rebase(int argc, char **argv)
958e1b
     } else {
958e1b
         char backing_name[1024];
958e1b
 
958e1b
-        bs_old_backing = bdrv_new("old_backing");
958e1b
+        bs_old_backing = bdrv_new("old_backing", &error_abort);
958e1b
         bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
958e1b
         ret = bdrv_open(bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
958e1b
                         old_backing_drv, &local_err);
958e1b
@@ -2310,7 +2310,7 @@ static int img_rebase(int argc, char **argv)
958e1b
             goto out;
958e1b
         }
958e1b
         if (out_baseimg[0]) {
958e1b
-            bs_new_backing = bdrv_new("new_backing");
958e1b
+            bs_new_backing = bdrv_new("new_backing", &error_abort);
958e1b
             ret = bdrv_open(bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
958e1b
                         new_backing_drv, &local_err);
958e1b
             if (ret) {
958e1b
diff --git a/qemu-io.c b/qemu-io.c
958e1b
index bbe2518..cc89947 100644
958e1b
--- a/qemu-io.c
958e1b
+++ b/qemu-io.c
958e1b
@@ -1831,7 +1831,7 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
958e1b
             return 1;
958e1b
         }
958e1b
     } else {
958e1b
-        bs = bdrv_new("hda");
958e1b
+        bs = bdrv_new("hda", &error_abort);
958e1b
 
958e1b
         if (bdrv_open(bs, name, opts, flags, NULL, &local_err) < 0) {
958e1b
             fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
958e1b
diff --git a/qemu-nbd.c b/qemu-nbd.c
958e1b
index 207a610..ff792ef 100644
958e1b
--- a/qemu-nbd.c
958e1b
+++ b/qemu-nbd.c
958e1b
@@ -572,7 +572,8 @@ int main(int argc, char **argv)
958e1b
         drv = NULL;
958e1b
     }
958e1b
 
958e1b
-    bs = bdrv_new("hda");
958e1b
+    bs = bdrv_new("hda", &error_abort);
958e1b
+
958e1b
     srcpath = argv[optind];
958e1b
     ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err);
958e1b
     if (ret < 0) {
958e1b
-- 
958e1b
1.8.3.1
958e1b