| Message ID | 087d3dc42c667ea146edc73492b0f4afdd3a911d.1320865627.git.quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 {
Anthony Liguori <anthony@codemonkey.ws> wrote: > 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. Can you give me one example? I don't know how to use -snapshot with migration. > 2) I don't think this is going to work very well with encrypted drives. To be hones, no clue. > Perhaps we could do something like: > > http://mid.gmane.org/1284213896-12705-2-git-send-email-aliguori@us.ibm.com That is something like I wanted to know. > And do reopen as a default implementation. That way we don't have to > do reopen for formats that don't need it (raw) Kevin told me that know that we allow online resize, we should also update that for raw, but I haven't tested to be sure one way or another. > or can flush caches without reopening the file (qed). qcow2 could be told to flush caches, it is that the code is not there. It shouldn't be _that_ difficult. But I am not able to understand anymore block_open <-> block_file_open relationship. > 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. Fully agree here, that would be another way to fix it. See that in my other answer I showed that Markus already have problems with ide + cmos, so I think that we should have: - initialization done before we open files/block/<whatever you call it> - open files/block/... - late initialization that uses that (almost nothing needs to be here and should be easy to audit). About NFS, iSCSI, FC, my understanding is that if you use anything different than cache=none you are playing with fire, and will get burned sooner or later (it took quite a bit for Christoph to make me understand that, but now I fully agree with him). Later, Juan.
On 11/09/2011 03:10 PM, Juan Quintela wrote: > Anthony Liguori<anthony@codemonkey.ws> wrote: >> 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. > > Can you give me one example? I don't know how to use -snapshot with migration. This is totally unsafe but has always worked for me. On the same box: $ qemu -hda foo.img -snapshot $ qemu -hda foo.img -snapshot -incoming tcp:localhost:1025 This is not the *only* way I test migration but it's very convenient for sniff testing. The problem with your patch is that it assumes that once you've opened a file, the name still exists. But that is not universally true. It needs to degrade in a useful way. I think just deferring open is probably the best strategy. > >> 2) I don't think this is going to work very well with encrypted drives. > > To be hones, no clue. Deferring open addresses this is a nice way I think. >> Perhaps we could do something like: >> >> http://mid.gmane.org/1284213896-12705-2-git-send-email-aliguori@us.ibm.com > > That is something like I wanted to know. > >> And do reopen as a default implementation. That way we don't have to >> do reopen for formats that don't need it (raw) > > Kevin told me that know that we allow online resize, we should also > update that for raw, but I haven't tested to be sure one way or another. > >> or can flush caches without reopening the file (qed). > > qcow2 could be told to flush caches, it is that the code is not there. > It shouldn't be _that_ difficult. But I am not able to understand > anymore block_open<-> block_file_open relationship. > >> 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. > > Fully agree here, that would be another way to fix it. See that in my > other answer I showed that Markus already have problems with ide + cmos, > so I think that we should have: I've posted patches that delay the geometry guess until the device model is initialized. That avoids this particular problem. Regards, Anthony Liguori > > - initialization done before we open files/block/<whatever you call it> > - open files/block/... > - late initialization that uses that (almost nothing needs to be here > and should be easy to audit). > > About NFS, iSCSI, FC, my understanding is that if you use anything > different than cache=none you are playing with fire, and will get burned > sooner or later (it took quite a bit for Christoph to make me understand > that, but now I fully agree with him). > > Later, Juan.
On 11/09/2011 05: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) I've ran 2 test jobs, that run autotest stress test on a vm that is going through ping pong bg migration using 3 migration protocols, tcp, unix and exec: * qemu-kvm.git * qemu-kvm.git patched with this patch With your patch all tests PASS, with unpatched tree, all of them FAIL. So your solution improves the situation quite dramatically. > 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 {
Am 09.11.2011 22:16, schrieb Anthony Liguori: > On 11/09/2011 03:10 PM, Juan Quintela wrote: >> Anthony Liguori<anthony@codemonkey.ws> wrote: >>> 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. >> >> Can you give me one example? I don't know how to use -snapshot with migration. > > This is totally unsafe but has always worked for me. On the same box: > > $ qemu -hda foo.img -snapshot > > $ qemu -hda foo.img -snapshot -incoming tcp:localhost:1025 It's always amazing to see how people depend on insane things like this. :-) > This is not the *only* way I test migration but it's very convenient for sniff > testing. The problem with your patch is that it assumes that once you've opened > a file, the name still exists. But that is not universally true. It needs to > degrade in a useful way. > > I think just deferring open is probably the best strategy. One of the problems with deferring open was that we want to detect simple errors (typo in the file name, or whatever) before doing the real live migration. The proposed solution was a read-only open/close sequence at the start, but I believe this would break your use of -snapshot as well. Kevin
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 {
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 {
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> --- blockdev.c | 43 ++++++++++++++++++++++++++++++++++++++----- blockdev.h | 6 ++++++ migration.c | 6 ++++++ 3 files changed, 50 insertions(+), 5 deletions(-)