Checkpoint-assisted migration proposal

From: Bohdan Trach <[email protected]>

dump-pc-mem command is added for checkpointing guest memory to file. Only system RAM region is saved. This checkpoint is later used to recover unchanged pages.

Signed-off-by Bohdan Trach <[email protected]>

---

(quoted diff) +2 -0

+##
+{ 'command': 'dump-pc-ram', 'data': {'file': 'str'} }

Quoted 10 lines · Bohdan Trach

From: Bohdan Trach <[email protected]>

dump-pc-mem command is added for checkpointing guest memory to file. Only system RAM region is saved. This checkpoint is later used to recover unchanged pages.

Signed-off-by Bohdan Trach <[email protected]>

---

arch_init.c static void migration_bitmap_sync(void) · line 610

Quoted 5 lines

arch_init.c +5 -0

+void qmp_dump_pc_ram(const char *file, Error **errp) {
+
+    int rc;
+    int fd;
+    fd = open(file,

Please use qemu_open() rather than raw open(), so that your command automatically supports /dev/fdset/nnn notation for reusing a file descriptor passed in via SCM_RIGHTS.

Quoted 4 lines

+++ b/qapi-schema.json

qapi-schema.json +0 -0

@@ -3648,3 +3648,14 @@
 # Since: 2.1
 ##

qapi-schema.json line 3655

Quoted 6 lines

qapi-schema.json +5 -0

 { 'command': 'rtc-reset-reinjection' }
+
+##
+# @dump-pc-ram:
+#
+# Checkpoints guest.

The whole guest, or just guest memory?

qapi-schema.json line 3659

Quoted 4 lines

qapi-schema.json +4 -0

+#
+# @file: the file to save the memory to as binary data
+#
+# Returns: Nothing on success

Missing a 'Since: 2.4' designation.

From: Bohdan Trach <[email protected]>

dump-pc-mem command is added for checkpointing guest memory to file. Only system RAM region is saved. This checkpoint is later used to recover unchanged pages.

Signed-off-by Bohdan Trach <[email protected]>

---
 arch_init.c      | 19 +++++++++++++++++++
 hmp-commands.hx  | 16 ++++++++++++++++
 hmp.c            |  9 +++++++++
 hmp.h            |  1 +
 qapi-schema.json | 11 +++++++++++
 qmp-commands.hx  | 19 +++++++++++++++++++
 6 files changed, 75 insertions(+)

arch_init.c +19 -0

index 4c8fcee..b8a4fb1 100644
@@ -603,6 +603,25 @@ static void migration_bitmap_sync(void)
     }
 }
 
