Re: [Qemu-devel] [PATCH] qcow2: make is_allocated return true for zero clusters

[Kevin Wolf <kwolf@redhat.com>]

Am 06.03.2013 um 18:02 hat Paolo Bonzini geschrieben:
> Otherwise, live migration of the top layer will miss zero clusters and
> let the backing file show through.  This also matches what is done in qed.
> 
> QCOW2_CLUSTER_ZERO clusters are invalid in v2 image files.  Check this
> directly in qcow2_get_cluster_offset instead of replicating the test
> everywhere.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Can you add a test case for this?

Also is_allocated() probably is the wrong interface now because it can
mean different things. The content of a zero cluster is indeed defined
by the image, but it may or may not be fully allocated yet. Have you
checked if the callers use it consistently in the former way?

>  block/qcow2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 56fccf9..bb04432 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -454,6 +454,9 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
>          *cluster_offset &= L2E_COMPRESSED_OFFSET_SIZE_MASK;
>          break;
>      case QCOW2_CLUSTER_ZERO:
> +        if (s->qcow_version < 3) {
> +            return -EIO;
> +        }

This leaks a cache entry.

Kevin