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

[Stefan Hajnoczi]

On Mon, Oct 11, 2010 at 03:04:03PM +0200, Avi Kivity wrote:
>  On 10/11/2010 12:09 PM, Stefan Hajnoczi wrote:
> >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<address@hidden>
> >>  >---
> >>  >   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.
> 
> My scenario ends up with data corruption if we move to an old qemu
> and then back again, without any aborts.
> 
> A leak is acceptable (it won't grow; it's just an unused, incorrect
> freelist), but data corruption is not.

The alternative is for the freelist to be a non-compat feature bit.
That means older QEMU binaries cannot use a QED image that has enabled
the freelist.

Stefan