diff mbox

[RFC,v2,3/6] block: Error parameter for open functions

Message ID 1378389342-4749-4-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Sept. 5, 2013, 1:55 p.m. UTC
Add an Error ** parameter to bdrv_open, bdrv_file_open and associated
functions to allow more specific error messages.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 98 ++++++++++++++++++++++++++++++++-------------------
 block/blkdebug.c      |  2 +-
 block/blkverify.c     |  4 +--
 block/cow.c           |  2 +-
 block/mirror.c        |  5 +--
 block/qcow.c          |  2 +-
 block/qcow2.c         |  4 +--
 block/qed.c           |  3 +-
 block/sheepdog.c      |  4 +--
 block/vmdk.c          |  5 +--
 block/vvfat.c         |  2 +-
 blockdev.c            | 30 +++++++---------
 include/block/block.h |  6 ++--
 qemu-img.c            | 23 +++++++-----
 qemu-io.c             | 14 +++++---
 qemu-nbd.c            |  6 ++--
 16 files changed, 125 insertions(+), 85 deletions(-)

Comments

Kevin Wolf Sept. 6, 2013, 1:35 p.m. UTC | #1
Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> Add an Error ** parameter to bdrv_open, bdrv_file_open and associated
> functions to allow more specific error messages.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c               | 98 ++++++++++++++++++++++++++++++++-------------------
>  block/blkdebug.c      |  2 +-
>  block/blkverify.c     |  4 +--
>  block/cow.c           |  2 +-
>  block/mirror.c        |  5 +--
>  block/qcow.c          |  2 +-
>  block/qcow2.c         |  4 +--
>  block/qed.c           |  3 +-
>  block/sheepdog.c      |  4 +--
>  block/vmdk.c          |  5 +--
>  block/vvfat.c         |  2 +-
>  blockdev.c            | 30 +++++++---------
>  include/block/block.h |  6 ++--
>  qemu-img.c            | 23 +++++++-----
>  qemu-io.c             | 14 +++++---
>  qemu-nbd.c            |  6 ++--
>  16 files changed, 125 insertions(+), 85 deletions(-)
> 
> diff --git a/block.c b/block.c
> index f485906..8da32b2 100644
> --- a/block.c
> +++ b/block.c
> @@ -525,7 +525,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
>  }
>  
>  static int find_image_format(BlockDriverState *bs, const char *filename,
> -                             BlockDriver **pdrv)
> +                             BlockDriver **pdrv, Error **errp)
>  {
>      int score, score_max;
>      BlockDriver *drv1, *drv;
> @@ -536,6 +536,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
>      if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
>          drv = bdrv_find_format("raw");
>          if (!drv) {
> +            error_setg(errp, "Could not find raw image format");
>              ret = -ENOENT;
>          }
>          *pdrv = drv;
> @@ -544,6 +545,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
>  
>      ret = bdrv_pread(bs, 0, buf, sizeof(buf));
>      if (ret < 0) {
> +        error_setg_errno(errp, -ret, "Could not read image for determining its "
> +                         "format");
>          *pdrv = NULL;
>          return ret;
>      }
> @@ -560,6 +563,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
>          }
>      }
>      if (!drv) {
> +        error_setg(errp, "Could not determine image format: No compatible "
> +                   "driver found");
>          ret = -ENOENT;
>      }
>      *pdrv = drv;
> @@ -679,10 +684,11 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
>   * Removes all processed options from *options.
>   */
>  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
> -    QDict *options, int flags, BlockDriver *drv)
> +    QDict *options, int flags, BlockDriver *drv, Error **errp)
>  {
>      int ret, open_flags;
>      const char *filename;
> +    Error *local_err = NULL;
>  
>      assert(drv != NULL);
>      assert(bs->file == NULL);
> @@ -711,6 +717,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>      bs->read_only = !(open_flags & BDRV_O_RDWR);
>  
>      if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
> +        error_setg(errp, "Driver '%s' is not whitelisted", drv->format_name);
>          return -ENOTSUP;
>      }
>  
> @@ -734,25 +741,30 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>      if (drv->bdrv_file_open) {
>          assert(file == NULL);
>          assert(drv->bdrv_parse_filename || filename != NULL);
> -        ret = drv->bdrv_file_open(bs, options, open_flags, NULL);
> +        ret = drv->bdrv_file_open(bs, options, open_flags, &local_err);
>      } else {
>          if (file == NULL) {
> -            qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
> -                          "block driver for the protocol level",
> -                          drv->format_name);
> +            error_setg(errp, "Can't use '%s' as a block driver for the "
> +                       "protocol level", drv->format_name);
>              ret = -EINVAL;
>              goto free_and_fail;
>          }
>          bs->file = file;
> -        ret = drv->bdrv_open(bs, options, open_flags, NULL);
> +        ret = drv->bdrv_open(bs, options, open_flags, &local_err);
>      }
>  
>      if (ret < 0) {
> +        if (error_is_set(&local_err)) {
> +            error_propagate(errp, local_err);
> +        } else {
> +            error_setg_errno(errp, -ret, "Could not open '%s'", filename);

filename can be NULL. This happens in cases like:

-drive file.driver=nbd,file.host=localhost

> +        }
>          goto free_and_fail;
>      }
>  
>      ret = refresh_total_sectors(bs, bs->total_sectors);
>      if (ret < 0) {
> +        error_setg_errno(errp, -ret, "Could not refresh total sector count");
>          goto free_and_fail;
>      }
>  

