[Qemu-devel] Re: [PATCH v2 3/7] docs: Add QED image format specification

[Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>]

On Sun, Oct 10, 2010 at 11:20:09AM +0200, Avi Kivity wrote:
>  On 10/08/2010 05:48 PM, Stefan Hajnoczi wrote:
> >Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
> >---
> >  docs/specs/qed_spec.txt |   94 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 94 insertions(+), 0 deletions(-)
> >
> >+Feature bits:
> >+* QED_F_BACKING_FILE = 0x01.  The image uses a backing file.
> >+* QED_F_NEED_CHECK = 0x02.  The image needs a consistency check before use.
> 
> Great that QED_F_NEED_CHECK is now non-optional.  However, suppose
> we add a new structure (e.g. persistent freelist); it's now
> impossible to tell whether the structure is updated or not:
> 
> 1 new qemu opens image
> 2 writes persistent freelist
> 3 clears need_check
> 4 shuts down
> 5 old qemu opens image
> 6 doesn't update persistent freelist
> 7 clears need_check
> 8 shuts down
> 
> The image is now inconsistent, but has need_check cleared.
> 
> We can address this by having a third feature bitmask that is
> autocleared by guests that don't recognize various bits; so the
> sequence becomes:
> 
> 1 new qemu opens image
> 2 writes persistent freelist
> 3 clears need_check
> 4 sets persistent_freelist
> 5 shuts down
> 6 old qemu opens image
> 7 clears persistent_freelist (and any other bits it doesn't recognize)
> 8 doesn't update persistent freelist
> 9 clears need_check
> 10 shuts down
> 
> The image is now consistent, since the persistent freelist has disappeared.

It is more complicated than just the feature bit.  The freelist requires
space in the image file.  Clearing the persistent_freelist bit leaks the
freelist.

We can solve this by using a compat feature bit and an autoclear feature
bit.  The autoclear bit specifies whether or not the freelist is valid
and the compat feature bit specifices whether or not the freelist
exists.

When the new qemu opens the image again it notices that the autoclear
bit is unset but the compat bit is set.  This means the freelist is
out-of-date and its space can be reclaimed.

I don't like the idea of doing these feature bit acrobatics because they
add complexity.  On the other hand your proposal ensures backward
compatibility in the case of an optional data structure that needs to
stay in sync with the image.  I'm just not 100% convinced it's worth it.

> >+* QED_CF_BACKING_FORMAT = 0x01.  The image has a specific backing file format stored.
> >+
> 
> It was suggested to have just a bit saying whether the backing
> format is raw or not.  This way you don't need to store the format.

Will fix in v3.

Stefan