[Qemu-devel] Re: [RFC][STABLE 0.13] Revert "qcow2: Use bdrv_(p)write_sync for metadata writes"

[Avi Kivity]

 On 08/24/2010 04:56 PM, Anthony Liguori wrote:
> > One doesn't follow from the other (though I'm no fan of internal 
> > snapshots, myself).
>
>
> It does.  Let's consider the failure scenarios:
>
> 1) guest submits write request
> 2) allocate extent
> 3) write data to disk (a)
> 4) write (a) completes
> 5) update reference count table for new extent (b)
> 6) write (b) completes
> 7) write extent table (c)
> 8) write (c) completes
> 9) complete guest write request
>
> If this all happened in order and we lost power, the worst case error 
> is that we leak a block which isn't terrible.
>
> But we're not guaranteed that this happens in order.
>
> If (b) or (c) happen before (a), then the image is not corrupted but 
> data gets lost.  That's okay because it's part of the guest contract.
>
> If (c) happens before (b), then we've created an extent that's 
> attached to a table with a zero reference count.  This is a corrupt 
> image.

If the only issue is new block allocation, it can be easily solved.  
Instead of allocating exactly the needed amount of blocks, allocate a 
large extent and hold them in memory.  The next allocation can then be 
filled from memory, so the allocation sync is amortized over many 
blocks.  A power fail will leak the preallocated blocks, losing some 
megabytes of address space, but not real disk space.


> Let's consider if we eliminate the reference count table which means 
> eliminating internal snapshots.
>
> 1) guest submits write request
> 2) allocate extent
> 3) write data to disk (a)
> 4) write (a) completes
> 5) write extent table (c)
> 6) write (c) completes
> 7) complete guest write request
>
> If this all happens in order and we lose power, we just leak a block.  
> It means we need a periodic fsck.
>
> If (c) completes before (a), then it means that the image is not 
> corrupted but data gets lost.  This is okay based on the guest contract.
>
> And that's it.  There is no scenario where the disk is corrupted.

_if_ that's the only failure mode.


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.