From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-stable@nongnu.org, stefanha@redhat.com
Subject: [Qemu-devel] [PATCH v5 08/21] block: expect errors from bdrv_co_is_allocated
Date: Wed, 4 Sep 2013 19:00:25 +0200 [thread overview]
Message-ID: <1378314038-15525-9-git-send-email-pbonzini@redhat.com> (raw)
In-Reply-To: <1378314038-15525-1-git-send-email-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>
---
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 1c8899a..a3f7577 100644
--- a/block.c
+++ b/block.c
@@ -1829,8 +1829,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 55bac50..61656fa 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 4bc679a..6b06a8e 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 9a63eaf..b0cbf34 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 b9a848d..b01998b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1485,8 +1485,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;
}
@@ -2076,6 +2083,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
next prev parent reply other threads:[~2013-09-04 17:01 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 01/21] cow: make reads go at a decent speed Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 02/21] cow: make writes go at a less indecent speed Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 03/21] cow: do not call bdrv_co_is_allocated Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 04/21] block: keep bs->total_sectors up to date even for growable block devices Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 05/21] block: make bdrv_co_is_allocated static Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 06/21] block: do not use ->total_sectors in bdrv_co_is_allocated Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 07/21] block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above distinction Paolo Bonzini
2013-09-04 17:00 ` Paolo Bonzini [this message]
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 09/21] qemu-img: always probe the input image for allocated sectors Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 10/21] block: make bdrv_has_zero_init return false for copy-on-write-images Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 11/21] block: introduce bdrv_get_block_status API Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value Paolo Bonzini
2014-05-05 14:34 ` Kevin Wolf
2014-05-05 14:58 ` Paolo Bonzini
2014-05-06 11:34 ` Kevin Wolf
2014-05-06 12:31 ` Paolo Bonzini
2014-05-06 13:08 ` Kevin Wolf
2014-05-06 13:20 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 13/21] block: return get_block_status data and flags for formats Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 14/21] block: use bdrv_has_zero_init to return BDRV_BLOCK_ZERO Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 15/21] block: return BDRV_BLOCK_ZERO past end of backing file Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 16/21] qemu-img: add a "map" subcommand Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 17/21] docs, qapi: document qemu-img map Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 18/21] raw-posix: return get_block_status data and flags Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 19/21] raw-posix: report unwritten extents as zero Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 20/21] block: add default get_block_status implementation for protocols Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 21/21] block: look for zero blocks in bs->file Paolo Bonzini
2013-09-05 13:55 ` [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata 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=1378314038-15525-9-git-send-email-pbonzini@redhat.com \
--to=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
--cc=stefanha@redhat.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).