> diff --git a/qemu-img.c b/qemu-img.c
> index b9a848d..607981e 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -264,6 +264,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
>      BlockDriverState *bs;
>      BlockDriver *drv;
>      char password[256];
> +    Error *local_err = NULL;
>      int ret;
>  
>      bs = bdrv_new("image");
> @@ -278,9 +279,11 @@ static BlockDriverState *bdrv_new_open(const char *filename,
>          drv = NULL;
>      }
>  
> -    ret = bdrv_open(bs, filename, NULL, flags, drv);
> +    ret = bdrv_open(bs, filename, NULL, flags, drv, &local_err);
>      if (ret < 0) {
> -        error_report("Could not open '%s': %s", filename, strerror(-ret));
> +        error_report("Could not open '%s': %s", filename,
> +                     error_get_pretty(local_err));
> +        error_free(local_err);
>          goto fail;
>      }
>  
> @@ -407,7 +410,7 @@ static int img_create(int argc, char **argv)
>      bdrv_img_create(filename, fmt, base_filename, base_fmt,
>                      options, img_size, BDRV_O_FLAGS, &local_err, quiet);
>      if (error_is_set(&local_err)) {
> -        error_report("%s", error_get_pretty(local_err));
> +        error_report("%s: %s", filename, error_get_pretty(local_err));

This is an independent change. :-)

It's probably also better handled with the loc_*() functions that define
an area where some given information (like the filename) is prepended to
all error messages printed by error_report().

>          error_free(local_err);
>          return 1;
>      }

Kevin
diff mbox

Patch

diff --git a/block.c b/block.c
index f485906..8da32b2 100644
--- a/block.c
+++ b/block.c
@@ -525,7 +525,7 @@  BlockDriver *bdrv_find_protocol(const char *filename,
 }
 
 static int find_image_format(BlockDriverState *bs, const char *filename,
-                             BlockDriver **pdrv)
+                             BlockDriver **pdrv, Error **errp)
 {
     int score, score_max;
     BlockDriver *drv1, *drv;
@@ -536,6 +536,7 @@  static int find_image_format(BlockDriverState *bs, const char *filename,
     if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
         drv = bdrv_find_format("raw");
         if (!drv) {
+            error_setg(errp, "Could not find raw image format");
             ret = -ENOENT;
         }
         *pdrv = drv;
@@ -544,6 +545,8 @@  static int find_image_format(BlockDriverState *bs, const char *filename,
 
     ret = bdrv_pread(bs, 0, buf, sizeof(buf));
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not read image for determining its "
+                         "format");
         *pdrv = NULL;
         return ret;
     }
@@ -560,6 +563,8 @@  static int find_image_format(BlockDriverState *bs, const char *filename,
         }
     }
     if (!drv) {
+        error_setg(errp, "Could not determine image format: No compatible "
+                   "driver found");
         ret = -ENOENT;
     }
     *pdrv = drv;
