From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53688) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xb58u-0000rf-Cs for qemu-devel@nongnu.org; Mon, 06 Oct 2014 06:03:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xb58o-0003Di-7L for qemu-devel@nongnu.org; Mon, 06 Oct 2014 06:03:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12816) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xb58o-0003DS-0D for qemu-devel@nongnu.org; Mon, 06 Oct 2014 06:03:42 -0400 Date: Mon, 6 Oct 2014 11:03:36 +0100 From: Stefan Hajnoczi Message-ID: <20141006100336.GE2694@stefanha-thinkpad.redhat.com> References: <1412239972-23493-1-git-send-email-aik@ozlabs.ru> <20141002145251.GD6250@stefanha-thinkpad.redhat.com> <542E2216.2040100@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="pY3vCvL1qV+PayAL" Content-Disposition: inline In-Reply-To: <542E2216.2040100@ozlabs.ru> Subject: Re: [Qemu-devel] [RFC PATCH] block/migration: Disable cache invalidate for incoming migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel@nongnu.org, Paolo Bonzini --pY3vCvL1qV+PayAL Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Oct 03, 2014 at 02:12:06PM +1000, Alexey Kardashevskiy wrote: > BDRV_O_INCOMING is only set when QEMU is about to receive migration and > we do not want QEMU to check the file at opening time as there is likely > garbage. Is there any other use of BDRV_O_INCOMING? There must be some as > bdrv_clear_incoming_migration_all() is called at the end of live > migration and I believe there must be a reason for it (cache does not > migrate, does it?). BDRV_O_INCOMING is just for live migration. The cached data is not migrated, this is why it must be refreshed upon migration handover. > bdrv_invalidate_cache() flushes cache as it could be already initialized > even if QEMU is receiving migration - QEMU could have cached some of real > disk data. Is that correct? I do not really understand why it would > happen if there is BDRV_O_INCOMING set but ok. .bdrv_open() can load metadata from image files (such as the qcow2 L1 tables) and it does this even when BDRV_O_INCOMING is set. That data needs to be re-read at migration handover to prevent the destination QEMU from running with stale image metadata. > diff --git a/nbd.c b/nbd.c > index e9b539b..953c378 100644 > - --- a/nbd.c > +++ b/nbd.c > @@ -972,6 +972,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t > dev_offset, > exp->ctx = bdrv_get_aio_context(bs); > bdrv_ref(bs); > bdrv_add_aio_context_notifier(bs, bs_aio_attached, bs_aio_detach, exp); > + bdrv_invalidate_cache(bs, NULL); > return exp; > } Please add a comment to explain why this call is necessary: /* NBD exports are used for non-shared storage migration. Make sure * that BDRV_O_INCOMING is cleared and the image is ready for write * access since the export could be available before migration handover. */ If someone can come up with a 2- or 3-line comment that explains it better, great. The rest of the patch looks like it will work. I'm not thrilled about putting BDRV_O_INCOMING logical inside bdrv_invalidate_cache() because there was no coupling there before, but the code seems correct now. --pY3vCvL1qV+PayAL Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUMmj4AAoJEJykq7OBq3PIwGUIAJIElCfkuoCmlcRriv75afe7 3r4AROwOt6uRAhQQn36lC2+crCkx0Y5n87QvM2tOd7QLvFcrR1xP9euK4TLnSfSH kxPCs2s0RbHQh6hFm1NQUoG3gQO0Eqy3sAQza4BPUWFW3gvYbRwy+LWKqw6Gaf3s CY181hmvJMmMjTOa6u9HbHSKpMSpO1u5gKXIVfR4y3me9e54yLpc0I7roMQne1Pu dXCCcFWjIHzObyYrjBeM3lFtRLDfnGMCignKPTBBVQPDAcRnPhdaCHs4GiEFKlWh g/ht7zgrHHfIysegaSYO8w95T3MQZMQHu/B1UUzASqtxu0+KU/82ksNezQNa5is= =cGO+ -----END PGP SIGNATURE----- --pY3vCvL1qV+PayAL--