[Qemu-devel] [RFC v2 6/6] bdrv: No silent error message discarding

[Max Reitz]

Using NULL as errp parameters to bdrv_open, bdrv_file_open and
bdrv_create in all block drivers which do not yet implement proper error
propagation is not a good idea, since this now silently discards error
messages. Fix this by using a proper parameter and printing its content
on error (until proper propagation is implemented).

Signed-off-by: Max Reitz <address@hidden>
---
Note that many error propagation are actually trivial (such as in
raw_bsd), but I intentionally refrained from implementing the propagation
and tenaciously followed the pattern:
  Error *local_err = NULL;
  foo(&local_err);
  if (/*error condition*/) {
      qerror_report_err(local_err):
      error_free(local_err);
  }
This is because the propagation may sometimes be trivial, however, it is
often still driver-specific, therefore this deserves its own patch for
every driver, in my opinion. Also, I think it is easier to review this
way. ;-)
---
 block/blkdebug.c  |  4 +++-
 block/blkverify.c |  8 ++++++--
 block/cow.c       |  9 +++++++--
 block/qcow.c      |  9 +++++++--
 block/qed.c       |  9 +++++++--
 block/raw_bsd.c   | 10 +++++++++-
 block/sheepdog.c  |  8 ++++++--
 block/vmdk.c      | 10 ++++++++--
 block/vvfat.c     | 10 ++++++++--
 9 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index e0ba16d..be948b2 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -387,8 +387,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
*options, int flags,
         goto fail;
     }
 
-    ret = bdrv_file_open(&bs->file, filename, NULL, flags, NULL);
+    ret = bdrv_file_open(&bs->file, filename, NULL, flags, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         goto fail;
     }
 
diff --git a/block/blkverify.c b/block/blkverify.c
index d428efd..cceb88f 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -141,8 +141,10 @@ static int blkverify_open(BlockDriverState *bs, QDict 
*options, int flags,
         goto fail;
     }
 
-    ret = bdrv_file_open(&bs->file, raw, NULL, flags, NULL);
+    ret = bdrv_file_open(&bs->file, raw, NULL, flags, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         goto fail;
     }
 
@@ -154,8 +156,10 @@ static int blkverify_open(BlockDriverState *bs, QDict 
*options, int flags,
     }
 
     s->test_file = bdrv_new("");
-    ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, NULL);
+    ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         bdrv_delete(s->test_file);
         s->test_file = NULL;
         goto fail;
