Re: [Qemu-devel] [RFC PATCH] block/migration: Disable cache invalidate for incoming migration

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]