qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	qemu-stable@nongnu.org, Anthony Liguori <anthony@codemonkey.ws>
Subject: [Qemu-devel] [PULL 26/42] block: expect errors from bdrv_co_is_allocated
Date: Fri,  6 Sep 2013 17:38:57 +0200	[thread overview]
Message-ID: <1378481953-23099-27-git-send-email-stefanha@redhat.com> (raw)
In-Reply-To: <1378481953-23099-1-git-send-email-stefanha@redhat.com>

From: Paolo Bonzini <pbonzini@redhat.com>

Some bdrv_is_allocated callers do not expect errors, but the fallback
in qcow2.c might make other callers trip on assertion failures or
infinite loops.

Fix the callers to always look for errors.

Cc: qemu-stable@nongnu.org
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c        |  7 +++++--
 block/cow.c    |  6 +++++-
 block/qcow2.c  |  4 +---
 block/stream.c |  2 +-
 qemu-img.c     | 16 ++++++++++++++--
 qemu-io-cmds.c |  4 ++++
 6 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index c6404eb..5df17de 100644
--- a/block.c
+++ b/block.c
@@ -1857,8 +1857,11 @@ int bdrv_commit(BlockDriverState *bs)
     buf = g_malloc(COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE);
 
     for (sector = 0; sector < total_sectors; sector += n) {
-        if (bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n)) {
-
+        ret = bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n);
+        if (ret < 0) {
+            goto ro_cleanup;
+        }
+        if (ret) {
             if (bdrv_read(bs, sector, buf, n) != 0) {
                 ret = -EIO;
                 goto ro_cleanup;
diff --git a/block/cow.c b/block/cow.c
index f4eca10..7450801 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -212,7 +212,11 @@ static int coroutine_fn cow_read(BlockDriverState *bs, int64_t sector_num,
     int ret, n;
 
     while (nb_sectors > 0) {
-        if (cow_co_is_allocated(bs, sector_num, nb_sectors, &n)) {
+        ret = cow_co_is_allocated(bs, sector_num, nb_sectors, &n);
+        if (ret < 0) {
+            return ret;
+        }
+        if (ret) {
             ret = bdrv_pread(bs->file,
                         s->cow_sectors_offset + sector_num * 512,
                         buf, n * 512);
diff --git a/block/qcow2.c b/block/qcow2.c
index 4d7bd78..cf03a14 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -696,13 +696,11 @@ static int coroutine_fn qcow2_co_is_allocated(BlockDriverState *bs,
     int ret;
 
     *pnum = nb_sectors;
-    /* FIXME We can get errors here, but the bdrv_co_is_allocated interface
-     * can't pass them on today */
     qemu_co_mutex_lock(&s->lock);
     ret = qcow2_get_cluster_offset(bs, sector_num << 9, pnum, &cluster_offset);
     qemu_co_mutex_unlock(&s->lock);
     if (ret < 0) {
-        *pnum = 0;
+        return ret;
     }
 
     return (cluster_offset != 0) || (ret == QCOW2_CLUSTER_ZERO);
diff --git a/block/stream.c b/block/stream.c
index e640bc5..078ce4a 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -124,7 +124,7 @@ wait:
         if (ret == 1) {
             /* Allocated in the top, no need to copy.  */
             copy = false;
-        } else {
+        } else if (ret >= 0) {
             /* Copy if allocated in the intermediate images.  Limit to the
              * known-unallocated area [sector_num, sector_num+n).  */
             ret = bdrv_is_allocated_above(bs->backing_hd, base,
diff --git a/qemu-img.c b/qemu-img.c
index 744c0d9..27cc006 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1508,8 +1508,15 @@ static int img_convert(int argc, char **argv)
                    are present in both the output's and input's base images (no
                    need to copy them). */
                 if (out_baseimg) {
-                    if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
-                                           n, &n1)) {
+                    ret = bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
+                                            n, &n1);
+                    if (ret < 0) {
+                        error_report("error while reading metadata for sector "
+                                     "%" PRId64 ": %s",
+                                     sector_num - bs_offset, strerror(-ret));
+                        goto out;
+                    }
+                    if (!ret) {
                         sector_num += n1;
                         continue;
                     }
@@ -2099,6 +2106,11 @@ static int img_rebase(int argc, char **argv)
 
             /* If the cluster is allocated, we don't need to take action */
             ret = bdrv_is_allocated(bs, sector, n, &n);
+            if (ret < 0) {
+                error_report("error while reading image metadata: %s",
+                             strerror(-ret));
+                goto out;
+            }
             if (ret) {
                 continue;
             }
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index f91b6c4..8565d49 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1830,6 +1830,10 @@ static int alloc_f(BlockDriverState *bs, int argc, char **argv)
     sector_num = offset >> 9;
     while (remaining) {
         ret = bdrv_is_allocated(bs, sector_num, remaining, &num);
+        if (ret < 0) {
+            printf("is_allocated failed: %s\n", strerror(-ret));
+            return 0;
+        }
         sector_num += num;
         remaining -= num;
         if (ret) {
-- 
1.8.3.1

  parent reply	other threads:[~2013-09-06 15:40 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-06 15:38 [Qemu-devel] [PULL 00/42] Block patches Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 01/42] throttle: Add a new throttling API implementing continuous leaky bucket Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 02/42] throttle: Add units tests Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 03/42] block: Enable the new throttling code in the block layer Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 04/42] block: Add support for throttling burst max in QMP and the command line Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 05/42] block: Add iops_size to do the iops accounting for a given io size Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 06/42] qemu-iotests: Adjust test result 039 Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 07/42] add qemu-img convert -n option (skip target volume creation) Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 08/42] w32: Fix access to host devices (regression) Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 09/42] aio / timers: fix build of test/test-aio.c on non-linux platforms Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 10/42] vvfat: use bdrv_new() to allocate BlockDriverState Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 11/42] iscsi: use bdrv_new() instead of stack structure Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 12/42] block: implement reference count for BlockDriverState Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 13/42] block: make bdrv_delete() static Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 14/42] migration: omit drive ref as we have bdrv_ref now Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 15/42] xen_disk: simplify blk_disconnect with refcnt Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 16/42] nbd: use BlockDriverState refcnt Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 17/42] block: use BDS ref for block jobs Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 18/42] qmp: Documentation for BLOCK_IMAGE_CORRUPTED Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 19/42] cow: make reads go at a decent speed Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 20/42] cow: make writes go at a less indecent speed Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 21/42] cow: do not call bdrv_co_is_allocated Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 22/42] block: keep bs->total_sectors up to date even for growable block devices Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 23/42] block: make bdrv_co_is_allocated static Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 24/42] block: do not use ->total_sectors in bdrv_co_is_allocated Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 25/42] block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above distinction Stefan Hajnoczi
