Re: [PATCH v7 37/47] qemu-img: Use child access functions

[Kevin Wolf]

Am 25.06.2020 um 17:22 hat Max Reitz geschrieben:
> This changes iotest 204's output, because blkdebug on top of a COW node
> used to make qemu-img map disregard the rest of the backing chain (the
> backing chain was broken by the filter).  With this patch, the
> allocation in the base image is reported correctly.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

> @@ -3437,6 +3441,7 @@ static int img_rebase(int argc, char **argv)
>      uint8_t *buf_old = NULL;
>      uint8_t *buf_new = NULL;
>      BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
> +    BlockDriverState *unfiltered_bs;
>      char *filename;
>      const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
>      int c, flags, src_flags, ret;
> @@ -3571,6 +3576,8 @@ static int img_rebase(int argc, char **argv)
>      }
>      bs = blk_bs(blk);
>  
> +    unfiltered_bs = bdrv_skip_filters(bs);
> +
>      if (out_basefmt != NULL) {
>          if (bdrv_find_format(out_basefmt) == NULL) {
>              error_report("Invalid format name: '%s'", out_basefmt);
> @@ -3582,7 +3589,7 @@ static int img_rebase(int argc, char **argv)
>      /* For safe rebasing we need to compare old and new backing file */
>      if (!unsafe) {
>          QDict *options = NULL;
> -        BlockDriverState *base_bs = backing_bs(bs);
> +        BlockDriverState *base_bs = bdrv_cow_bs(unfiltered_bs);
>  
>          if (base_bs) {
>              blk_old_backing = blk_new(qemu_get_aio_context(),
> @@ -3738,8 +3745,9 @@ static int img_rebase(int argc, char **argv)
>                   * If cluster wasn't changed since prefix_chain, we don't 
> need
>                   * to take action
>                   */
> -                ret = bdrv_is_allocated_above(backing_bs(bs), 
> prefix_chain_bs,
> -                                              false, offset, n, &n);
> +                ret = bdrv_is_allocated_above(bdrv_cow_bs(unfiltered_bs),
> +                                              prefix_chain_bs, false,
> +                                              offset, n, &n);
>                  if (ret < 0) {
>                      error_report("error while reading image metadata: %s",
>                                   strerror(-ret));

img_rebase() has these additional calls:

    /* If the cluster is allocated, we don't need to take action */
    ret = bdrv_is_allocated(bs, offset, n, &n);

And:

    if (out_baseimg && *out_baseimg) {
        ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt);
    } else {
        ret = bdrv_change_backing_file(bs, NULL, NULL);
    }

Shouldn't they use unfiltered_bs?

(Not that it's likely that anyone would use 'qemu-img rebase' with a
filter, but while you're touching it...)

Kevin