[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH] block/migration: Disable cache invalidate f
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC PATCH] block/migration: Disable cache invalidate for incoming migration |
Date: |
Mon, 6 Oct 2014 11:03:36 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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.
pgpwVn0laDgS2.pgp
Description: PGP signature