[Qemu-devel] [RFC v2 4/6] block: Error parameter for create functions

[Max Reitz]

Add an Error ** parameter to bdrv_create and its associated functions to
allow more specific error messages.

Signed-off-by: Max Reitz <address@hidden>
---
 block.c               | 72 +++++++++++++++++++++++++++++++++------------------
 block/cow.c           |  2 +-
 block/qcow.c          |  2 +-
 block/qcow2.c         |  2 +-
 block/qed.c           |  2 +-
 block/raw_bsd.c       |  2 +-
 block/vvfat.c         |  2 +-
 include/block/block.h |  5 ++--
 qemu-img.c            | 16 ++++--------
 9 files changed, 61 insertions(+), 44 deletions(-)

diff --git a/block.c b/block.c
index 8da32b2..d734265 100644
--- a/block.c
+++ b/block.c
@@ -367,18 +367,26 @@ typedef struct CreateCo {
     char *filename;
     QEMUOptionParameter *options;
     int ret;
+    Error *err;
 } CreateCo;
 
 static void coroutine_fn bdrv_create_co_entry(void *opaque)
 {
+    Error *local_err = NULL;
+    int ret;
+
     CreateCo *cco = opaque;
     assert(cco->drv);
 
-    cco->ret = cco->drv->bdrv_create(cco->filename, cco->options, NULL);
+    ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(&cco->err, local_err);
+    }
+    cco->ret = ret;
 }
 
 int bdrv_create(BlockDriver *drv, const char* filename,
-    QEMUOptionParameter *options)
+    QEMUOptionParameter *options, Error **errp)
 {
     int ret;
 
@@ -388,9 +396,11 @@ int bdrv_create(BlockDriver *drv, const char* filename,
         .filename = g_strdup(filename),
         .options = options,
         .ret = NOT_DONE,
+        .err = NULL,
     };
 
     if (!drv->bdrv_create) {
+        error_setg(errp, "Driver '%s' does not support image creation", 
drv->format_name);
         ret = -ENOTSUP;
         goto out;
     }
@@ -407,22 +417,37 @@ int bdrv_create(BlockDriver *drv, const char* filename,
     }
 
     ret = cco.ret;
+    if (ret < 0) {
+        if (error_is_set(&cco.err)) {
+            error_propagate(errp, cco.err);
+        } else {
+            error_setg_errno(errp, -ret, "Could not create image");
+        }
+    }
 
 out:
     g_free(cco.filename);
     return ret;
 }
 
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
+int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
+                     Error **errp)
 {
     BlockDriver *drv;
+    Error *local_err = NULL;
+    int ret;
 
     drv = bdrv_find_protocol(filename, true);
     if (drv == NULL) {
+        error_setg(errp, "Could not find protocol for file '%s'", filename);
         return -ENOENT;
     }
 
-    return bdrv_create(drv, filename, options);
+    ret = bdrv_create(drv, filename, options, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
+    return ret;
 }
 
 /*
@@ -1053,11 +1078,9 @@ int bdrv_open(BlockDriverState *bs, const char 
*filename, QDict *options,
                 drv->format_name);
         }
 
-        ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
+        ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, 
&local_err);
         free_option_parameters(create_options);
         if (ret < 0) {
-            error_setg_errno(errp, -ret, "Could not create temporary overlay "
-                             "'%s'", tmp_filename);
             goto fail;
         }
 
@@ -4553,6 +4576,7 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
     BlockDriverState *bs = NULL;
     BlockDriver *drv, *proto_drv;
     BlockDriver *backing_drv = NULL;
+    Error *local_err = NULL;
     int ret = 0;
 
     /* Find driver and parse its options */
@@ -4639,10 +4663,8 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
             bs = bdrv_new("");
 
             ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
-                            backing_drv, NULL);
+                            backing_drv, &local_err);
             if (ret < 0) {
-                error_setg_errno(errp, -ret, "Could not open '%s'",
-                                 backing_file->value.s);
                 goto out;
             }
             bdrv_get_geometry(bs, &size);
@@ -4661,22 +4683,19 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
         print_option_parameters(param);
         puts("");
     }