+void qmp_dump_pc_ram(const char *file, Error **errp) {
+
+    int rc;
+    int fd;
+    fd = open(file,
+              O_CREAT|O_WRONLY,
+              S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+    assert(-1 != fd);
+
+    RAMBlock *block = QLIST_FIRST_RCU(&ram_list.blocks);
+    uint64_t offset;
+    for (offset=0; offset<ram_size; offset+=TARGET_PAGE_SIZE) {
+        rc = write(fd, block->host+offset, TARGET_PAGE_SIZE);
+        assert(TARGET_PAGE_SIZE == rc);
+    }
+    rc = close(fd);
+    assert(0 == rc);
+}
+
 /**
  * ram_save_page: Send the given page to the stream
  *

hmp-commands.hx +16 -0

index 3089533..0c47a4f 100644
@@ -1043,6 +1043,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
 ETEXI
 
     {
+        .name       = "dump-pc-ram",
+        .args_type  = "filename:F",
+        .params     = "filename",
+        .help       = "dump pc ram to file",
+        .mhandler.cmd = hmp_dump_pc_ram,
+    },
+
+
+STEXI
+@item dump-pc-ram
+@findex dump-guest-memory
+Dump pc ram to file.
+  filename: dump file name
+ETEXI
+
+    {
         .name       = "snapshot_blkdev",
         .args_type  = "reuse:-n,device:B,snapshot-file:s?,format:s?",
         .params     = "[-n] device [new-image-file] [format]",

hmp.c +9 -0

index f31ae27..5e27dd8 100644
@@ -1473,6 +1473,15 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
     g_free(prot);
 }
 
+void hmp_dump_pc_ram(Monitor *mon, const QDict *qdict)
+{
+    Error *errp = NULL;
+    const char *file = qdict_get_str(qdict, "filename");
+
+    qmp_dump_pc_ram(file, &errp);
+    hmp_handle_error(mon, &errp);
+}
+
 void hmp_netdev_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;

hmp.h +1 -0

index 2b9308b..805a71b 100644
@@ -79,6 +79,7 @@ void hmp_block_job_complete(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
+void hmp_dump_pc_ram(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
 void hmp_getfd(Monitor *mon, const QDict *qdict);

qapi-schema.json +11 -0

index ac9594d..338bfd3 100644
@@ -3648,3 +3648,14 @@
 # Since: 2.1
 ##
 { 'command': 'rtc-reset-reinjection' }
+
+##
+# @dump-pc-ram:
+#
+# Checkpoints guest.
+#
+# @file: the file to save the memory to as binary data
+#
+# Returns: Nothing on success
+##
+{ 'command': 'dump-pc-ram', 'data': {'file': 'str'} }

qmp-commands.hx +19 -0

index 3a42ad0..0be3c42 100644
@@ -854,6 +854,25 @@ Notes:
 EQMP
 
     {
+        .name       = "dump-pc-ram",
+        .args_type  = "file:s",
+        .params     = "file",
+        .help       = "dump pc ram to file",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = qmp_marshal_input_dump_pc_ram,
+    },
+
+SQMP
+dump
+
+
+Dump pc ram to file.
+
+Arguments:
+
+EQMP
+
+    {
         .name       = "query-dump-guest-memory-capability",
         .args_type  = "",
     .mhandler.cmd_new = qmp_marshal_input_query_dump_guest_memory_capability,

Signature · 1 line

2.0.5

Quoted 5 lines · * Bohdan Trach ([email protected]) wrote:

From: Bohdan Trach <[email protected]>

dump-pc-mem command is added for checkpointing guest memory to file. Only system RAM region is saved. This checkpoint is later used to recover unchanged pages.

Why not just use the 'dump_guest_memory' commands; they dump it in interesting existing formats; they have headers in the files as well rather than just a raw blob of data.
If you wanted to restrict to only certain RAM blocks, then I'd suggest adding a feature to that existing command.
You might also find that you want other RAMBlocks as well, for example where RAM is added using hot plu, those are separate RAM blocks.

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Hi David, thank you for the feedback!

Quoted 7 lines · * Bohdan Trach ([email protected]) wrote:

Quoted 5 lines

From: Bohdan Trach <[email protected]>

dump-pc-mem command is added for checkpointing guest memory to file. Only system RAM region is saved. This checkpoint is later used to recover unchanged pages.

Quoted 7 lines

Why not just use the 'dump_guest_memory' commands; they dump it in interesting existing formats; they have headers in the files as well rather than just a raw blob of data.
If you wanted to restrict to only certain RAM blocks, then I'd suggest adding a feature to that existing command.
You might also find that you want other RAMBlocks as well, for example where RAM is added using hot plu, those are separate RAM blocks.

We will try to rework these patches to use existing formats. Current format was used because it is extremely simple to work with. The restriction of saving only 'pc.ram' RAMBlock is just the consequence of this design choice.

Quoted 3 lines

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Signature · 2 lines

With best regards,
Bohdan Trach

From: Bohdan Trach <[email protected]>

This commit adds functions, which are used to work with checkpoint files. A new command-line option -checkpoint is added, which is used to specify the checkpoint file. Currently, MD5 function from OpenSSL is used to checkpoint memory.

Signed-off-by Bohdan Trach <[email protected]>

---
 arch_init.c     | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure       |   2 +
 qemu-options.hx |   9 ++++
 vl.c            |  12 +++++
 4 files changed, 172 insertions(+)

arch_init.c +27 -0

index b8a4fb1..eda86d4 100644
@@ -27,6 +27,7 @@
 #ifndef _WIN32
 #include <sys/types.h>
 #include <sys/mman.h>
+#include <openssl/md5.h>
 #endif
 #include "config.h"
 #include "monitor/monitor.h"

@@ -140,6 +141,8 @@ static struct defconfig_file {
 
 static const uint8_t ZERO_TARGET_PAGE[TARGET_PAGE_SIZE];
 
+int fd_checkpoint = -1;
+
 int qemu_read_default_config_files(bool userconfig)
 {
     int ret;

@@ -184,6 +187,30 @@ static void XBZRLE_cache_lock(void)
         qemu_mutex_lock(&XBZRLE.lock);
 }
 
+#ifdef DEBUG_ARCH_INIT
+static char* md5s(const uint8_t *digest) {
+    /* MD5 is 16 bytes, i.e., 32 hexadecimal digits. + 1 for trailing \0. */
+    static const size_t size = 32 + 1;
+    static char hex_digits[32+1];
+
+    /* snprintf(hex_digits, */
+    /*          MD5_DIGEST_LENGTH+1, */
+    /*          "%016lx", */
+    /*          *((uint64_t*)digest)); */
+    /* snprintf(hex_digits+MD5_DIGEST_LENGTH, */
+    /*          MD5_DIGEST_LENGTH+1, */
+    /*          "%016lx", */
+    /*          *((uint64_t*)(digest+sizeof(uint64_t)))); */
+    int digit;
+    for (digit = 0; digit < 32; digit += 2) {
+        snprintf(hex_digits+digit, 3, "%02x", digest[digit/2]);
+    }
+
+    hex_digits[size-1] = '\0';
+    return hex_digits;
+}
+#endif
+
 static void XBZRLE_cache_unlock(void)
 {
     if (migrate_use_xbzrle())

arch_init.c +12 -0

+/* indexed by page number */
+static uint64_t hashes_size = 0;
+static uint64_t hashes_entries = 0;
+static uint8_t *hashes = 0;
+
+static uint32_t get_page_nr(uint64_t addr) {
+    assert((addr % TARGET_PAGE_SIZE) == 0);
+    return (addr / TARGET_PAGE_SIZE);
+}
+
+typedef struct  {
+    uint8_t hash[MD5_DIGEST_LENGTH];

arch_init.c +19 -0

+    uint8_t all_zeroes[TARGET_PAGE_SIZE];
+    bzero(all_zeroes, TARGET_PAGE_SIZE);
+    MD5(all_zeroes, TARGET_PAGE_SIZE, all_zeroes_hash);
+
+    hash_offset_array = g_malloc(max_hash_offset_entries * sizeof(hash_offset_entry));
+    assert(0 != hash_offset_array);
+    bzero(hash_offset_array, max_hash_offset_entries * sizeof(hash_offset_entry));
+}
+
+void init_checksum_lookup_table(const char *checkpoint_path);
+void init_checksum_lookup_table(const char *checkpoint_path) {
+    int rc;
+    uint8_t* ram;
+    RAMBlock *block;
+
+    DPRINTF("ram_size=%lu\n", ram_size);
+
+    struct stat sb;
+    rc = stat(checkpoint_path, &sb);

arch_init.c +42 -0

+    fd_checkpoint = open(checkpoint_path, O_RDWR);
+    assert(fd_checkpoint != -1);
+
+    uint64_t idx;
+    for (idx=0; idx<ram_size; idx+=TARGET_PAGE_SIZE) {
+        rc = read(fd_checkpoint, ram+idx, TARGET_PAGE_SIZE);
+        assert(rc == TARGET_PAGE_SIZE);
+        assert(hash_offset_entries < max_hash_offset_entries);
+        MD5((unsigned char*)(ram+idx),
+            TARGET_PAGE_SIZE,
+            (unsigned char*)hash_offset_array[hash_offset_entries].hash);
+
+        hash_offset_array[hash_offset_entries].offset = idx;
+
+        DPRINTF("hash=%s offset=%08lx\n",
+                md5s(hash_offset_array[hash_offset_entries].hash),
+                hash_offset_array[hash_offset_entries].offset);
+
+        hash_offset_entries++;
+    };
+
+    qsort(hash_offset_array, hash_offset_entries, sizeof(hash_offset_entry),
+          cmp_hash_offset_entry);
+}
+
+int is_ram_addr(void* host);
+int is_ram_addr(void* host) {
+    static RAMBlock *block = NULL;
+
+    block = QLIST_FIRST_RCU(&ram_list.blocks);
+    assert(0 == strncmp(block->mr->name, "pc.ram", strlen("pc.ram")));
+
+    return (host >= memory_region_get_ram_ptr(block->mr) &&
+            host < memory_region_get_ram_ptr(block->mr) + block->used_length);
+}
+
+static int is_outgoing_with_checkpoint(void) {
+    return (fd_checkpoint != -1);
+}
+
 void qmp_dump_pc_ram(const char *file, Error **errp) {
 
     int rc;

@@ -869,6 +1016,8 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     bitmap_sync_count = 0;
     migration_bitmap_sync_init();
 
+    qsort(hashes, hashes_entries, MD5_DIGEST_LENGTH, uint128_compare);
+
     if (migrate_use_xbzrle()) {
         XBZRLE_cache_lock();
         XBZRLE.cache = cache_init(migrate_xbzrle_cache_size() /

configure +2 -0

index 6969f6f..fd6dd23 100755
@@ -337,6 +337,8 @@ vhdx=""
 quorum=""
 numa=""
 
+LIBS="-lcrypto"
+
 # parse CC options first
 for opt do
   optarg=`expr "x$opt" : 'x[^=]*=\(.*\)'`

qemu-options.hx +9 -0

index 319d971..ece4758 100644
@@ -268,6 +268,15 @@ If @var{slots} and @var{maxmem} are not specified, memory hotplug won't
 be enabled and the guest startup RAM will never increase.
 ETEXI
 
+DEF("checkpoint", HAS_ARG, QEMU_OPTION_checkpoint,
+    "-checkpoint file         path to checkpoint file\n", QEMU_ARCH_ALL)
+STEXI
+@item -checkpoint @var{path}
+@findex -checkpoint
+Checkpoint file to use during incoming migrations.
+Reduces network traffic and total migration time.
+ETEXI
+
 DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
     "-mem-path FILE  provide backing storage for guest RAM\n", QEMU_ARCH_ALL)
 STEXI

vl.c +12 -0

index 74c2681..d423e99 100644
@@ -134,6 +134,7 @@ int display_opengl;
 static int display_remote;
 const char* keyboard_layout = NULL;
 ram_addr_t ram_size;
+const char *checkpoint_path = NULL;
 const char *mem_path = NULL;
 int mem_prealloc = 0; /* force preallocation of physical target memory */
 bool enable_mlock = false;

@@ -2643,6 +2644,9 @@ out:
     return 0;
 }
 
+void init_checksum_lookup_table(const char *checkpoint_path);
+void veecycle_init(void);
+
 static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
 {
     uint64_t sz;

@@ -3116,6 +3120,9 @@ int main(int argc, char **argv, char **envp)
                 }
                 break;
 #endif
+            case QEMU_OPTION_checkpoint:
+                checkpoint_path = optarg;
+                break;
             case QEMU_OPTION_mempath:
                 mem_path = optarg;
                 break;

@@ -4331,6 +4338,7 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    veecycle_init();
     qdev_prop_check_globals();
     if (vmstate_dump_file) {
         /* dump and exit */

@@ -4339,6 +4347,10 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (incoming) {
+        if (checkpoint_path) {
+            init_checksum_lookup_table(checkpoint_path);
+        }
+
         Error *local_err = NULL;
         qemu_start_incoming_migration(incoming, &local_err);
         if (local_err) {

Signature · 1 line

2.0.5

Is anything in qemu/int128.h useful here?
However, as mentioned in my previous follow up,
I think you need something stronger than MD5 to stop collisions; sha256 seems appropriate and CPUs have acceleration instructions for it these days.

You seem to do this trick of declaring and then defining a lot; if you need it only within a file then make it static and then you don't need the declaration unless you use it before it's definition. If you want to use it outside of this file then the declaration should be in a header.

I guess this stuff should be in migration/ram.c these days?

Nice name; but if you're using a cute name make sure that you put a big comment to let people know what they're looking at!

This also makes it PC specific; what about everything else?

arch_init.c static void migration_bitmap_sync(void) · line 711

Quoted 2 lines

arch_init.c +2 -0

+    /* Ignore checkpoint file if size is different from VM's current memory size. */
+    assert(sb.st_size == ram_size);

Why does this matter? Can't you reuse the hash of pages that are at different locations in the stored file? e.g. a hash from an old/future boot of the same VM or one where the page got moved but unchanged?

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

From: Bohdan Trach <[email protected]>

Extend memory page saving and loading functions to utilize information available in checkpoints to avoid sending full pages over the network.

Signed-off-by Bohdan Trach <[email protected]>

---
 arch_init.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 158 insertions(+), 9 deletions(-)

arch_init.c +13 -6

index eda86d4..fca56f0 100644
@@ -128,6 +128,8 @@ static uint64_t bitmap_sync_count;
 #define RAM_SAVE_FLAG_CONTINUE 0x20
 #define RAM_SAVE_FLAG_XBZRLE   0x40
 /* 0x80 is reserved in migration.h start with 0x100 next */
+#define RAM_SAVE_FLAG_HASH     0x100
+#define RAM_SAVE_FLAG_PAGE_HASH     0x200
 
 static struct defconfig_file {
     const char *filename;

@@ -790,6 +792,7 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
     uint8_t *p;
     int ret;
     bool send_async = true;
+    uint8_t hash[MD5_DIGEST_LENGTH];
 
     p = memory_region_get_ram_ptr(mr) + offset;
 

@@ -841,16 +844,45 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
 
     /* XBZRLE overflow or normal page */
     if (pages == -1) {
-        *bytes_transferred += save_page_header(f, block,
-                                               offset | RAM_SAVE_FLAG_PAGE);
-        if (send_async) {
-            qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE);
-        } else {
-            qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
+        if (is_outgoing_with_checkpoint() &&
+            0 == strncmp(block->mr->name, "pc.ram", strlen("pc.ram"))) {
+            MD5(p, TARGET_PAGE_SIZE, hash);
+
+            if (NULL != bsearch(hash, hashes, hashes_entries,
+                                MD5_DIGEST_LENGTH, uint128_compare)) {
+
+                *bytes_transferred += save_page_header(f, block, offset | RAM_SAVE_FLAG_HASH);
+#ifdef DEBUG_HASH
+                qemu_put_buffer(f, p, TARGET_PAGE_SIZE);

arch_init.c +73 -3

+                *bytes_transferred += TARGET_PAGE_SIZE;
+                *bytes_transferred += MD5_DIGEST_LENGTH;
+                pages = 1;
+                DPRINTF("ram_save_page: FLAG_PAGE_HASH guest_phy_addr=%08lx flags=%lx hash=%s\n", offset&TARGET_PAGE_MASK, (offset | RAM_SAVE_FLAG_PAGE_HASH)& ~TARGET_PAGE_MASK, md5s(hash));
+            }
+        }
+        if (pages == -1) {
+            *bytes_transferred += save_page_header(f, block,
+                                                   offset | RAM_SAVE_FLAG_PAGE);
+            if (send_async) {
+                qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE);
+            } else {
+                qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
+            }
+            *bytes_transferred += TARGET_PAGE_SIZE;
+            pages = 1;
+            acct_info.norm_pages++;
+            DPRINTF("ram_save_page: FLAG_PAGE guest_phy_addr=%08lx flags=%lx", offset&TARGET_PAGE_MASK, (offset | RAM_SAVE_FLAG_PAGE)& ~TARGET_PAGE_MASK);
         }
-        *bytes_transferred += TARGET_PAGE_SIZE;
-        pages = 1;
-        acct_info.norm_pages++;
     }
 
     XBZRLE_cache_unlock();

@@ -963,6 +995,8 @@ void free_xbzrle_decoded_buf(void)
     xbzrle_decoded_buf = NULL;
 }
 
+extern const char *checkpoint_path;
+
 static void migration_end(void)
 {
     if (migration_bitmap) {

@@ -1281,6 +1315,58 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
     }
 }
 
+/**
+ * If migration source determined we already have the chunk, it only
+ * sends a hash of the page's content. Read it from local storage,
+ * e.g., an old checkpoint.
+ * @param host Address which, after this function, should have a content matching the functions 2nd parameter.
+ * @param hash The hash value.
+ * @param size Size of the memory region in bytes. Typically, size is a single page, e.g., 4 KiB.
+ * @param fd file descriptor of checkpoint file
+ */
+void ram_handle_hash(void *host, uint64_t guest_phy_addr, uint8_t *hash, uint64_t size);
+void ram_handle_hash(void *host, uint64_t guest_phy_addr, uint8_t *hash, uint64_t size)
+{
+    assert(fd_checkpoint != -1);
+
+    /* fprintf(stdout, "ram_handle_hash: incoming has %u!\n", hash); */
+    uint8_t local_page_hash[MD5_DIGEST_LENGTH];
+    MD5(host, TARGET_PAGE_SIZE, local_page_hash);
+
+    if (0 != memcmp(local_page_hash, hash, MD5_DIGEST_LENGTH)) {
+        /* Computed hash does not match the hash the migration source
+           sent us for this page. */
+        hash_offset_entry* v = bsearch(hash, hash_offset_array, hash_offset_entries,
+                                       sizeof(hash_offset_entry), cmp_hash_offset_entry);
+        if (v == NULL) {
+            /* For some reason the source thought the destination
+               already has this block. But it doesn't. Hmmm ... */
+            DPRINTF("ram_handle_hash: unknown hash %s at guest phy addr %08lx\n", md5s(hash), guest_phy_addr);
+            assert(0);
+        }
+
+        DPRINTF("ram_handle_hash: guest_phy_addr=%08lx, hash=%s, offset=%08lx\n", guest_phy_addr, md5s(hash), v->offset);
+
+        off_t offset_actual = lseek(fd_checkpoint, v->offset, SEEK_SET);
+        assert(offset_actual == v->offset);
+
+        ssize_t read_actual = read(fd_checkpoint, host, TARGET_PAGE_SIZE);
+        assert(read_actual == TARGET_PAGE_SIZE);
+        MD5(host, TARGET_PAGE_SIZE, local_page_hash);
+        if (0 != memcmp(local_page_hash, hash, MD5_DIGEST_LENGTH)) {
+            DPRINTF("ram_handle_hash: local_page_hash=%s\n", md5s(local_page_hash));
+            assert(0);
+        }
+    }
+}
+
+static void add_remote_hash(ram_addr_t addr, uint8_t *hash) {
+    uint64_t page_nr = get_page_nr(addr);
+    memcpy(&hashes[page_nr * MD5_DIGEST_LENGTH],
+           hash,
+           MD5_DIGEST_LENGTH);
+}
+
 static int ram_load(QEMUFile *f, void *opaque, int version_id)
 {
     int flags = 0, ret = 0;

@@ -1302,6 +1388,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
         ram_addr_t addr, total_ram_bytes;
         void *host;
         uint8_t ch;
+        uint8_t hash[MD5_DIGEST_LENGTH];
 
         addr = qemu_get_be64(f);
         flags = addr & ~TARGET_PAGE_MASK;

arch_init.c +61 -0

+            if (fd_checkpoint != -1) {
+                if (ch != 0) {
+                    MD5(host, TARGET_PAGE_SIZE, hash);
+                    add_remote_hash(addr, hash);
+                } else {
+                    add_remote_hash(addr, all_zeroes_hash);
+                }
+            }
+            break;
+        case RAM_SAVE_FLAG_HASH:
+            host = host_from_stream_offset(f, addr, flags);
+            if (!host) {
+                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
+                ret = -EINVAL;
+                break;
+            }
+
+#ifdef DEBUG_HASH
+            uint8_t src_page[TARGET_PAGE_SIZE];
+            qemu_get_buffer(f, src_page, TARGET_PAGE_SIZE);
+#endif
+            qemu_get_buffer(f, hash, MD5_DIGEST_LENGTH);
+
+            ram_handle_hash(host, addr, hash, TARGET_PAGE_SIZE);
+
+#ifdef DEBUG_HASH
+            uint8_t local_hash[MD5_DIGEST_LENGTH];
+            MD5(host, TARGET_PAGE_SIZE, local_hash);
+            assert(0 == memcmp(local_hash, hash, MD5_DIGEST_LENGTH));
+
+            uint8_t src_page_hash[MD5_DIGEST_LENGTH];
+            MD5(src_page, TARGET_PAGE_SIZE, src_page_hash);
+            assert(0 == memcmp(src_page_hash, hash, MD5_DIGEST_LENGTH));
+            assert(0 == memcmp(src_page, host, TARGET_PAGE_SIZE));
+#endif
+            assert(is_ram_addr(host));
+            add_remote_hash(addr, hash);
+            DPRINTF("ram_load: FLAG_HASH, recv_hash=%s, addr=%08lx\n", md5s(hash), addr);
+            break;
+        case RAM_SAVE_FLAG_PAGE_HASH:
+            host = host_from_stream_offset(f, addr, flags);
+            if (!host) {
+                error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
+                ret = -EINVAL;
+                break;
+            }
+
+            qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
+
+            qemu_get_buffer(f, hash, MD5_DIGEST_LENGTH);
+
+            assert(is_ram_addr(host));
+            add_remote_hash(addr, hash);
+            DPRINTF("ram_load: FLAG_PAGE_HASH, hash=%s, addr=%08lx\n", md5s(hash), addr);
             break;
         case RAM_SAVE_FLAG_PAGE:
             host = host_from_stream_offset(f, addr, flags);

@@ -1363,6 +1505,13 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                 break;
             }
             qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
+
+            if (is_ram_addr(host)) {
+                uint8_t hash[MD5_DIGEST_LENGTH];
+                MD5(host, TARGET_PAGE_SIZE, hash);
+                add_remote_hash(addr, hash);
+                DPRINTF("ram_load: FLAG_PAGE, addr=%08lx, hash=%s\n", addr, md5s(hash));
+            }
             break;
         case RAM_SAVE_FLAG_XBZRLE:
             host = host_from_stream_offset(f, addr, flags);

Signature · 1 line

2.0.5

There are a couple of things I don't understand about this:
1) How does the source fill it's hashes table? Is it just given the same dump file as the destination?
2) Why does RAM_SAVE_FLAG_PAGE_HASH exist; if you're sending the full page to the destination, why do we also send the hash?

I think there's a problem here that given the source is still running it's CPU and changing memory; it can be writing to the page at the same time, so the page you send might not match the hash you send; we're guaranteed to resend the page again if it was written to, but that still doesn't make these two things match; although as I say above I'm not sure why SAVE_FLAG_PAGE_HASH exists.

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Quoted 5 lines · Dr. David Alan Gilbert

There are a couple of things I don't understand about this:
1) How does the source fill it's hashes table? Is it just given the same dump file as the destination?
2) Why does RAM_SAVE_FLAG_PAGE_HASH exist; if you're sending the full page to the destination, why do we also send the hash?

