When doing a measure on an image with a backing file and discard-no-unref is enabled, the code should take this into account.
If for example you have a snapshot image with a base, and you do a discard within the snapshot, it will be ZERO and ALLOCATED, but without host offset. Now if we commit this snapshot, and the clusters in the base image were allocated, the clusters will only be set to ZERO, but the host offset will not be cleared. Therefor ZERO & ALLOCATED clusters in the top image need to check the base to see if space will be freed or not, to have a correct measure output. Bug-Url: https://gitlab.com/qemu-project/qemu/-/issues/2369 Signed-off-by: Jean-Louis Dupond <jean-lo...@dupond.be> --- block/qcow2.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 956128b409..1ce7ebbab4 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -5163,9 +5163,16 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs, } else { int64_t offset; int64_t pnum = 0; + BlockDriverState *parent = bdrv_filter_or_cow_bs(in_bs); + BDRVQcow2State *s = NULL; + + if (parent) { + s = parent->opaque; + } for (offset = 0; offset < ssize; offset += pnum) { int ret; + int retp = 0; ret = bdrv_block_status_above(in_bs, NULL, offset, ssize - offset, &pnum, NULL, @@ -5176,10 +5183,33 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs, goto err; } - if (ret & BDRV_BLOCK_ZERO) { + /* If we have a parent in the chain and the current block is zero but allocated, + * then we want to check the allocation state of the parent block. + * If it was allocated and now zero, we want + * to include it into the calculation, cause it will not free space when + * committing the top into base with discard-no-unref enabled. + */ + if (parent && + ((ret & (BDRV_BLOCK_ZERO | BDRV_BLOCK_ALLOCATED)) == + (BDRV_BLOCK_ZERO | BDRV_BLOCK_ALLOCATED)) && + s->discard_no_unref) { + int64_t pnum_parent = 0; + retp = bdrv_block_status_above(parent, NULL, offset, + ssize - offset, &pnum_parent, NULL, + NULL); + // Check if parent block has an offset + if (retp & BDRV_BLOCK_OFFSET_VALID) { + pnum = retp; + } + } + if (ret & BDRV_BLOCK_ZERO && !retp) { /* Skip zero regions (safe with no backing file) */ - } else if ((ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) == - (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) { + } else if (((ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) == + (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) || + (((ret & (BDRV_BLOCK_ZERO | BDRV_BLOCK_ALLOCATED)) == + (BDRV_BLOCK_ZERO | BDRV_BLOCK_ALLOCATED)) && + s && s->discard_no_unref && + retp & BDRV_BLOCK_OFFSET_VALID)) { /* Extend pnum to end of cluster for next iteration */ pnum = ROUND_UP(offset + pnum, cluster_size) - offset; -- 2.45.2
