Fix migration with NFS & iscsi/Fiber channel

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 <[email protected]>

---
 blockdev.c  |   42 +++++++++++++++++++++++++++++++++++++----- blockdev.h  |    6 ++++++
 migration.c |    6 ++++++

 3 files changed, 49 insertions(+), 5 deletions(-)

blockdev.c +37 -5

index f9bb659..3f3df7a 100644
@@ -116,6 +116,7 @@ void drive_uninit(DriveInfo *dinfo)
     bdrv_delete(dinfo->bdrv);
     QTAILQ_REMOVE(&drives, dinfo, next);
     qemu_free(dinfo->id);
+    qemu_free(dinfo->file);
     qemu_free(dinfo);
  }
 

@@ -136,6 +137,36 @@ 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 wth error %d\n",
+                           dinfo->file, res);
+                   return res;
+           }
+        }
+    }
+    return 0;
+}
+
  DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
  {
     const char *buf;

@@ -156,7 +187,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
     const char *devaddr;
     DriveInfo *dinfo;
     int snapshot = 0;
-    int ret;
 
     *fatal_error = 1;
 

@@ -487,10 +517,12 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
 
     bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
 
-    ret = bdrv_open(dinfo->bdrv, file, bdrv_flags, drv);
-    if (ret < 0) {
-        fprintf(stderr, "qemu: could not open disk image %s: %s\n",
-                        file, strerror(-ret));
+    dinfo->file = qemu_strdup(file);
+    dinfo->bdrv_flags = bdrv_flags;
+    dinfo->drv = drv;
+    dinfo->opened = 1;
+
+    if (drive_open(dinfo) < 0) {
         goto error;
     }
 

blockdev.h +4 -0

index 4536b5c..e009fee 100644
@@ -29,6 +29,10 @@ struct DriveInfo {
     QemuOpts *opts;
     char serial[BLOCK_SERIAL_STRLEN + 1];
     QTAILQ_ENTRY(DriveInfo) next;
+    int opened;
+    int bdrv_flags;
+    char *file;
+    BlockDriver *drv;
  };
 
  #define MAX_IDE_DEVS   2

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 <[email protected]>

---
 blockdev.c  |   42 +++++++++++++++++++++++++++++++++++++-----
 blockdev.h  |    6 ++++++
 migration.c |    6 ++++++
 3 files changed, 49 insertions(+), 5 deletions(-)

blockdev.c +21 -0

index f9bb659..3f3df7a 100644
@@ -116,6 +116,7 @@ void drive_uninit(DriveInfo *dinfo)
     bdrv_delete(dinfo->bdrv);
     QTAILQ_REMOVE(&drives, dinfo, next);
     qemu_free(dinfo->id);
+    qemu_free(dinfo->file);
     qemu_free(dinfo);
 }
 

@@ -136,6 +137,36 @@ 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);

