On 21.08.20 17:29, Kevin Wolf wrote: > 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 <mre...@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?
Oh, yes, the second one definitely. As for the first one, I don’t think there’s a difference. But why not, we really want to query unfiltered_bs, so it’s better to do so explicitly than through the implicit fall-through behavior of block_status. Max
signature.asc
Description: OpenPGP digital signature