-    ret = bdrv_create(drv, filename, param);
-    if (ret < 0) {
-        if (ret == -ENOTSUP) {
-            error_setg(errp,"Formatting or formatting option not supported for 
"
-                            "file format '%s'", fmt);
-        } else if (ret == -EFBIG) {
-            const char *cluster_size_hint = "";
-            if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
-                cluster_size_hint = " (try using a larger cluster size)";
-            }
-            error_setg(errp, "The image size is too large for file format 
'%s'%s",
-                       fmt, cluster_size_hint);
-        } else {
-            error_setg(errp, "%s: error while creating %s: %s", filename, fmt,
-                       strerror(-ret));
+    ret = bdrv_create(drv, filename, param, &local_err);
+    if (ret == -EFBIG) {
+        /* This is generally a better message than whatever the driver would
+         * deliver (especially because of the cluster_size_hint), since that
+         * is most probably not much different from "image too large". */
+        const char *cluster_size_hint = "";
+        if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
+            cluster_size_hint = " (try using a larger cluster size)";
         }
+        error_setg(errp, "The image size is too large for file format '%s'"
+                   "%s", fmt, cluster_size_hint);
+        error_free(local_err);
+        local_err = NULL;
     }
 
 out:
@@ -4686,6 +4705,9 @@ out:
     if (bs) {
         bdrv_delete(bs);
     }
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
 }
 
 AioContext *bdrv_get_aio_context(BlockDriverState *bs)
diff --git a/block/cow.c b/block/cow.c
index f890db0..1c12996 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -276,7 +276,7 @@ static int cow_create(const char *filename, 
QEMUOptionParameter *options,
         options++;
     }
 
-    ret = bdrv_create_file(filename, options);
+    ret = bdrv_create_file(filename, options, NULL);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/qcow.c b/block/qcow.c
index ab3f1ab..50c6657 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -676,7 +676,7 @@ static int qcow_create(const char *filename, 
QEMUOptionParameter *options,
         options++;
     }
 
-    ret = bdrv_create_file(filename, options);
+    ret = bdrv_create_file(filename, options, NULL);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index 8692931..e16d352 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1354,7 +1354,7 @@ static int qcow2_create2(const char *filename, int64_t 
total_size,
     uint8_t* refcount_table;
     int ret;
 
-    ret = bdrv_create_file(filename, options);
+    ret = bdrv_create_file(filename, options, NULL);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/qed.c b/block/qed.c
index ba1f9d8..3552daf 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -554,7 +554,7 @@ static int qed_create(const char *filename, uint32_t 
cluster_size,
     int ret = 0;
     BlockDriverState *bs = NULL;
 
-    ret = bdrv_create_file(filename, NULL);
+    ret = bdrv_create_file(filename, NULL, NULL);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 793121a..0f1b677 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -133,7 +133,7 @@ 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);
+    return bdrv_create_file(filename, options, NULL);
 }
 
 static int raw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/block/vvfat.c b/block/vvfat.c
index 76a30a5..65e5bd0 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2926,7 +2926,7 @@ 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);
+    ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, NULL);
     if (ret < 0) {
         goto err;
     }
diff --git a/include/block/block.h b/include/block/block.h
index efafd7a..15e226d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -117,8 +117,9 @@ BlockDriver *bdrv_find_format(const char *format_name);
 BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
                                           bool readonly);
 int bdrv_create(BlockDriver *drv, const char* filename,
-    QEMUOptionParameter *options);
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options);
+    QEMUOptionParameter *options, Error **errp);
+int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
+                     Error **errp);
 BlockDriverState *bdrv_new(const char *device_name);
 void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
diff --git a/qemu-img.c b/qemu-img.c
index 607981e..a0c400b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1136,6 +1136,7 @@ static int img_convert(int argc, char **argv)
     float local_progress = 0;
     int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
     bool quiet = false;
+    Error *local_err = NULL;
 
     fmt = NULL;
     out_fmt = "raw";
@@ -1333,18 +1334,11 @@ static int img_convert(int argc, char **argv)
     }
 
     /* Create the new image */
-    ret = bdrv_create(drv, out_filename, param);
+    ret = bdrv_create(drv, out_filename, param, &local_err);
     if (ret < 0) {
-        if (ret == -ENOTSUP) {
-            error_report("Formatting not supported for file format '%s'",
-                         out_fmt);
-        } else if (ret == -EFBIG) {
-            error_report("The image size is too large for file format '%s'",
-                         out_fmt);
-        } else {
-            error_report("%s: error while converting %s: %s",
-                         out_filename, out_fmt, strerror(-ret));
-        }
+        error_report("%s: error while converting %s: %s",
+                     out_filename, out_fmt, error_get_pretty(local_err));
+        error_free(local_err);
         goto out;
     }
 
-- 
1.8.3.1