@@ -679,10 +684,11 @@  static int bdrv_open_flags(BlockDriverState *bs, int flags)
  * Removes all processed options from *options.
  */
 static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
-    QDict *options, int flags, BlockDriver *drv)
+    QDict *options, int flags, BlockDriver *drv, Error **errp)
 {
     int ret, open_flags;
     const char *filename;
+    Error *local_err = NULL;
 
     assert(drv != NULL);
     assert(bs->file == NULL);
@@ -711,6 +717,7 @@  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     bs->read_only = !(open_flags & BDRV_O_RDWR);
 
     if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
+        error_setg(errp, "Driver '%s' is not whitelisted", drv->format_name);
         return -ENOTSUP;
     }
 
@@ -734,25 +741,30 @@  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     if (drv->bdrv_file_open) {
         assert(file == NULL);
         assert(drv->bdrv_parse_filename || filename != NULL);
-        ret = drv->bdrv_file_open(bs, options, open_flags, NULL);
+        ret = drv->bdrv_file_open(bs, options, open_flags, &local_err);
     } else {
         if (file == NULL) {
-            qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
-                          "block driver for the protocol level",
-                          drv->format_name);
+            error_setg(errp, "Can't use '%s' as a block driver for the "
+                       "protocol level", drv->format_name);
             ret = -EINVAL;
             goto free_and_fail;
         }
         bs->file = file;
-        ret = drv->bdrv_open(bs, options, open_flags, NULL);
+        ret = drv->bdrv_open(bs, options, open_flags, &local_err);
     }
 
     if (ret < 0) {
+        if (error_is_set(&local_err)) {
+            error_propagate(errp, local_err);
+        } else {
+            error_setg_errno(errp, -ret, "Could not open '%s'", filename);
+        }
         goto free_and_fail;
     }
 
     ret = refresh_total_sectors(bs, bs->total_sectors);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not refresh total sector count");
         goto free_and_fail;
     }
 
@@ -781,12 +793,13 @@  free_and_fail:
  * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
  */
 int bdrv_file_open(BlockDriverState **pbs, const char *filename,
-                   QDict *options, int flags)
+                   QDict *options, int flags, Error **errp)
 {
     BlockDriverState *bs;
     BlockDriver *drv;
     const char *drvname;
     bool allow_protocol_prefix = false;
+    Error *local_err = NULL;
     int ret;
 
     /* NULL means an empty set of options */
@@ -805,8 +818,8 @@  int bdrv_file_open(BlockDriverState **pbs, const char *filename,
         qdict_put(options, "filename", qstring_from_str(filename));
         allow_protocol_prefix = true;
     } else {
-        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't specify 'file' and "
-                      "'filename' options at the same time");
+        error_setg(errp, "Can't specify 'file' and 'filename' options at the "
+                   "same time");
         ret = -EINVAL;
         goto fail;
     }
@@ -815,53 +828,53 @@  int bdrv_file_open(BlockDriverState **pbs, const char *filename,
     drvname = qdict_get_try_str(options, "driver");
     if (drvname) {
         drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR));
+        if (!drv) {
+            error_setg(errp, "Unknown driver '%s'", drvname);
+        }
         qdict_del(options, "driver");
     } else if (filename) {
         drv = bdrv_find_protocol(filename, allow_protocol_prefix);
         if (!drv) {
-            qerror_report(ERROR_CLASS_GENERIC_ERROR, "Unknown protocol");
+            error_setg(errp, "Unknown protocol");
         }
     } else {
-        qerror_report(ERROR_CLASS_GENERIC_ERROR,
-                      "Must specify either driver or file");
+        error_setg(errp, "Must specify either driver or file");
         drv = NULL;
     }
 
     if (!drv) {
+        /* errp has been set already */
         ret = -ENOENT;
         goto fail;
     }
 
     /* Parse the filename and open it */
     if (drv->bdrv_parse_filename && filename) {
-        Error *local_err = NULL;
         drv->bdrv_parse_filename(filename, options, &local_err);
         if (error_is_set(&local_err)) {
-            qerror_report_err(local_err);
-            error_free(local_err);
+            error_propagate(errp, local_err);
             ret = -EINVAL;
             goto fail;
         }
         qdict_del(options, "filename");
     } else if (!drv->bdrv_parse_filename && !filename) {
-        qerror_report(ERROR_CLASS_GENERIC_ERROR,
-                      "The '%s' block driver requires a file name",
-                      drv->format_name);
+        error_setg(errp, "The '%s' block driver requires a file name",
+                   drv->format_name);
         ret = -EINVAL;
         goto fail;
     }
 