1. Migration source is assumed to have the same dump file as the destination. The design was optimized for the case of ping-pong migrations over SAN, where checkpoint file is always available. We also have proof-of-concept code that transfers available hashes from the migration destination to the source over the network, but it didn't make it into these patches.

2. We send the hash to avoid hash calculations on the receiving side to save some CPU time. This flag can be removed, as I don't think the benefits it provides are big.

Quoted 5 lines

I think there's a problem here that given the source is still running it's CPU and changing memory; it can be writing to the page at the same time, so the page you send might not match the hash you send; we're guaranteed to resend the page again if it was written to, but that still doesn't make these two things match; although as I say above I'm not sure why SAVE_FLAG_PAGE_HASH exists.

This is true. In this case, we will just delete the SAVE_FLAG_PAGE_HASH flag.

Quoted 3 lines

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Signature · 2 lines

With best regards,
Bohdan Trach

Why is the hash needed on the destination; if it's a page which the source has decided isn't in a matching page, what does the destination use the hash for?

Quoted 7 lines

Quoted 5 lines

I think there's a problem here that given the source is still running it's CPU and changing memory; it can be writing to the page at the same time, so the page you send might not match the hash you send; we're guaranteed to resend the page again if it was written to, but that still doesn't make these two things match; although as I say above I'm not sure why SAVE_FLAG_PAGE_HASH exists.

