qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-stable] [PATCH 13/38] block: expect errors from b


From: Doug Goldstein
Subject: Re: [Qemu-devel] [Qemu-stable] [PATCH 13/38] block: expect errors from bdrv_co_is_allocated
Date: Wed, 25 Sep 2013 16:27:58 -0500

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]