-    ret = bdrv_open_common(bs, NULL, options, flags, drv);
+    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 (qdict_size(options) != 0) {
         const QDictEntry *entry = qdict_first(options);
-        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block protocol '%s' doesn't "
-                      "support the option '%s'",
-                      drv->format_name, entry->key);
+        error_setg(errp, "Block protocol '%s' doesn't support the option '%s'",
+                   drv->format_name, entry->key);
         ret = -EINVAL;
         goto fail;
     }
@@ -888,11 +901,12 @@  fail:
  * function (even on failure), so if the caller intends to reuse the dictionary,
  * it needs to use QINCREF() before calling bdrv_file_open.
  */
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
 {
     char backing_filename[PATH_MAX];
     int back_flags, ret;
     BlockDriver *back_drv = NULL;
+    Error *local_err = NULL;
 
     if (bs->backing_hd != NULL) {
         QDECREF(options);
@@ -925,11 +939,12 @@  int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
 
     ret = bdrv_open(bs->backing_hd,
                     *backing_filename ? backing_filename : NULL, options,
-                    back_flags, back_drv);
+                    back_flags, back_drv, &local_err);
     if (ret < 0) {
         bdrv_delete(bs->backing_hd);
         bs->backing_hd = NULL;
         bs->open_flags |= BDRV_O_NO_BACKING;
+        error_propagate(errp, local_err);
         return ret;
     }
     return 0;
@@ -963,7 +978,7 @@  static void extract_subqdict(QDict *src, QDict **dst, const char *start)
  * dictionary, it needs to use QINCREF() before calling bdrv_open.
  */
 int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
-              int flags, BlockDriver *drv)
+              int flags, BlockDriver *drv, Error **errp)
 {
     int ret;
     /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
@@ -971,6 +986,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     BlockDriverState *file = NULL;
     QDict *file_options = NULL;
     const char *drvname;
+    Error *local_err = NULL;
 
     /* NULL means an empty set of options */
     if (options == NULL) {
@@ -989,7 +1005,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         char backing_filename[PATH_MAX];
 
         if (qdict_size(options) != 0) {
-            error_report("Can't use snapshot=on with driver-specific options");
+            error_setg(errp, "Can't use snapshot=on with driver-specific options");
             ret = -EINVAL;
             goto fail;
         }
@@ -1000,7 +1016,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
 
         /* if there is a backing file, use it */
         bs1 = bdrv_new("");
-        ret = bdrv_open(bs1, filename, NULL, 0, drv);
+        ret = bdrv_open(bs1, filename, NULL, 0, drv, &local_err);
         if (ret < 0) {
             bdrv_delete(bs1);
             goto fail;
@@ -1011,6 +1027,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
 
         ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not get temporary filename");
             goto fail;
         }
 
@@ -1019,6 +1036,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
             snprintf(backing_filename, sizeof(backing_filename),
                      "%s", filename);
         } else if (!realpath(filename, backing_filename)) {
+            error_setg_errno(errp, errno, "Could not resolve path '%s'", filename);
             ret = -errno;
             goto fail;
         }
@@ -1038,6 +1056,8 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
         free_option_parameters(create_options);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not create temporary overlay "
+                             "'%s'", tmp_filename);
             goto fail;
         }
 
@@ -1054,7 +1074,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     extract_subqdict(options, &file_options, "file.");
 
     ret = bdrv_file_open(&file, filename, file_options,
-                         bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
+                         bdrv_open_flags(bs, flags | BDRV_O_UNMAP), &local_err);
     if (ret < 0) {
         goto fail;
     }