This is true. In this case, we will just delete the SAVE_FLAG_PAGE_HASH flag.

But how do you know to delete the SAVE_FLAG_PAGE_HASH flag?

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Quoted 4 lines · Dr. David Alan Gilbert

Why is the hash needed on the destination; if it's a page which the source has decided isn't in a matching page, what does the destination use the hash for?

After the migration has finished, the hashes are still stored in RAM for the next migration, when the current destination becomes the new migration source. This way there is no need to recompute the checksums on the next migration -- they are already in RAM.

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

I think there's a problem here that given the source is still running it's CPU and changing memory; it can be writing to the page at the same time, so the page you send might not match the hash you send; we're guaranteed to resend the page again if it was written to, but that still doesn't make these two things match; although as I say above I'm not sure why SAVE_FLAG_PAGE_HASH exists.

Quoted 4 lines

Quoted 1 line

This is true. In this case, we will just delete the SAVE_FLAG_PAGE_HASH flag.

But how do you know to delete the SAVE_FLAG_PAGE_HASH flag?

Sorry for not stating this clear enough. We will remove this flag from the code, and send pages with SAVE_FLAG_PAGE instead. In this case the destination will compute the hash.

Quoted 4 lines

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Signature · 2 lines

With best regards,
Bohdan Trach

From: Bohdan Trach <[email protected]>

