Re: [Qemu-devel] [BUG] qemu-1.1.2 [FIXED-BY] qcow2: Fix avail_sectors in cluster allocation code

[Kevin Wolf]

Am 18.12.2012 10:46, schrieb Philipp Hahn:
> Hello Kevin, hello Michael,
> 
> On Wednesday 12 December 2012 17:54:58 Kevin Wolf wrote:
>> Am 12.12.2012 15:09, schrieb Philipp Hahn:
>>> Am Mittwoch 12 Dezember 2012 14:41:49 schrieb Kevin Wolf:
>>>> As you can see in the commit message of that patch I was convinced that
>>>> no bug did exist in practice and this was only dangerous with respect to
>>>> future changes. Therefore my first question is if you're using an
>>>> unmodified upstream qemu or if some backported patches are applied to
>>>> it? If it's indeed unmodified, we should probably review the code once
>>>> again to understand why it makes a difference.
>>>
>>> This were all unmodified versions directly from git between
>>> "qemu-kvm-1.1.0" and "qemu-kvm-1.2.0"
>>>
>>> "git checkout b7ab0fea37c15ca9e249c42c46f5c48fd1a0943c" works,
>>> "git checkout b7ab0fea37c15ca9e249c42c46f5c48fd1a0943c~1" is broken.
>>> "git checkout qemu-kvm-1.1.2"  is broken,
>>> "git checkout qemu-kvm-1.1.2 ; git cherry-pick
>>> b7ab0fea37c15ca9e249c42c46f5c48fd1a0943c"  works
>>
>> Ok, thanks for clarifying. Then I must have missed some interesting case
>> while doing the patch.
> 
> I think I found your missing link:
> After filling in "QCowL2Meta *m", that request ist queued:
>   QLIST_INSERT_HEAD(&s->cluster_allocs, m, next_in_flight);
> do prevent double allocating the same cluster for overlapping requests, which 
> is checked in do_alloc_cluster_offset().
> I guess that since the sector count was wrong, the overlap detection didn't 
> work and the two concurrent write requests to the same cluster overwrote each 
> other.

I'm still not seeing it. The overlap detection code doesn't use
m->nb_available at all, so why would it make a difference?

I guess I need some people who can decently review code - Laszlo, Paolo,
any idea why upstream commit b7ab0fea does change the behaviour,
contrary to what I claimed in the commit message?

>> Ideally we would find a sequence of qemu-io commands to reliably
>> reproduce this.
> 
> You're the block guru, so I leave that to you (or anybody else who knows more 
> about the working of qemu-io.) ;-)

Yeah, as soon as I know what's happening, calling qemu-io with the right
options shouldn't be a problem.

Kevin