@@ -1067,7 +1087,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     }
 
     if (!drv) {
-        ret = find_image_format(file, filename, &drv);
+        ret = find_image_format(file, filename, &drv, &local_err);
     }
 
     if (!drv) {
@@ -1075,7 +1095,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     }
 
     /* Open the image */
-    ret = bdrv_open_common(bs, file, options, flags, drv);
+    ret = bdrv_open_common(bs, file, options, flags, drv, &local_err);
     if (ret < 0) {
         goto unlink_and_fail;
     }
@@ -1090,7 +1110,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         QDict *backing_options;
 
         extract_subqdict(options, &backing_options, "backing.");
-        ret = bdrv_open_backing_file(bs, backing_options);
+        ret = bdrv_open_backing_file(bs, backing_options, &local_err);
         if (ret < 0) {
             goto close_and_fail;
         }
@@ -1099,9 +1119,9 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     /* Check if any unknown options were used */
     if (qdict_size(options) != 0) {
         const QDictEntry *entry = qdict_first(options);
-        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by "
-            "device '%s' doesn't support the option '%s'",
-            drv->format_name, bs->device_name, entry->key);
+        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;
@@ -1130,11 +1150,17 @@  fail:
     QDECREF(bs->options);
     QDECREF(options);
     bs->options = NULL;
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
     return ret;
 
 close_and_fail:
     bdrv_close(bs);
     QDECREF(options);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
     return ret;
 }
 
@@ -4613,7 +4639,7 @@  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);
+                            backing_drv, NULL);
             if (ret < 0) {
                 error_setg_errno(errp, -ret, "Could not open '%s'",
                                  backing_file->value.s);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 52d65ff..e0ba16d 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -387,7 +387,7 @@  static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    ret = bdrv_file_open(&bs->file, filename, NULL, flags);
+    ret = bdrv_file_open(&bs->file, filename, NULL, flags, NULL);
     if (ret < 0) {
         goto fail;
     }
diff --git a/block/blkverify.c b/block/blkverify.c
index 5d716bb..d428efd 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -141,7 +141,7 @@  static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    ret = bdrv_file_open(&bs->file, raw, NULL, flags);
+    ret = bdrv_file_open(&bs->file, raw, NULL, flags, NULL);
     if (ret < 0) {
         goto fail;
     }
@@ -154,7 +154,7 @@  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);
+    ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, NULL);
     if (ret < 0) {
         bdrv_delete(s->test_file);
         s->test_file = NULL;
diff --git a/block/cow.c b/block/cow.c
index 207fea1..f890db0 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -281,7 +281,7 @@  static int cow_create(const char *filename, QEMUOptionParameter *options,
         return ret;
     }
 
-    ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR);
+    ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, NULL);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/mirror.c b/block/mirror.c
index 86de458..b85326b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -505,14 +505,15 @@  static void mirror_iostatus_reset(BlockJob *job)
 static void mirror_complete(BlockJob *job, Error **errp)
 {
     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+    Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_open_backing_file(s->target, NULL);
+    ret = bdrv_open_backing_file(s->target, NULL, &local_err);
     if (ret < 0) {
         char backing_filename[PATH_MAX];
         bdrv_get_full_backing_filename(s->target, backing_filename,
                                        sizeof(backing_filename));
-        error_setg_file_open(errp, -ret, backing_filename);
+        error_propagate(errp, local_err);
         return;
     }
     if (!s->synced) {
diff --git a/block/qcow.c b/block/qcow.c
index 6f7e73d..ab3f1ab 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -681,7 +681,7 @@  static int qcow_create(const char *filename, QEMUOptionParameter *options,
         return ret;
     }
 
-    ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR);
+    ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, NULL);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index 98d1551..8692931 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1359,7 +1359,7 @@  static int qcow2_create2(const char *filename, int64_t total_size,
         return ret;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR);
+    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
     if (ret < 0) {
         return ret;
     }
@@ -1412,7 +1412,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,
-        BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv);
+        BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, NULL);
     if (ret < 0) {
         goto out;
     }
diff --git a/block/qed.c b/block/qed.c
index 1a9d01b..ba1f9d8 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -559,7 +559,8 @@  static int qed_create(const char *filename, uint32_t cluster_size,
         return ret;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB);
