From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48082) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SA1N2-0003rz-9C for qemu-devel@nongnu.org; Tue, 20 Mar 2012 11:53:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SA1Mb-00080C-C3 for qemu-devel@nongnu.org; Tue, 20 Mar 2012 11:53:11 -0400 Received: from mail-lb0-f173.google.com ([209.85.217.173]:41967) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SA1Mb-0007zP-1o for qemu-devel@nongnu.org; Tue, 20 Mar 2012 11:52:45 -0400 Received: by lbon3 with SMTP id n3so162112lbo.4 for ; Tue, 20 Mar 2012 08:52:41 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20120320154202.GA896@stefanha-thinkpad.localdomain> References: <1331055149-10982-1-git-send-email-benoit.canet@gmail.com> <1331055149-10982-9-git-send-email-benoit.canet@gmail.com> <20120320154202.GA896@stefanha-thinkpad.localdomain> Date: Tue, 20 Mar 2012 16:52:41 +0100 Message-ID: From: =?UTF-8?Q?Beno=C3=AEt_Canet?= Content-Type: multipart/alternative; boundary=bcaec54fbd3c3f703004bbaeaada Subject: Re: [Qemu-devel] [RFC PATCH 08/10] qed: add bdrv_post_incoming_migration operation checking the image List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com --bcaec54fbd3c3f703004bbaeaada Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable So should I drop the "block: rename *_invalidate_cache_* to *_post_incoming_migration_*" commit ? On Tue, Mar 20, 2012 at 4:42 PM, Stefan Hajnoczi wrote= : > On Tue, Mar 06, 2012 at 06:32:27PM +0100, Beno=C3=AEt Canet wrote: > > @@ -1529,6 +1529,19 @@ static int > bdrv_qed_change_backing_file(BlockDriverState *bs, > > return ret; > > } > > > > +static void bdrv_qed_check_if_needed(BlockDriverState *bs) > > +{ > > + /* close the block device if the verification fail */ > > + if (check_image_if_needed(bs)) { > > + bdrv_close(bs); > > + } > > We open the image for incoming migration while the VM is still running. > That means the metadata and header can still change before migration > switchover. So we need to drop everything we know about this image and > start from scratch. > > qcow2 does it like this: > > qcow2_close(bs); > memset(s, 0, sizeof(BDRVQcowState)); > qcow2_open(bs, flags); > > We should do something similar so that the QED header is re-read. This > probably means check_image_if_needed() doesn't need to be factored out > of qed_open(). > > (Note that the underlying file and BlockDriverState don't get reopened, > we're simply reinitializing the QED/QCOW2 image format layer here.) > > Stefan > --bcaec54fbd3c3f703004bbaeaada Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

So should I drop the "block: rename *_invalidate_cache_= * to *_post_incoming_migration_*" commit ?

On Tue, Mar 20, 2012 at 4:42 PM, Stefan Hajnoczi &= lt;stefanha@gmail.com> = wrote:
On Tue, Mar 06, 2012 at 06= :32:27PM +0100, Beno=C3=AEt Canet wrote:
> @@ -1529,6 +1529,19 @@ static int bdrv_qed_change_backing_file(BlockDr= iverState *bs,
> =C2=A0 =C2=A0 =C2=A0return ret;
> =C2=A0}
>
> +static void bdrv_qed_check_if_needed(BlockDriverState *bs)
> +{
> + =C2=A0 =C2=A0/* close the block device if the verification fail */ > + =C2=A0 =C2=A0if (check_image_if_needed(bs)) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0bdrv_close(bs);
> + =C2=A0 =C2=A0}

We open the image for incoming migration while the VM is still runnin= g.
That means the metadata and header can still change before migration
switchover. =C2=A0So we need to drop everything we know about this image an= d
start from scratch.

qcow2 does it like this:

qcow2_close(bs);
memset(s, 0, sizeof(BDRVQcowState));
qcow2_open(bs, flags);

We should do something similar so that the QED header is re-read. =C2=A0Thi= s
probably means check_image_if_needed() doesn't need to be factored out<= br> of qed_open().

(Note that the underlying file and BlockDriverState don't get reopened,=
we're simply reinitializing the QED/QCOW2 image format layer here.)

Stefan

--bcaec54fbd3c3f703004bbaeaada--