From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43899) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTMIl-0006Um-V0 for qemu-devel@nongnu.org; Wed, 23 Nov 2011 18:32:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RTMId-0005et-Kn for qemu-devel@nongnu.org; Wed, 23 Nov 2011 18:32:27 -0500 Received: from mail-gx0-f173.google.com ([209.85.161.173]:43525) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTMId-0005eo-G5 for qemu-devel@nongnu.org; Wed, 23 Nov 2011 18:32:19 -0500 Received: by ggnb1 with SMTP id b1so2283380ggn.4 for ; Wed, 23 Nov 2011 15:32:19 -0800 (PST) Message-ID: <4ECD827F.7050700@codemonkey.ws> Date: Wed, 23 Nov 2011 17:32:15 -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: Andrzej Zaborowski , qemu-devel@nongnu.org, Blue Swirl , Michael Walle , "Edgar E. Iglesias" , Richard Henderson 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 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 > --- > 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 {