Re: [Qemu-devel] [PATCH 1/5] qcow2: Fix warnings in check_refcount()

[Anthony Liguori <aliguori@us.ibm.com>]

Kevin Wolf wrote:
> Am Freitag, 17. April 2009 23:03 schrieb Anthony Liguori:
>   
>> I'm basically at the point of not wanting to touch qcow2 without serious
>> testing.  That said, I can do enough on my own to satisfy me so I'll
>> commit this series later today or tomorrow.
>>     
>
> I perfectly understand that you don't want to break it again. But then, the 
> only way to avoid new bugs is to stop development completely. This isn't a 
> solution either.
>   

Every patch I commit gets tested.  I have various tests that I run 
depending on which subsystem the patch touches.  Right now, for qcow2, I 
don't have nearly enough.  I was hoping that Christoph had something 
laying around that I could use since it looks like qemu-io would be a 
great harness for qcow2 changes.

But I can write a pretty easy script myself on top of qemu-io so it's no 
big deal.  I'm not suggesting holding up development.

> This is even more true for changes which are actually made for testing and 
> debugging purposes like these. This series is what helped me to find the 
> corruption bug.
>
> What we should do is to make sure that qcow2 patches (especially those 
> touching the core) are given a thorough review before committing.
>   

In theory, r5006 did.  That wasn't enough.

>>> And even though I think that this series can't break anything, we
>>> definitely could use a strong test suite. I'm almost sure that there is
>>> at least one bug left (the one Jamie Lokier saw from 5006 on, but nobody
>>> ever found it).
>>>       
>> You don't think that was Nolan's fix?
>>     
>
> Hm, I haven't look very much in detail at it. But according to the commit log 
> only qcow_is_allocated() was affected, and I can't see how booting Jamie's 
> Windows guest would call this function.
>   

The bug was in get_cluster_offset() so it could have caused much more 
subtle breakages.

-- 
Regards,

Anthony Liguori