On Wed, Sep 25, 2013 at 7:57 AM, Michael Roth <[email protected]> wrote:
> From: Paolo Bonzini <[email protected]>
>
> 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: [email protected]
> Reviewed-by: Eric Blake <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> Signed-off-by: Stefan Hajnoczi <[email protected]>
> (cherry picked from commit d663640c04f2aab810915c556390211d75457704)
>
> Conflicts:
>
> block/cow.c
>
> *modified to avoid dependency on upstream's e641c1e8
>
> Signed-off-by: Michael Roth <[email protected]>
> ---
> 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() ?
> + 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
>
>
--
Doug Goldstein
