Re: [Qemu-devel] Re: [PATCH] qcow2/virtio corruption: Don't allocate the same cluster twice

[Avi Kivity]

Gleb Natapov wrote:
> The reason we need to copy unmodified sectors in alloc_cluster_link_l2()
> is exactly to handle concurrent writes into the same cluster. This is
> essentially RMW. I don't see why concurrent writes should not work with
> the logic in place. There is a bug there currently of cause :) Can
> somebody check this patch:
>
>
> diff --git a/block-qcow2.c b/block-qcow2.c
> index 7840634..801d26d 100644
> --- a/block-qcow2.c
> +++ b/block-qcow2.c
> @@ -995,8 +995,8 @@ static int alloc_cluster_link_l2(BlockDriverState *bs, 
> uint64_t cluster_offset,
>          if(l2_table[l2_index + i] != 0)
>              old_cluster[j++] = l2_table[l2_index + i];
>  
> -        l2_table[l2_index + i] = cpu_to_be64((cluster_offset +
> -                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
> +        l2_table[l2_index + i] = cpu_to_be64(((cluster_offset +
> +                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED));
>       }
>  
>      if (bdrv_pwrite(s->hd, l2_offset + l2_index * sizeof(uint64_t),
> @@ -1005,7 +1005,8 @@ static int alloc_cluster_link_l2(BlockDriverState *bs, 
> uint64_t cluster_offset,
>          goto err;
>  
>      for (i = 0; i < j; i++)
> -        free_any_clusters(bs, be64_to_cpu(old_cluster[i]), 1);
> +        free_any_clusters(bs, be64_to_cpu(old_cluster[i]) & ~QCOW_OFLAG_COPIED,
> +                          1);
>  
>      ret = 0;
>  err:
>   

I can confirm that it fixes the problem.

--
error compiling committee.c: too many arguments to function