From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35093) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNPkV-0004XU-0j for qemu-devel@nongnu.org; Tue, 11 Mar 2014 12:41:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WNPkP-0004Q2-VY for qemu-devel@nongnu.org; Tue, 11 Mar 2014 12:41:50 -0400 Received: from lnantes-156-75-100-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:56035 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNPkP-0004Py-Pw for qemu-devel@nongnu.org; Tue, 11 Mar 2014 12:41:45 -0400 Date: Tue, 11 Mar 2014 17:41:45 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140311164145.GC3276@irqsave.net> References: <1394531919-14346-1-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1394531919-14346-1-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] block: Update image size in bdrv_invalidate_cache() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com, quintela@redhat.com The Tuesday 11 Mar 2014 =E0 10:58:39 (+0100), Kevin Wolf wrote : > After migration has completed, we call bdrv_invalidate_cache() so that > drivers which cache some data drop their stale copy of the data and > reread it from the image file to get a new version of data that the > source modified while the migration was running. >=20 > Reloading metadata from the image file is useless, though, if the size > of the image file stays stale (this is a value that is cached for all > image formats in block.c). Reads from (meta)data after the old EOF > return only zeroes, causing image corruption. >=20 > We need to update bs->total_sectors in all layers that could potentiall= y > have changed their size (i.e. backing files are not a concern - if they > are changed, we're in bigger trouble) >=20 > Signed-off-by: Kevin Wolf > --- > block.c | 10 +++++++++- > block/qcow2.c | 2 ++ > block/qed.c | 3 +++ > 3 files changed, 14 insertions(+), 1 deletion(-) >=20 > diff --git a/block.c b/block.c > index f1ef4b0..7b306fb 100644 > --- a/block.c > +++ b/block.c > @@ -4776,9 +4776,17 @@ flush_parent: > =20 > void bdrv_invalidate_cache(BlockDriverState *bs) > { > - if (bs->drv && bs->drv->bdrv_invalidate_cache) { > + if (!bs->drv) { > + return; > + } > + > + if (bs->drv->bdrv_invalidate_cache) { > bs->drv->bdrv_invalidate_cache(bs); > + } else if (bs->file) { > + bdrv_invalidate_cache(bs->file); > } > + > + refresh_total_sectors(bs, bs->total_sectors); > } > =20 > void bdrv_invalidate_cache_all(void) > diff --git a/block/qcow2.c b/block/qcow2.c > index cfe80be..b5b1e8c 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1176,6 +1176,8 @@ static void qcow2_invalidate_cache(BlockDriverSta= te *bs) > =20 > qcow2_close(bs); > =20 > + bdrv_invalidate_cache(bs->file); > + > options =3D qdict_new(); > qdict_put(options, QCOW2_OPT_LAZY_REFCOUNTS, > qbool_from_int(s->use_lazy_refcounts)); > diff --git a/block/qed.c b/block/qed.c > index 8802ad3..837accd 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -1563,6 +1563,9 @@ static void bdrv_qed_invalidate_cache(BlockDriver= State *bs) > BDRVQEDState *s =3D bs->opaque; > =20 > bdrv_qed_close(bs); > + > + bdrv_invalidate_cache(bs->file); > + > memset(s, 0, sizeof(BDRVQEDState)); > bdrv_qed_open(bs, NULL, bs->open_flags, NULL); > } > --=20 > 1.8.1.4 >=20 >=20 I have the impression that you are silently fixing other nits; However: Reviewed-by: Benoit Canet