diff --git a/block/cow.c b/block/cow.c
index 1c12996..0b93b45 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -263,6 +263,7 @@ static int cow_create(const char *filename, 
QEMUOptionParameter *options,
     struct stat st;
     int64_t image_sectors = 0;
     const char *image_filename = NULL;
+    Error *local_err = NULL;
     int ret;
     BlockDriverState *cow_bs;
 
@@ -276,13 +277,17 @@ static int cow_create(const char *filename, 
QEMUOptionParameter *options,
         options++;
     }
 
-    ret = bdrv_create_file(filename, options, NULL);
+    ret = bdrv_create_file(filename, options, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
-    ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, NULL);
+    ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/qcow.c b/block/qcow.c
index 50c6657..9b4020f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -661,6 +661,7 @@ static int qcow_create(const char *filename, 
QEMUOptionParameter *options,
     int64_t total_size = 0;
     const char *backing_file = NULL;
     int flags = 0;
+    Error *local_err = NULL;
     int ret;
     BlockDriverState *qcow_bs;
 
@@ -676,13 +677,17 @@ static int qcow_create(const char *filename, 
QEMUOptionParameter *options,
         options++;
     }
 
-    ret = bdrv_create_file(filename, options, NULL);
+    ret = bdrv_create_file(filename, options, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
-    ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, NULL);
+    ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/qed.c b/block/qed.c
index 3552daf..168a68a 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -551,17 +551,22 @@ static int qed_create(const char *filename, uint32_t 
cluster_size,
     QEDHeader le_header;
     uint8_t *l1_table = NULL;
     size_t l1_size = header.cluster_size * header.table_size;
+    Error *local_err = NULL;
     int ret = 0;
     BlockDriverState *bs = NULL;
 
-    ret = bdrv_create_file(filename, NULL, NULL);
+    ret = bdrv_create_file(filename, NULL, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
     ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB,
-                         NULL);
+                         &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 0f1b677..2effe72 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -133,7 +133,15 @@ static int raw_has_zero_init(BlockDriverState *bs)
 static int raw_create(const char *filename, QEMUOptionParameter *options,
                       Error **errp)
 {
-    return bdrv_create_file(filename, options, NULL);
+    Error *local_err = NULL;
+    int ret;
+
+    ret = bdrv_create_file(filename, options, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return ret;
 }
 
 static int raw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 6722771..a9d8019 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1401,9 +1401,10 @@ static int sd_prealloc(const char *filename)
     uint32_t idx, max_idx;
     int64_t vdi_size;
     void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
+    Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
+    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err);
     if (ret < 0) {
         goto out;
     }
@@ -1449,6 +1450,7 @@ static int sd_create(const char *filename, 
QEMUOptionParameter *options,
     char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN];
     uint32_t snapid;
     bool prealloc = false;
+    Error *local_err = NULL;
 
     s = g_malloc0(sizeof(BDRVSheepdogState));
 
@@ -1502,8 +1504,10 @@ static int sd_create(const char *filename, 
QEMUOptionParameter *options,
             goto out;
         }
 
-        ret = bdrv_file_open(&bs, backing_file, NULL, 0, NULL);
+        ret = bdrv_file_open(&bs, backing_file, NULL, 0, &local_err);
         if (ret < 0) {
+            qerror_report_err(local_err);
+            error_free(local_err);
             goto out;
         }
 
diff --git a/block/vmdk.c b/block/vmdk.c
index 1e1b860..e62be63 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -697,6 +697,7 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
     int64_t flat_offset;
     char extent_path[PATH_MAX];
     BlockDriverState *extent_file;
+    Error *local_err = NULL;
 
     while (*p) {
         /* parse extent line:
@@ -727,8 +728,10 @@ 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, bs->open_flags,
-                             NULL);
+                             &local_err);
         if (ret) {
+            qerror_report_err(local_err);
+            error_free(local_err);
             return ret;
         }
 
@@ -1575,6 +1578,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options,
         "ddb.geometry.heads = \"%d\"\n"
         "ddb.geometry.sectors = \"63\"\n"
         "ddb.adapterType = \"%s\"\n";
+    Error *local_err = NULL;
 
     if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
         return -EINVAL;
@@ -1637,8 +1641,10 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options,
     }
     if (backing_file) {
         BlockDriverState *bs = bdrv_new("");
-        ret = bdrv_open(bs, backing_file, NULL, 0, NULL, NULL);
+        ret = bdrv_open(bs, backing_file, NULL, 0, NULL, &local_err);
         if (ret != 0) {
+            qerror_report_err(local_err);
+            error_free(local_err);
             bdrv_delete(bs);
             return ret;
         }
diff --git a/block/vvfat.c b/block/vvfat.c
index 65e5bd0..fcb97f4 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2909,6 +2909,7 @@ static int enable_write_target(BDRVVVFATState *s)
 {
     BlockDriver *bdrv_qcow;
     QEMUOptionParameter *options;
+    Error *local_err = NULL;
     int ret;
     int size = sector2cluster(s, s->sector_count);
     s->used_clusters = calloc(size, 1);
@@ -2926,16 +2927,21 @@ static int enable_write_target(BDRVVVFATState *s)
     set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512);
     set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:");
 
-    ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, NULL);
+    ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         goto err;
     }
 
     s->qcow = bdrv_new("");
 
     ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
-            BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow, 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_delete(s->qcow);
         goto err;
     }
-- 
1.8.3.1