qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
To: Avi Kivity <avi@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	qemu-devel@nongnu.org, Christoph Hellwig <hch@lst.de>
Subject: [Qemu-devel] Re: [PATCH v2 3/7] docs: Add QED image format specification
Date: Mon, 11 Oct 2010 14:42:41 +0100	[thread overview]
Message-ID: <20101011134241.GA5439@stefan-thinkpad.transitives.com> (raw)
In-Reply-To: <4CB30B43.2040706@redhat.com>

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<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.
> 
> 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

  reply	other threads:[~2010-10-11 13:43 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-08 15:48 [Qemu-devel] [PATCH v2 0/7] qed: Add QEMU Enhanced Disk format Stefan Hajnoczi
2010-10-08 15:48 ` [Qemu-devel] [PATCH v2 1/7] qcow2: Make get_bits_from_size() common Stefan Hajnoczi
2010-10-08 18:01   ` [Qemu-devel] " Anthony Liguori
2010-10-08 15:48 ` [Qemu-devel] [PATCH v2 2/7] cutils: Add bytes_to_str() to format byte values Stefan Hajnoczi
2010-10-11 11:09   ` [Qemu-devel] " Kevin Wolf
2010-10-13  9:15   ` [Qemu-devel] " Markus Armbruster
2010-10-13  9:28     ` Kevin Wolf
2010-10-13 10:58       ` Stefan Hajnoczi
2010-10-13 10:25   ` [Qemu-devel] " Avi Kivity
2010-10-08 15:48 ` [Qemu-devel] [PATCH v2 3/7] docs: Add QED image format specification Stefan Hajnoczi
2010-10-10  9:20   ` [Qemu-devel] " Avi Kivity
2010-10-11 10:09     ` Stefan Hajnoczi
2010-10-11 13:04       ` Avi Kivity
2010-10-11 13:42         ` Stefan Hajnoczi [this message]
2010-10-11 13:44           ` Avi Kivity
2010-10-11 14:06             ` Stefan Hajnoczi
2010-10-11 14:12               ` Avi Kivity
2010-10-11 15:02             ` Anthony Liguori
2010-10-11 15:24               ` Avi Kivity
2010-10-11 15:41                 ` Anthony Liguori
2010-10-11 15:47                   ` Avi Kivity
2010-10-11 14:54         ` Anthony Liguori
2010-10-11 14:58           ` Avi Kivity
2010-10-11 15:49             ` Anthony Liguori
2010-10-11 16:02               ` Avi Kivity
2010-10-11 16:10                 ` Anthony Liguori
2010-10-12 10:25                   ` Avi Kivity
2010-10-11 13:58   ` Kevin Wolf
2010-10-11 15:30     ` Stefan Hajnoczi
2010-10-11 15:39       ` Avi Kivity
2010-10-11 15:46         ` Stefan Hajnoczi
2010-10-11 16:18           ` Anthony Liguori
2010-10-11 17:14             ` Anthony Liguori
2010-10-12  8:07               ` Kevin Wolf
2010-10-12 13:16                 ` Stefan Hajnoczi
2010-10-12 13:32                   ` Anthony Liguori
2010-10-11 15:50       ` Kevin Wolf
2010-10-08 15:48 ` [Qemu-devel] [PATCH v2 4/7] qed: Add QEMU Enhanced Disk image format Stefan Hajnoczi
2010-10-11 15:16   ` [Qemu-devel] " Kevin Wolf
2010-10-08 15:48 ` [Qemu-devel] [PATCH v2 5/7] qed: Table, L2 cache, and cluster functions Stefan Hajnoczi
2010-10-12 14:44   ` [Qemu-devel] " Kevin Wolf
2010-10-13 13:41     ` Stefan Hajnoczi
2010-10-08 15:48 ` [Qemu-devel] [PATCH v2 6/7] qed: Read/write support Stefan Hajnoczi
2010-10-10  9:10   ` [Qemu-devel] " Avi Kivity
2010-10-11 10:37     ` Stefan Hajnoczi
2010-10-11 13:10       ` Avi Kivity
2010-10-11 13:55         ` Stefan Hajnoczi
2010-10-11 14:57         ` Anthony Liguori
2010-10-12 15:08   ` Kevin Wolf
2010-10-12 15:22     ` Anthony Liguori
2010-10-12 15:39       ` Kevin Wolf
2010-10-12 15:59         ` Stefan Hajnoczi
2010-10-12 16:16           ` Anthony Liguori
2010-10-12 16:21             ` Avi Kivity
2010-10-13 12:13             ` Stefan Hajnoczi
2010-10-13 13:07               ` Kevin Wolf
2010-10-13 13:24                 ` Anthony Liguori
2010-10-13 13:50                   ` Avi Kivity
2010-10-13 14:07                     ` Stefan Hajnoczi
2010-10-13 14:08                       ` Anthony Liguori
2010-10-13 14:10                       ` Avi Kivity
2010-10-13 14:11                         ` Anthony Liguori
2010-10-13 14:16                           ` Avi Kivity
2010-10-13 14:53                             ` Anthony Liguori
2010-10-13 15:08                               ` Avi Kivity
2010-10-13 15:42                                 ` Anthony Liguori
2010-10-14 11:06                         ` Stefan Hajnoczi
2010-10-13 14:10                     ` Anthony Liguori
2010-10-08 15:48 ` [Qemu-devel] [PATCH v2 7/7] qed: Consistency check support Stefan Hajnoczi
2010-10-11 13:21 ` [Qemu-devel] Re: [PATCH v2 0/7] qed: Add QEMU Enhanced Disk format Kevin Wolf
2010-10-11 15:37   ` Stefan Hajnoczi
2010-10-16  7:51 ` [Qemu-devel] " Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101011134241.GA5439@stefan-thinkpad.transitives.com \
    --to=stefanha@linux.vnet.ibm.com \
    --cc=aliguori@us.ibm.com \
    --cc=avi@redhat.com \
    --cc=hch@lst.de \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).