2013-09-06 15:38 ` Stefan Hajnoczi [this message]
2013-09-06 15:38 ` [Qemu-devel] [PULL 27/42] qemu-img: always probe the input image for allocated sectors Stefan Hajnoczi
2013-09-06 15:38 ` [Qemu-devel] [PULL 28/42] block: make bdrv_has_zero_init return false for copy-on-write-images Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 29/42] block: introduce bdrv_get_block_status API Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 30/42] block: define get_block_status return value Stefan Hajnoczi
2013-09-13  8:04   ` Peter Lieven
2013-09-13  8:26     ` Paolo Bonzini
2013-09-13  8:51       ` Peter Lieven
2013-09-06 15:39 ` [Qemu-devel] [PULL 31/42] block: return get_block_status data and flags for formats Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 32/42] block: use bdrv_has_zero_init to return BDRV_BLOCK_ZERO Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 33/42] block: return BDRV_BLOCK_ZERO past end of backing file Stefan Hajnoczi
2013-09-13  7:33   ` Peter Lieven
2013-09-13  8:25     ` Paolo Bonzini
2013-09-13 15:20       ` Kevin Wolf
2013-09-16  5:45         ` Peter Lieven
2013-09-06 15:39 ` [Qemu-devel] [PULL 34/42] qemu-img: add a "map" subcommand Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 35/42] docs, qapi: document qemu-img map Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 36/42] raw-posix: return get_block_status data and flags Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 37/42] raw-posix: report unwritten extents as zero Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 38/42] block: add default get_block_status implementation for protocols Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 39/42] block: look for zero blocks in bs->file Stefan Hajnoczi
2013-09-13  8:00   ` Peter Lieven
2013-09-13  9:08     ` Peter Lieven
2013-09-13  9:10       ` Paolo Bonzini
2013-09-13  9:14         ` Peter Lieven
2013-09-13  9:17           ` Paolo Bonzini
2013-09-13  9:29             ` Peter Lieven
2013-09-06 15:39 ` [Qemu-devel] [PULL 40/42] dataplane: Fix startup race Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 41/42] qemu-iotests: Whitespace cleanup Stefan Hajnoczi
2013-09-06 15:39 ` [Qemu-devel] [PULL 42/42] qemu-iotests: Fixed test case 026 Stefan Hajnoczi

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=1378481953-23099-27-git-send-email-stefanha@redhat.com \
    --to=stefanha@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /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).