From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55128) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XcARY-0002vG-2m for qemu-devel@nongnu.org; Thu, 09 Oct 2014 05:55:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XcARS-0002CT-5J for qemu-devel@nongnu.org; Thu, 09 Oct 2014 05:55:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50856) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XcARR-0002BX-UZ for qemu-devel@nongnu.org; Thu, 09 Oct 2014 05:55:26 -0400 Message-ID: <54365B89.4020902@redhat.com> Date: Thu, 09 Oct 2014 11:55:21 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1412823046-29885-1-git-send-email-aik@ozlabs.ru> In-Reply-To: <1412823046-29885-1-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block/migration: Disable cache invalidate for incoming migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi Il 09/10/2014 04:50, Alexey Kardashevskiy ha scritto: > When migrated using libvirt with "--copy-storage-all", at the end of > migration there is race between NBD mirroring task trying to do flush > and migration completion, both end up invalidating cache. Since qcow2 > driver does not handle this situation very well, random crashes happen. > > This disables the BDRV_O_INCOMING flag for the block device being migrated > once the cache has been invalidated. > > Signed-off-by: Alexey Kardashevskiy > --- > Changes: > v1 (v0 was RFC): > * added a comment to nbd_export_new() as suggested by Stefan > --- > block.c | 18 +++++------------- > migration.c | 1 - > nbd.c | 6 ++++++ > 3 files changed, 11 insertions(+), 14 deletions(-) > > diff --git a/block.c b/block.c > index c5a251c..6314af7 100644 > --- a/block.c > +++ b/block.c > @@ -5048,6 +5048,11 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) > return; > } > > + if (!(bs->open_flags & BDRV_O_INCOMING)) { > + return; > + } > + bs->open_flags &= ~(BDRV_O_INCOMING); Unnecessary parentheses. > + > if (bs->drv->bdrv_invalidate_cache) { > bs->drv->bdrv_invalidate_cache(bs, &local_err); > } else if (bs->file) { > @@ -5083,19 +5088,6 @@ void bdrv_invalidate_cache_all(Error **errp) > } > } > > -void bdrv_clear_incoming_migration_all(void) > -{ > - BlockDriverState *bs; > - > - QTAILQ_FOREACH(bs, &bdrv_states, device_list) { > - AioContext *aio_context = bdrv_get_aio_context(bs); > - > - aio_context_acquire(aio_context); > - bs->open_flags = bs->open_flags & ~(BDRV_O_INCOMING); > - aio_context_release(aio_context); > - } > -} > - > int bdrv_flush(BlockDriverState *bs) > { > Coroutine *co; > diff --git a/migration.c b/migration.c > index 8d675b3..c49a05a 100644 > --- a/migration.c > +++ b/migration.c > @@ -103,7 +103,6 @@ static void process_incoming_migration_co(void *opaque) > } > qemu_announce_self(); > > - bdrv_clear_incoming_migration_all(); > /* Make sure all file formats flush their mutable metadata */ > bdrv_invalidate_cache_all(&local_err); > if (local_err) { > diff --git a/nbd.c b/nbd.c > index e9b539b..a7bce45 100644 > --- a/nbd.c > +++ b/nbd.c > @@ -972,6 +972,12 @@ 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); > + /* > + * 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. > + */ > + bdrv_invalidate_cache(bs, NULL); > return exp; > } > > Apart from the above style remark, Reviewed-by: Paolo Bonzini