This patchset contains a checkpoint-assisted migration feature as proposed earlier on this list [1]. It allows reusing existing memory snapshots of guests to speed up migration of VMs between physical hosts.

[1] https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg01555.html

Bohdan Trach (3):
memory: Add dump-pc-mem command for checkpointing
memory: implement checkpoint handling
migration: use checkpoint during migration

Ping.

Quoted 1 line · Bohdan Trach

Ping.

It's taking a while, just because there are some other patches in the queue. I'll get to this soon.

Thanks for your patience.

Amit

Ping.

The patches are:
http://patchwork.ozlabs.org/patch/462043/
http://patchwork.ozlabs.org/patch/462040/
http://patchwork.ozlabs.org/patch/462045/

Description:
https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg01555.html
https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02014.html

Also, this code is being extended to also optionally use deduplication and dirty page tracking to save even more bandwidth.

From: Bohdan Trach <[email protected]>

This patchset contains a checkpoint-assisted migration feature as proposed earlier on this list [1]. It allows reusing existing memory snapshots of guests to speed up migration of VMs between physical hosts.

[1] https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg01555.html

From 'checkpointing', I was afraid this was going to use some

Could you please include a file in the docs/ directory that documents how this works, so it's easier to comment on the general idea?

From 'checkpointing', I was afraid this was going to use some checkpoint-restore framework, but instead it's a new checkpointing method that you're adding to qemu.