+    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB,
+                         NULL);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/sheepdog.c b/block/sheepdog.c
index b1a0f7a..6722771 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1403,7 +1403,7 @@  static int sd_prealloc(const char *filename)
     void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
     int ret;
 
-    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR);
+    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
     if (ret < 0) {
         goto out;
     }
@@ -1502,7 +1502,7 @@  static int sd_create(const char *filename, QEMUOptionParameter *options,
             goto out;
         }
 
-        ret = bdrv_file_open(&bs, backing_file, NULL, 0);
+        ret = bdrv_file_open(&bs, backing_file, NULL, 0, NULL);
         if (ret < 0) {
             goto out;
         }
diff --git a/block/vmdk.c b/block/vmdk.c
index eb8bba2..1e1b860 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -726,7 +726,8 @@  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);
+        ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags,
+                             NULL);
         if (ret) {
             return ret;
         }
@@ -1636,7 +1637,7 @@  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);
+        ret = bdrv_open(bs, backing_file, NULL, 0, NULL, NULL);
         if (ret != 0) {
             bdrv_delete(bs);
             return ret;
diff --git a/block/vvfat.c b/block/vvfat.c
index d10159d..76a30a5 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2934,7 +2934,7 @@  static int enable_write_target(BDRVVVFATState *s)
     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);
+            BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow, NULL);
     if (ret < 0) {
         bdrv_delete(s->qcow);
         goto err;
diff --git a/blockdev.c b/blockdev.c
index e70e16e..b1b95fb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -707,17 +707,11 @@  static DriveInfo *blockdev_init(QemuOpts *all_opts,
     }
 
     QINCREF(bs_opts);
-    ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv);
+    ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
 
     if (ret < 0) {
-        if (ret == -EMEDIUMTYPE) {
-            error_report("could not open disk image %s: not in %s format",
-                         file ?: dinfo->id, drv ? drv->format_name :
-                         qdict_get_str(bs_opts, "driver"));
-        } else {
-            error_report("could not open disk image %s: %s",
-                         file ?: dinfo->id, strerror(-ret));
-        }
+        error_report("could not open disk image %s: %s",
+                     file ?: dinfo->id, error_get_pretty(error));
         goto err;
     }
 
@@ -957,9 +951,9 @@  static void external_snapshot_prepare(BlkTransactionState *common,
     /* TODO Inherit bs->options or only take explicit options with an
      * extended QMP command? */
     ret = bdrv_open(state->new_bs, new_image_file, NULL,
-                    flags | BDRV_O_NO_BACKING, drv);
+                    flags | BDRV_O_NO_BACKING, drv, &local_err);
     if (ret != 0) {
-        error_setg_file_open(errp, -ret, new_image_file);
+        error_propagate(errp, local_err);
     }
 }
 
@@ -1189,11 +1183,12 @@  static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
                                     int bdrv_flags, BlockDriver *drv,
                                     const char *password, Error **errp)
 {
+    Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv);
+    ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv, &local_err);
     if (ret < 0) {
-        error_setg_file_open(errp, -ret, filename);
+        error_propagate(errp, local_err);
         return;
     }
 
@@ -1583,10 +1578,10 @@  void qmp_drive_backup(const char *device, const char *target,
     }
 
     target_bs = bdrv_new("");
-    ret = bdrv_open(target_bs, target, NULL, flags, drv);
+    ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err);
     if (ret < 0) {
         bdrv_delete(target_bs);
-        error_setg_file_open(errp, -ret, target);
+        error_propagate(errp, local_err);
         return;
     }
 
@@ -1723,10 +1718,11 @@  void qmp_drive_mirror(const char *device, const char *target,
      * file.
      */
     target_bs = bdrv_new("");
-    ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv);
+    ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
+                    &local_err);
     if (ret < 0) {
         bdrv_delete(target_bs);
-        error_setg_file_open(errp, -ret, target);
+        error_propagate(errp, local_err);
         return;
     }
 
diff --git a/include/block/block.h b/include/block/block.h
index e6b391c..efafd7a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -127,10 +127,10 @@  void bdrv_delete(BlockDriverState *bs);
 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,
-                   QDict *options, int flags);
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
+                   QDict *options, int flags, Error **errp);
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
 int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
