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

[Anthony Liguori <anthony@codemonkey.ws>]

On 11/09/2011 01:16 PM, 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).
>
> 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)
>
> it is not necesary that source of migration close the file.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Couple thoughts:

1) Pretty sure this would break -snapshot.  I do test migration with -snapshot 
so please don't break it.

2) I don't think this is going to work very well with encrypted drives.

Perhaps we could do something like:

http://mid.gmane.org/1284213896-12705-2-git-send-email-aliguori@us.ibm.com

And do reopen as a default implementation.  That way we don't have to do reopen 
for formats that don't need it (raw) or can flush caches without reopening the 
file (qed).

It doesn't fix NFS close-to-open, but I think the right way to do that is to 
defer the open, not to reopen.

Regards,

Anthony Liguori

> ---
>   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 {