Re: [Qemu-devel] [PATCH 1/2] Reopen files after migration

[Anthony Liguori <anthony@codemonkey.ws>]

On 11/23/2011 09:34 AM, Juan Quintela wrote:
> We need to invalidate the Read Cache on the destination, otherwise we
> have corruption.  Easy way to reproduce it is:
>
> - create an qcow2 images
> - start qemu on destination of migration (qemu .... -incoming tcp:...)
> - start qemu on source of migration and do one install.
> - migrate at the end of install (when lot of disk IO has happened).

Have you actually tried this on the master?  It should work because of:

commit 06d9260ffa9dfa0e96e015501e43480ab66f96f6
Author: Anthony Liguori <aliguori@us.ibm.com>
Date:   Mon Nov 14 15:09:46 2011 -0600

     qcow2: implement bdrv_invalidate_cache (v2)

> Destination of migration has a local copy of the L1/L2 tables that existed
> at the beginning, before the install started.  We have disk corruption at
> this point.  The solution (for NFS) is to just re-open the file.  Operations
> have to happen in this order:
>
> - source of migration: flush()
> - destination: close(file);
> - destination: open(file)

You cannot reliably coordinate this with this series.  You never actually close 
the file on the source so I can't see how it would even work with this series.

I thought we had a long discussion on this and all agreed that opening O_DIRECT 
and fcntl()'ing it away was the best solution here?

Regards,

Anthony Liguori

>
> it is not necesary that source of migration close the file.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   blockdev.c  |   43 ++++++++++++++++++++++++++++++++++++++-----
>   blockdev.h  |    6 ++++++
>   migration.c |    6 ++++++
>   3 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 0827bf7..a10de7a 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -182,6 +182,7 @@ static void drive_uninit(DriveInfo *dinfo)
>       qemu_opts_del(dinfo->opts);
>       bdrv_delete(dinfo->bdrv);
>       g_free(dinfo->id);
> +    g_free(dinfo->file);
>       QTAILQ_REMOVE(&drives, dinfo, next);
>       g_free(dinfo);
>   }
> @@ -216,6 +217,37 @@ static int parse_block_error_action(const char *buf, int is_read)
>       }
>   }
>
> +static int drive_open(DriveInfo *dinfo)
> +{
> +    int res = bdrv_open(dinfo->bdrv, dinfo->file,
> +                        dinfo->bdrv_flags, dinfo->drv);
> +
> +    if (res<  0) {
> +        fprintf(stderr, "qemu: could not open disk image %s: %s\n",
> +                        dinfo->file, strerror(errno));
> +    }
> +    return res;
> +}
> +
> +int drives_reinit(void)
> +{
> +    DriveInfo *dinfo;
> +
> +    QTAILQ_FOREACH(dinfo,&drives, next) {
> +        if (dinfo->opened&&  !bdrv_is_read_only(dinfo->bdrv)) {
> +            int res;
> +            bdrv_close(dinfo->bdrv);
> +            res = drive_open(dinfo);
> +            if (res) {
> +                fprintf(stderr, "qemu: re-open of %s failed with error %d\n",
> +                        dinfo->file, res);
> +                return res;
> +            }
> +        }
> +    }
> +    return 0;
> +}
> +
>   DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>   {
>       const char *buf;
> @@ -236,7 +268,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>       const char *devaddr;
>       DriveInfo *dinfo;
>       int snapshot = 0;
> -    int ret;
>
>       translation = BIOS_ATA_TRANSLATION_AUTO;
>       media = MEDIA_DISK;
> @@ -514,10 +545,12 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>
>       bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
>
> -    ret = bdrv_open(dinfo->bdrv, file, bdrv_flags, drv);
> -    if (ret<  0) {
> -        error_report("could not open disk image %s: %s",
> -                     file, strerror(-ret));
> +    dinfo->file = g_strdup(file);
> +    dinfo->bdrv_flags = bdrv_flags;
> +    dinfo->drv = drv;
> +    dinfo->opened = 1;
> +
> +    if (drive_open(dinfo)<  0) {
>           goto err;
>       }
>
> diff --git a/blockdev.h b/blockdev.h
> index 3587786..733eb72 100644
> --- a/blockdev.h
> +++ b/blockdev.h
> @@ -38,6 +38,10 @@ struct DriveInfo {
>       char serial[BLOCK_SERIAL_STRLEN + 1];
>       QTAILQ_ENTRY(DriveInfo) next;
>       int refcount;
> +    int opened;
> +    int bdrv_flags;
> +    char *file;
> +    BlockDriver *drv;
>   };
>
>   DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
> @@ -53,6 +57,8 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
>                       const char *optstr);
>   DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi);
>
> +extern int drives_reinit(void);
> +
>   /* device-hotplug */
>
>   DriveInfo *add_init_drive(const char *opts);
> diff --git a/migration.c b/migration.c
> index 4b17566..764b233 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -17,6 +17,7 @@
>   #include "buffered_file.h"
>   #include "sysemu.h"
>   #include "block.h"
> +#include "blockdev.h"
>   #include "qemu_socket.h"
>   #include "block-migration.h"
>   #include "qmp-commands.h"
> @@ -89,6 +90,11 @@ void process_incoming_migration(QEMUFile *f)
>       qemu_announce_self();
>       DPRINTF("successfully loaded vm state\n");
>
> +    if (drives_reinit() != 0) {
> +        fprintf(stderr, "reopening of drives failed\n");
> +        exit(1);
> +    }
> +
>       if (autostart) {
>           vm_start();
>       } else {