-              int flags, BlockDriver *drv);
+              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 b9a848d..607981e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -264,6 +264,7 @@  static BlockDriverState *bdrv_new_open(const char *filename,
     BlockDriverState *bs;
     BlockDriver *drv;
     char password[256];
+    Error *local_err = NULL;
     int ret;
 
     bs = bdrv_new("image");
@@ -278,9 +279,11 @@  static BlockDriverState *bdrv_new_open(const char *filename,
         drv = NULL;
     }
 
-    ret = bdrv_open(bs, filename, NULL, flags, drv);
+    ret = bdrv_open(bs, filename, NULL, flags, drv, &local_err);
     if (ret < 0) {
-        error_report("Could not open '%s': %s", filename, strerror(-ret));
+        error_report("Could not open '%s': %s", filename,
+                     error_get_pretty(local_err));
+        error_free(local_err);
         goto fail;
     }
 
@@ -407,7 +410,7 @@  static int img_create(int argc, char **argv)
     bdrv_img_create(filename, fmt, base_filename, base_fmt,
                     options, img_size, BDRV_O_FLAGS, &local_err, quiet);
     if (error_is_set(&local_err)) {
-        error_report("%s", error_get_pretty(local_err));
+        error_report("%s: %s", filename, error_get_pretty(local_err));
         error_free(local_err);
         return 1;
     }
@@ -1912,6 +1915,7 @@  static int img_rebase(int argc, char **argv)
     int unsafe = 0;
     int progress = 0;
     bool quiet = false;
+    Error *local_err = NULL;
 
     /* Parse commandline parameters */
     fmt = NULL;
@@ -2015,18 +2019,21 @@  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,
-                        old_backing_drv);
+                        old_backing_drv, &local_err);
         if (ret) {
-            error_report("Could not open old backing file '%s'", backing_name);
+            error_report("Could not open old backing file '%s': %s",
+                         backing_name, error_get_pretty(local_err));
+            error_free(local_err);
             goto out;
         }
         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);
+                        new_backing_drv, &local_err);
             if (ret) {
-                error_report("Could not open new backing file '%s'",
-                             out_baseimg);
+                error_report("Could not open new backing file '%s': %s",
+                             out_baseimg, error_get_pretty(local_err));
+                error_free(local_err);
                 goto out;
             }
         }
diff --git a/qemu-io.c b/qemu-io.c
index d54dc86..88f74b2 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -46,21 +46,27 @@  static const cmdinfo_t close_cmd = {
 
 static int openfile(char *name, int flags, int growable)
 {
+    Error *local_err = NULL;
+
     if (qemuio_bs) {
         fprintf(stderr, "file open already, try 'help close'\n");
         return 1;
     }
 
     if (growable) {
-        if (bdrv_file_open(&qemuio_bs, name, NULL, flags)) {
-            fprintf(stderr, "%s: can't open device %s\n", progname, name);
+        if (bdrv_file_open(&qemuio_bs, name, NULL, flags, &local_err)) {
+            fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
+                    error_get_pretty(local_err));
+            error_free(local_err);
             return 1;
         }
     } else {
         qemuio_bs = bdrv_new("hda");
 
-        if (bdrv_open(qemuio_bs, name, NULL, flags, NULL) < 0) {
-            fprintf(stderr, "%s: can't open device %s\n", progname, name);
+        if (bdrv_open(qemuio_bs, name, NULL, 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);
             bdrv_delete(qemuio_bs);
             qemuio_bs = NULL;
             return 1;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index f044546..c26c98e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -355,6 +355,7 @@  int main(int argc, char **argv)
 #endif
     pthread_t client_thread;
     const char *fmt = NULL;
+    Error *local_err = NULL;
 
     /* The client thread uses SIGTERM to interrupt the server.  A signal
      * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -573,10 +574,11 @@  int main(int argc, char **argv)
 
     bs = bdrv_new("hda");
     srcpath = argv[optind];
-    ret = bdrv_open(bs, srcpath, NULL, flags, drv);
+    ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err);
     if (ret < 0) {
         errno = -ret;
-        err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
+        err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind],
+            error_get_pretty(local_err));
     }
 
     fd_size = bdrv_getlength(bs);