Can you describe when checkpoints are taken, and what is checkpointed? How is it stored on the disk?

I'm sure the patches have all the details, but it's easier to check the soundness of the idea if there's a high-level doc that explains this, and then we can discuss the finer points over patches.

Overall, I think this approach can benefit some workloads, and since it's not affecting a lot of common code, we could look at adding it.

Also, apologies for not getting to this earlier.

Hi Amit,

Quoted 2 lines · Amit Shah

Could you please include a file in the docs/ directory that documents how this works, so it's easier to comment on the general idea?

sure, we will add this.

Quoted 6 lines

From 'checkpointing', I was afraid this was going to use some checkpoint-restore framework, but instead it's a new checkpointing method that you're adding to qemu.

Can you describe when checkpoints are taken, and what is checkpointed? How is it stored on the disk?

Checkpoints are taken after a migration (at the source). If a checkpoint exists at the destination, the VM's state is reconstructed from the local checkpoint as well as updated pages from the source. This checkpoint-assisted migration can be faster, if network is the bottleneck, and saves network bandwidth.

We can, in principle, reuse the existing checkpoint format of QEMU. The current implementation writes its own checkpoint because it was less effort on our side. We write the VM's main memory into a single file.

Quoted 3 lines

I'm sure the patches have all the details, but it's easier to check the soundness of the idea if there's a high-level doc that explains this, and then we can discuss the finer points over patches.

We've recently published a paper about the general idea and expected benefits for a number of workloads (
http://se.inf.tu-dresden.de/pubs/papers/knauth2015vecycle.pdf )

Quoted 4 lines

Overall, I think this approach can benefit some workloads, and since it's not affecting a lot of common code, we could look at adding it.

Also, apologies for not getting to this earlier.

Kind regards,
Thomas.