From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:42732) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ROEJe-0004NF-2D for qemu-devel@nongnu.org; Wed, 09 Nov 2011 15:00:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ROEJa-0003lz-3y for qemu-devel@nongnu.org; Wed, 09 Nov 2011 15:00:10 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:60925) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ROEJZ-0003lk-PT for qemu-devel@nongnu.org; Wed, 09 Nov 2011 15:00:06 -0500 Received: by iakk32 with SMTP id k32so2375342iak.4 for ; Wed, 09 Nov 2011 12:00:05 -0800 (PST) Message-ID: <4EBADBC0.8000201@codemonkey.ws> Date: Wed, 09 Nov 2011 14:00:00 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <087d3dc42c667ea146edc73492b0f4afdd3a911d.1320865627.git.quintela@redhat.com> In-Reply-To: <087d3dc42c667ea146edc73492b0f4afdd3a911d.1320865627.git.quintela@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] Reopen files after migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org 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 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 {