blockdev.c +6 -5

 DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
 {
     const char *buf;

@@ -156,7 +187,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
     const char *devaddr;
     DriveInfo *dinfo;
     int snapshot = 0;
-    int ret;
 
     *fatal_error = 1;
 

@@ -487,10 +517,12 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
 
     bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
 
-    ret = bdrv_open(dinfo->bdrv, file, bdrv_flags, drv);
-    if (ret < 0) {
-        fprintf(stderr, "qemu: could not open disk image %s: %s\n",
-                        file, strerror(-ret));
+    dinfo->file = qemu_strdup(file);
+    dinfo->bdrv_flags = bdrv_flags;
+    dinfo->drv = drv;
+    dinfo->opened = 1;
+
+    if (drive_open(dinfo) < 0) {
         goto error;
     }
 

blockdev.h +0 -0

index 4536b5c..e009fee 100644

blockdev.h +2 -0

 };
 
 #define MAX_IDE_DEVS	2

@@ -42,6 +46,8 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
 QemuOpts *drive_add(const char *file, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
 
+extern int drives_reinit(void);
+
 /* device-hotplug */
 
 DriveInfo *add_init_drive(const char *opts);

migration.c +6 -0

index a8b65e5..fdff804 100644
@@ -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 "qemu-objects.h"

@@ -69,6 +70,11 @@ void process_incoming_migration(QEMUFile *f)
 
     incoming_expected = false;
 
+    if (drives_reinit() != 0) {
+        fprintf(stderr, "reopening of drives failed\n");
+        exit(1);
+    }
+
     if (autostart)
         vm_start();
 }

Linux allows to invalidate block devices.  This is needed for the incoming migration part.

Signed-off-by Juan Quintela <[email protected]>

---
 block.h           |    2 ++ block/raw-posix.c |   24 ++++++++++++++++++++++++ blockdev.c        |    9 +++++----

 3 files changed, 31 insertions(+), 4 deletions(-)

block.h +2 -0

index f923add..5ac96a5 100644
@@ -34,6 +34,8 @@ typedef struct QEMUSnapshotInfo {
  #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
  #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
  #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
+#define BDRV_O_INVALIDATE  0x0400 /* invalidate buffer cache for this device.
+                                     re-read things from server */
 
  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 

block/raw-posix.c +4 -0

index 6b72470..9439cf1 100644
@@ -51,6 +51,7 @@
  #include <sys/param.h>
  #include <linux/cdrom.h>
  #include <linux/fd.h>
+#include <linux/fs.h>
  #endif
  #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
  #include <signal.h>

@@ -168,6 +169,29 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
     s->fd = fd;
     s->aligned_buf = NULL;
 
+#ifdef __linux__
+    if ((bdrv_flags & BDRV_O_INVALIDATE)) {
+        struct stat buf;

Quoted 39 lines · Juan Quintela

Linux allows to invalidate block devices.  This is needed for the incoming migration part.

Signed-off-by Juan Quintela <[email protected]>

---
 block.h           |    2 ++ block/raw-posix.c |   24 ++++++++++++++++++++++++ blockdev.c        |    9 +++++----

 3 files changed, 31 insertions(+), 4 deletions(-)

block.h +2 -0

index f923add..5ac96a5 100644
@@ -34,6 +34,8 @@ typedef struct QEMUSnapshotInfo {
  #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
  #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
  #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
+#define BDRV_O_INVALIDATE  0x0400 /* invalidate buffer cache for this device.
+                                     re-read things from server */
 
  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 

block/raw-posix.c +1 -0

index 6b72470..9439cf1 100644
@@ -51,6 +51,7 @@
  #include <sys/param.h>
  #include <linux/cdrom.h>
  #include <linux/fd.h>
+#include <linux/fs.h>
  #endif
  #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
  #include <signal.h>

@@ -168,6 +169,29 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
     s->fd = fd;
     s->aligned_buf = NULL;
 

block/raw-posix.c static int raw_open_common(BlockDriverState *bs, const char *filename, · line 184

Quoted 13 lines

block/raw-posix.c +13 -0

+#ifdef __linux__
+    if ((bdrv_flags & BDRV_O_INVALIDATE)) {
+        struct stat buf;
+        int res;
+
+        res = fstat(fd, &buf);
+
+        if (res < 0) {
+            return -errno;
+        }
+
+        if (S_ISBLK(buf.st_mode)) {
+            printf("we are in a block device: %s\n", filename);

Leftover debugging?

Linux allows to invalidate block devices. This is needed for the incoming migration part.

Hi

This patch set creates infrastructure to invalidate buffers on migration target machine. The best way to see the problems is:

# create a new qcow2 image
qemu-img create -f qcow2 foo.img
# start the destination host
qemu .... path=foo.img....
# start the source host with one installation
qemu .... path=foo.img....
# migrate after lots of disk writes

Destination will have "read" the beggining of the blocks of the file (where the headers are). There are two bugs here:
a- we need to re-read the image after migration, to have the new values
(reopening fixes it)
b- we need to be sure that we read the new blocks that are on the server,
not the buffered ones locally from the start of the run.
NFS: flush on source and close + open on target invalidates the cache Block devices: on linux, BLKFLSBUF invalidates all the buffers for that device. This fixes iSCSI & FiberChannel.

I tested iSCSI & NFS. NFS patch have been on RHEL5 kvm forever (I just forget to send the patch upstream). Our NFS gurus & cluster gurus told that this is enough for linux to ensure consistence.

Once there, I fixed a couple of minor bugs (the first 3 patches):
- migration should exit with error 1 as everything else.
- memory leak on drive_uninit.
- fix cleanup on error on drive_init()

Later, Juan.

Juan Quintela (5):
migration: exit with error code
blockdev: don't leak id on removal
blockdev: release resources in the error case Reopen files after migration
drive_open: Add invalidate option for block devices

 block.h           |    2 ++
 block/raw-posix.c |   24 ++++++++++++++++++++++++
 blockdev.c        |   53 +++++++++++++++++++++++++++++++++++++++++++++--------
 blockdev.h        |    6 ++++++
 migration.c       |    8 +++++++-
 vl.c              |    2 +-
 6 files changed, 85 insertions(+), 10 deletions(-)