| Message ID | 1372862071-28225-7-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 03, 2013 at 04:34:20PM +0200, Paolo Bonzini wrote: > diff --git a/qemu-img.c b/qemu-img.c > index f8c97d3..857e2ca 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -2073,6 +2073,10 @@ 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 from file"); > + goto out; > + } We should print the errno valid and saying "while reading from file" is a little misleading: "error while checking cluster allocation status: %d", ret
Il 05/07/2013 11:19, Stefan Hajnoczi ha scritto: > > /* 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 from file"); > > + goto out; > > + } > > We should print the errno valid and saying "while reading from file" is > a little misleading: > > "error while checking cluster allocation status: %d", ret I think this is too specific. In the end, errors from bdrv_is_allocated are almost always due to a problem reading the metadata from the file. I will change it to "error reading image metadata", or something like that. (And also use strerror). Paolo
diff --git a/block/qcow2.c b/block/qcow2.c index 0eceefe..e2b4202 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -648,13 +648,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 802acae..28b702d 100644 --- a/block/stream.c +++ b/block/stream.c @@ -120,7 +120,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 f8c97d3..857e2ca 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2073,6 +2073,10 @@ 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 from file"); + goto out; + } if (ret) { continue; }
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 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block/qcow2.c | 4 +--- block/stream.c | 2 +- qemu-img.c | 4 ++++ 3 files changed, 6 insertions(+), 4 deletions(-)