qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Alberto Garcia <berto@igalia.com>,
	"open list:qed" <qemu-block@nongnu.org>,
	Greg Kurz <groug@kaod.org>, Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: [PULL 16/17] block/qed: bdrv_qed_do_open: deal with errp
Date: Tue,  9 Mar 2021 09:52:01 -0600	[thread overview]
Message-ID: <20210309155202.1312571-17-eblake@redhat.com> (raw)
In-Reply-To: <20210309155202.1312571-1-eblake@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Always set errp on failure. The generic bdrv_open_driver supports
driver functions which can return a negative value but forget to set
errp.  That's a strange thing. Let's improve bdrv_qed_do_open to not
behave this way. This allows the simplification of code in
bdrv_qed_co_invalidate_cache().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210202124956.63146-14-vsementsov@virtuozzo.com>
[eblake: commit message grammar tweak]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/qed.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index b27e7546cabd..f45c640513f5 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -393,6 +393,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,

     ret = bdrv_pread(bs->file, 0, &le_header, sizeof(le_header));
     if (ret < 0) {
+        error_setg(errp, "Failed to read QED header");
         return ret;
     }
     qed_header_le_to_cpu(&le_header, &s->header);
@@ -408,25 +409,30 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,
         return -ENOTSUP;
     }
     if (!qed_is_cluster_size_valid(s->header.cluster_size)) {
+        error_setg(errp, "QED cluster size is invalid");
         return -EINVAL;
     }

     /* Round down file size to the last cluster */
     file_size = bdrv_getlength(bs->file->bs);
     if (file_size < 0) {
+        error_setg(errp, "Failed to get file length");
         return file_size;
     }
     s->file_size = qed_start_of_cluster(s, file_size);

     if (!qed_is_table_size_valid(s->header.table_size)) {
+        error_setg(errp, "QED table size is invalid");
         return -EINVAL;
     }
     if (!qed_is_image_size_valid(s->header.image_size,
                                  s->header.cluster_size,
                                  s->header.table_size)) {
+        error_setg(errp, "QED image size is invalid");
         return -EINVAL;
     }
     if (!qed_check_table_offset(s, s->header.l1_table_offset)) {
+        error_setg(errp, "QED table offset is invalid");
         return -EINVAL;
     }

@@ -438,6 +444,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,

     /* Header size calculation must not overflow uint32_t */
     if (s->header.header_size > UINT32_MAX / s->header.cluster_size) {
+        error_setg(errp, "QED header size is too large");
         return -EINVAL;
     }

@@ -445,6 +452,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,
         if ((uint64_t)s->header.backing_filename_offset +
             s->header.backing_filename_size >
             s->header.cluster_size * s->header.header_size) {
+            error_setg(errp, "QED backing filename offset is invalid");
             return -EINVAL;
         }

@@ -453,6 +461,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,
                               bs->auto_backing_file,
                               sizeof(bs->auto_backing_file));
         if (ret < 0) {
+            error_setg(errp, "Failed to read backing filename");
             return ret;
         }
         pstrcpy(bs->backing_file, sizeof(bs->backing_file),
@@ -475,6 +484,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,

         ret = qed_write_header_sync(s);
         if (ret) {
+            error_setg(errp, "Failed to update header");
             return ret;
         }

@@ -487,6 +497,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,

     ret = qed_read_l1_table_sync(s);
     if (ret) {
+        error_setg(errp, "Failed to read L1 table");
         goto out;
     }

@@ -503,6 +514,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options,

             ret = qed_check(s, &result, true);
             if (ret) {
+                error_setg(errp, "Image corrupted");
                 goto out;
             }
         }
@@ -1537,22 +1549,16 @@ static void coroutine_fn bdrv_qed_co_invalidate_cache(BlockDriverState *bs,
                                                       Error **errp)
 {
     BDRVQEDState *s = bs->opaque;
-    Error *local_err = NULL;
     int ret;

     bdrv_qed_close(bs);

     bdrv_qed_init_state(bs);
     qemu_co_mutex_lock(&s->table_lock);
-    ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, &local_err);
+    ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, errp);
     qemu_co_mutex_unlock(&s->table_lock);
-    if (local_err) {
-        error_propagate_prepend(errp, local_err,
-                                "Could not reopen qed layer: ");
-        return;
-    } else if (ret < 0) {
-        error_setg_errno(errp, -ret, "Could not reopen qed layer");
-        return;
+    if (ret < 0) {
+        error_prepend(errp, "Could not reopen qed layer: ");
     }
 }

-- 
2.30.1



  parent reply	other threads:[~2021-03-09 16:22 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-09 15:51 [PULL 00/17] NBD patches through 2021-03-09 Eric Blake
2021-03-09 15:51 ` [PULL 01/17] MAINTAINERS: add Vladimir as co-maintainer of NBD Eric Blake
2021-03-09 15:51 ` [PULL 02/17] nbd: server: Report holes for raw images Eric Blake
2021-03-09 15:51 ` [PULL 03/17] utils: Enhance testsuite for do_strtosz() Eric Blake
2021-03-09 15:51 ` [PULL 04/17] utils: Improve qemu_strtosz() to have 64 bits of precision Eric Blake
2021-03-09 15:51 ` [PULL 05/17] utils: Deprecate hex-with-suffix sizes Eric Blake
2021-03-09 15:51 ` [PULL 06/17] block: check return value of bdrv_open_child and drop error propagation Eric Blake
2021-03-09 15:51 ` [PULL 07/17] blockdev: fix drive_backup_prepare() missed error Eric Blake
2021-03-09 15:51 ` [PULL 08/17] block: drop extra error propagation for bdrv_set_backing_hd Eric Blake
2021-03-09 15:51 ` [PULL 09/17] block/mirror: drop extra error propagation in commit_active_start() Eric Blake
2021-03-09 15:51 ` [PULL 10/17] blockjob: return status from block_job_set_speed() Eric Blake
2021-03-09 15:51 ` [PULL 11/17] block/qcow2: qcow2_get_specific_info(): drop error propagation Eric Blake
2021-03-09 15:51 ` [PULL 12/17] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface Eric Blake
2021-03-09 15:51 ` [PULL 13/17] block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps Eric Blake
2021-03-09 15:51 ` [PULL 14/17] block/qcow2: read_cache_sizes: return status value Eric Blake
2021-03-09 15:52 ` [PULL 15/17] block/qcow2: simplify qcow2_co_invalidate_cache() Eric Blake
2021-03-09 15:52 ` Eric Blake [this message]
2021-03-09 15:52 ` [PULL 17/17] block/qcow2: refactor qcow2_update_options_prepare error paths Eric Blake
2021-03-11 16:20 ` [PULL 00/17] NBD patches through 2021-03-09 Peter Maydell
2021-03-11 19:02 ` Peter Maydell
2021-03-11 19:21   ` Eric Blake
2021-03-11 20:40     ` Peter Maydell
2021-03-17 11:17   ` Thomas Huth
2021-03-17 11:28     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210309155202.1312571-17-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=berto@igalia.com \
    --cc=groug@kaod.org \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).