Il 25/09/2013 23:27, Doug Goldstein ha scritto: > On Wed, Sep 25, 2013 at 7:57 AM, Michael Roth <mdr...@linux.vnet.ibm.com> > wrote: >> From: Paolo Bonzini <pbonz...@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-sta...@nongnu.org >> Reviewed-by: Eric Blake <ebl...@redhat.com> >> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> >> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> >> (cherry picked from commit d663640c04f2aab810915c556390211d75457704) >> >> Conflicts: >> >> block/cow.c >> >> *modified to avoid dependency on upstream's e641c1e8 >> >> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.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 d5ce8d3..8ce8b91 100644 >> --- a/block.c >> +++ b/block.c >> @@ -1803,8 +1803,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 1cc2e89..e1b73d6 100644 >> --- a/block/cow.c >> +++ b/block/cow.c >> @@ -189,7 +189,11 @@ static int coroutine_fn cow_read(BlockDriverState *bs, >> int64_t sector_num, >> int ret, n; >> >> while (nb_sectors > 0) { >> - if (bdrv_co_is_allocated(bs, sector_num, nb_sectors, &n)) { >> + ret = bdrv_co_is_allocated(bs, sector_num, nb_sectors, &n); > > Is suppose to be ret = cow_co_is_allocated() ?
No, it's correct to have it like this in the backport. >> + 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 3376901..7f7282e 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 7fe9e48..4e8d177 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_co_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 ffbcf31..ffe48ad 100644 >> --- a/qemu-io-cmds.c >> +++ b/qemu-io-cmds.c >> @@ -1829,6 +1829,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.7.9.5 >> >> >