[Qemu-devel] [PATCH v2 0/8] Obtain dirty bitmap via VM logging

[Qemu-devel] [PATCH v2 0/8] Obtain dirty bitmap via VM logging

[Sanidhya Kashyap]

Hi,

The following patches introduce the support of dirty bitmap logging and dumping
to a specified file. Still, some work is still left in the area of runstates that
I will try to work on after discussing this patch series.

v1 --> v2:
* Added two new run states to avoid simultaneous execution of both migration and
  bitmap dump process.
* Removed FILE pointer usage.
* Dumping the data only in machine-readable format.
* Tried to rectify mistakes of the previous version.



Sanidhya Kashyap (8):
  enable sharing of the function between migration and bitmap dump
  bitmap dump code via QAPI framework
  RunState: added two new flags for bitmap dump and migration process
  bitmap dump process with runstates
  hmp interface for dirty bitmap dump
  cancel mechanism for an already running dump bitmap process
  set the frequency of the dump bitmap process
  python script for extracting bitmap from a binary file

 arch_init.c               |  19 +--
 hmp-commands.hx           |  45 +++++++
 hmp.c                     |  33 ++++++
 hmp.h                     |   3 +
 include/exec/ram_addr.h   |   4 +
 migration.c               |   7 ++
 qapi-schema.json          |  42 ++++++-
 qmp-commands.hx           |  76 ++++++++++++
 savevm.c                  | 290 ++++++++++++++++++++++++++++++++++++++++++++++
 scripts/extract-bitmap.py |  64 ++++++++++
 vl.c                      |  29 ++++-
 11 files changed, 602 insertions(+), 10 deletions(-)
 create mode 100755 scripts/extract-bitmap.py

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/8] enable sharing of the function between migration and bitmap dump

[Sanidhya Kashyap]

As advised by Eric, I have enabled sharing of the function between of the
function that syncs the dirty bitmap obtained via kvm ioctl. I have tried
to make the least changes to the functions by concentrating only on the
function definitions.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 arch_init.c             | 19 +++++++++++--------
 include/exec/ram_addr.h |  4 ++++
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 685ba0e..48eb90a 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -434,20 +434,22 @@ ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
     return (next - base) << TARGET_PAGE_BITS;
 }
 
-static inline bool migration_bitmap_set_dirty(ram_addr_t addr)
+static inline bool bitmap_set_dirty(ram_addr_t addr, unsigned long *bitmap,
+                                                     bool migration_flag)
 {
     bool ret;
     int nr = addr >> TARGET_PAGE_BITS;
 
-    ret = test_and_set_bit(nr, migration_bitmap);
+    ret = test_and_set_bit(nr, bitmap);
 
-    if (!ret) {
+    if (!ret && migration_flag) {
         migration_dirty_pages++;
     }
     return ret;
 }
 
-static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
+void bitmap_sync_range(ram_addr_t start, ram_addr_t length,
+                              unsigned long *bitmap, bool migration_flag)
 {
     ram_addr_t addr;
     unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
@@ -461,8 +463,8 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
         for (k = page; k < page + nr; k++) {
             if (src[k]) {
                 unsigned long new_dirty;
-                new_dirty = ~migration_bitmap[k];
-                migration_bitmap[k] |= src[k];
+                new_dirty = ~bitmap[k];
+                bitmap[k] |= src[k];
                 new_dirty &= src[k];
                 migration_dirty_pages += ctpopl(new_dirty);
                 src[k] = 0;
@@ -476,7 +478,7 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
                 cpu_physical_memory_reset_dirty(start + addr,
                                                 TARGET_PAGE_SIZE,
                                                 DIRTY_MEMORY_MIGRATION);
-                migration_bitmap_set_dirty(start + addr);
+                bitmap_set_dirty(start + addr, bitmap, migration_flag);
             }
         }
     }
@@ -512,7 +514,8 @@ static void migration_bitmap_sync(void)
     address_space_sync_dirty_bitmap(&address_space_memory);
 
     QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-        migration_bitmap_sync_range(block->mr->ram_addr, block->length);
+        bitmap_sync_range(block->mr->ram_addr, block->length,
+                          migration_bitmap, true);
     }
     trace_migration_bitmap_sync_end(migration_dirty_pages
                                     - num_dirty_pages_init);
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 2edfa96..ca7d248 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -146,5 +146,9 @@ static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
 void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
                                      unsigned client);
 
+
+void bitmap_sync_range(ram_addr_t start, ram_addr_t length,
+                              unsigned long *bitmap, bool migration_flag);
+
 #endif
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/8] bitmap dump code via QAPI framework

[Sanidhya Kashyap]

Following are the changes made with respect to the previous version:
Chen's advice
1) Replaced DIRTY_MEMORY_LOG_BITMAP with DIRTY_MEMORY_MIGRATION and
completely removed the DIRTY_MEMORY_LOG_BITMAP flag.

Eric's advice
2) Replaced FILE pointer with file descriptor.
3) Replaced fopen/fclose with qemu_open / qemu_close.
4) Removed text format, output only in machine-readable format.
5) Defined constants.


Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 qapi-schema.json |  17 ++++
 qmp-commands.hx  |  33 ++++++++
 savevm.c         | 246 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 296 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 7bc33ea..17e5147 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4722,3 +4722,20 @@
               'btn'     : 'InputBtnEvent',
               'rel'     : 'InputMoveEvent',
               'abs'     : 'InputMoveEvent' } }
+##
+# @log-dirty-bitmap
+#
+# dumps the dirty bitmap to a file by logging the
+# memory for a specified number of times with a
+# a defined time differnce
+#
+# @filename: name of the file in which the bitmap will be saved.
+# @epochs: number of times the memory will be logged.
+# @frequency: time difference in milliseconds between each epoch.
+#
+# Since 2.1
+##
+{ 'command' : 'log-dirty-bitmap',
+  'data'    : { 'filename'      : 'str',
+                '*epochs'       : 'int',
+                '*frequency'    : 'int' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d8aa4ed..183a636 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3572,3 +3572,36 @@ Example:
                    } } ] }
 
 EQMP
+
+    {
+        .name       = "log-dirty-bitmap",
+        .args_type  = "filename:s,epochs:i?,frequency:i?,readable:-r?",
+        .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap,
+    },
+
+SQMP
+log-dirty-bitmap
+--------------------
+
+start logging the memory of the VM for writable working set
+
+Arguments:
+
+- "filename": name of the file, in which the bitmap will be saved
+- "epochs": number of times, the memory will be logged
+- "frequency": time difference in milliseconds between each epoch
+
+Examples:
+-> { "execute" : "log-dirty-bitmap",
+     "arguments" : {
+         "filename" : "/tmp/fileXXX",
+         "epochs" : 3,
+         "frequency" : 10 } }
+
+<- { "return": {} }
+
+Note: The epochs, frequency and readable are optional. epochs default
+value is 3 while that of frequency is 10.
+
+EQMP
+
diff --git a/savevm.c b/savevm.c
index da8aa24..525b388 100644
--- a/savevm.c
+++ b/savevm.c
@@ -41,6 +41,9 @@
 #include "qemu/iov.h"
 #include "block/snapshot.h"
 #include "block/qapi.h"
+#include "exec/address-spaces.h"
+#include "exec/ram_addr.h"
+#include "qemu/bitmap.h"
 
 #define SELF_ANNOUNCE_ROUNDS 5
 
@@ -1002,6 +1005,249 @@ void do_savevm(Monitor *mon, const QDict *qdict)
     }
 }
 
+/*
+ * Adding the functionality of continuous logging of the
+ * dirty bitmap which is almost similar to the migration
+ * thread
+ */
+
+enum {
+    LOG_BITMAP_STATE_ERROR = -1,
+    LOG_BITMAP_STATE_NONE,
+    LOG_BITMAP_STATE_SETUP,
+    LOG_BITMAP_STATE_ACTIVE,
+    LOG_BITMAP_STATE_CANCELING,
+    LOG_BITMAP_STATE_COMPLETED
+};
+
+typedef struct BitmapLogState BitmapLogState;
+static unsigned long *logging_bitmap;
+static int64_t MIN_EPOCH_VALUE = 3;
+static int64_t MIN_FREQUENCY_VALUE = 10;
+static int64_t LOG_SIZE_MAX = 100000;
+
+struct BitmapLogState {
+    int state;
+    int fd;
+    int64_t current_frequency;
+    int64_t total_epochs;
+    QemuThread thread;
+};
+
+/*
+ * helper functions
+ */
+
+static inline void logging_lock(void)
+{
+    qemu_mutex_lock_iothread();
+    qemu_mutex_lock_ramlist();
+}
+
+static inline void logging_unlock(void)
+{
+    qemu_mutex_unlock_ramlist();
+    qemu_mutex_unlock_iothread();
+}
+
+static inline void logging_bitmap_set_dirty(ram_addr_t addr)
+{
+    int nr  = addr >> TARGET_PAGE_BITS;
+    set_bit(nr, logging_bitmap);
+}
+
+static bool logging_state_set_status(BitmapLogState *b,
+                                     int old_state,
+                                     int new_state)
+{
+    return atomic_cmpxchg(&b->state, old_state, new_state);
+}
+
+static inline bool check_value(int64_t value, int64_t min_value,
+                               const char *str, Error **errp)
+{
+    if (value < min_value) {
+        error_setg(errp, "%s's value must be greater than %ld",
+                         str, min_value);
+        return false;
+    }
+    if (value > LOG_SIZE_MAX) {
+        error_setg(errp, "%s's value must be less than %ld",
+                         str, LOG_SIZE_MAX);
+        return false;
+    }
+    return true;
+}
+
+/*
+ * inspired from migration mechanism
+ */
+
+static BitmapLogState *logging_current_state(void)
+{
+    static BitmapLogState current_bitmaplogstate = {
+        .state = LOG_BITMAP_STATE_NONE,
+    };
+
+    return &current_bitmaplogstate;
+}
+
+/*
+ * syncing the logging_bitmap with the ram_list dirty bitmap
+ */
+
+static void dirty_bitmap_sync(void)
+{
+    RAMBlock *block;
+    address_space_sync_dirty_bitmap(&address_space_memory);
+    QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+        bitmap_sync_range(block->mr->ram_addr, block->length,
+                          logging_bitmap, false);
+    }
+}
+
+static inline void logging_bitmap_close(BitmapLogState *b)
+{
+    logging_lock();
+    memory_global_dirty_log_stop();
+    logging_unlock();
+
+    g_free(logging_bitmap);
+    qemu_close(b->fd);
+    logging_bitmap = NULL;
+    b = NULL;
+}
+
+static void *bitmap_logging_thread(void *opaque)
+{
+    /*
+     * setup basic structures
+     */
+
+    BitmapLogState *b = opaque;
+    int fd = b->fd;
+    int64_t epoch_count = 0;
+    int64_t total_epochs = b->total_epochs;
+    int64_t ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
+    size_t bitmap_size = BITS_TO_LONGS(ram_bitmap_pages) *
+                         sizeof(unsigned long);
+
+    logging_state_set_status(b, LOG_BITMAP_STATE_NONE,
+                                LOG_BITMAP_STATE_SETUP);
+
+    logging_bitmap = bitmap_new(ram_bitmap_pages);
+
+    if (logging_bitmap == NULL) {
+        b->state = LOG_BITMAP_STATE_ERROR;
+        goto log_thread_end;
+    }
+
+    logging_state_set_status(b, LOG_BITMAP_STATE_SETUP,
+                                LOG_BITMAP_STATE_ACTIVE);
+    /*
+     *  start the logging period
+     */
+    logging_lock();
+    memory_global_dirty_log_start();
+    dirty_bitmap_sync();
+    bitmap_zero(logging_bitmap, ram_bitmap_pages);
+    logging_unlock();
+
+    if (qemu_write_full(fd, &ram_bitmap_pages, sizeof(int64_t)) < 0) {
+        b->state = LOG_BITMAP_STATE_ERROR;
+        goto log_thread_end;
+    }
+
+    /*
+     * sync the dirty bitmap along with saving it
+     * using the FILE pointer f.
+     */
+    while (epoch_count < total_epochs) {
+        if (!runstate_is_running() || b->state != LOG_BITMAP_STATE_ACTIVE) {
+            goto log_thread_end;
+        }
+        bitmap_zero(logging_bitmap, ram_bitmap_pages);
+        logging_lock();
+        dirty_bitmap_sync();
+        logging_unlock();
+        if (qemu_write_full(fd, logging_bitmap, bitmap_size) < 0) {
+            b->state = LOG_BITMAP_STATE_ERROR;
+            goto log_thread_end;
+        }
+        g_usleep(b->current_frequency * 1000);
+        epoch_count++;
+    }
+
+    /*
+     * stop the logging period.
+     */
+ log_thread_end:
+    logging_bitmap_close(b);
+    if (b->state == LOG_BITMAP_STATE_ACTIVE) {
+        logging_state_set_status(b, LOG_BITMAP_STATE_ACTIVE,
+                                    LOG_BITMAP_STATE_COMPLETED);
+    } else if (b->state == LOG_BITMAP_STATE_CANCELING) {
+        logging_state_set_status(b, LOG_BITMAP_STATE_CANCELING,
+                                    LOG_BITMAP_STATE_COMPLETED);
+    } else if (b->state == LOG_BITMAP_STATE_ERROR) {
+        logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
+                                    LOG_BITMAP_STATE_COMPLETED);
+    }
+    return NULL;
+}
+
+void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
+                          int64_t epochs, bool has_frequency,
+                          int64_t frequency, Error **errp)
+{
+    int fd = -1;
+    BitmapLogState *b = logging_current_state();
+    Error *local_err = NULL;
+    if (b->state == LOG_BITMAP_STATE_ACTIVE ||
+            b->state == LOG_BITMAP_STATE_SETUP ||
+            b->state == LOG_BITMAP_STATE_CANCELING) {
+        b = NULL;
+        error_setg(errp, "dirty bitmap dump in progress");
+        return;
+    }
+
+    if (b->state == LOG_BITMAP_STATE_COMPLETED) {
+        b->state = LOG_BITMAP_STATE_NONE;
+    }
+
+    if (!has_epochs) {
+        epochs = MIN_EPOCH_VALUE;
+    }
+    if (!has_frequency) {
+        frequency = MIN_FREQUENCY_VALUE;
+    }
+
+    if (!check_value(epochs, MIN_EPOCH_VALUE, "epoch", &local_err) ||
+        !check_value(frequency, MIN_FREQUENCY_VALUE, "frequency", &local_err)) {
+        if (local_err) {
+            b = NULL;
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
+    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
+    if (fd < 0) {
+        error_setg_file_open(errp, errno, filename);
+        b = NULL;
+        return;
+    }
+
+    b->total_epochs = epochs;
+    b->current_frequency = frequency;
+    b->fd = fd;
+    qemu_thread_create(&b->thread, "dirty-bitmap-dump",
+                       bitmap_logging_thread, b,
+                       QEMU_THREAD_JOINABLE);
+
+    return;
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 3/8] RunState: added two new flags for bitmap dump and migration process

[Sanidhya Kashyap]

I have added two new flags - RUN_STATE_MIGRATE and RUN_STATE_DUMP_BITMAP.
These both flags behave same as RUN_STATE_RUNNING flag. The purpose of
introducing these flags is to avoid running both migration and dump bitmap
process simultaneously.

I haven't added many transitions to the RUN_STATE_DUMP_BITMAP. I will try
to include the transitions on the basis of discussions.

On the other hand, I have tried to add the transitions that might occur during
the migration process. There is a possibility that some transitions can be
redundant (as pointed by Chen, this is not my patch problem,  but I have tried
to cover what I thought is necessary).

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 qapi-schema.json |  7 ++++++-
 vl.c             | 29 ++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 17e5147..2918fc4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -234,12 +234,17 @@
 # @watchdog: the watchdog action is configured to pause and has been triggered
 #
 # @guest-panicked: guest has been panicked as a result of guest OS panic
+#
+# @migrate: migration process is being executed
+#
+# @dump-bitmap: dump the writable working set of the guest
+#
 ##
 { 'enum': 'RunState',
   'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
             'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
             'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
-            'guest-panicked' ] }
+            'guest-panicked', 'migrate', 'dump-bitmap' ] }
 
 ##
 # @SnapshotInfo
diff --git a/vl.c b/vl.c
index 709d8cd..a2ffd66 100644
--- a/vl.c
+++ b/vl.c
@@ -576,31 +576,39 @@ static const RunStateTransition runstate_transitions_def[] = {
     /*     from      ->     to      */
     { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
     { RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_DEBUG, RUN_STATE_MIGRATE },
 
     { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
 
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_MIGRATE },
 
     { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
     { RUN_STATE_IO_ERROR, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_IO_ERROR, RUN_STATE_MIGRATE },
 
     { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
     { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_PAUSED, RUN_STATE_MIGRATE },
 
     { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_POSTMIGRATE, RUN_STATE_MIGRATE },
 
     { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
     { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
     { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
+    { RUN_STATE_PRELAUNCH, RUN_STATE_MIGRATE },
 
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
 
     { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
 
+    { RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING},
+
     { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
     { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
     { RUN_STATE_RUNNING, RUN_STATE_IO_ERROR },
@@ -611,6 +619,8 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
     { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
     { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
+    { RUN_STATE_RUNNING, RUN_STATE_DUMP_BITMAP },
+    { RUN_STATE_RUNNING, RUN_STATE_MIGRATE },
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
@@ -621,12 +631,27 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
     { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
     { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_SUSPENDED, RUN_STATE_MIGRATE },
 
     { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
     { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_WATCHDOG, RUN_STATE_MIGRATE },
 
     { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
     { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_MIGRATE },
+
+    { RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING },
+
+    { RUN_STATE_MIGRATE, RUN_STATE_POSTMIGRATE },
+    { RUN_STATE_MIGRATE, RUN_STATE_PAUSED },
+    { RUN_STATE_MIGRATE, RUN_STATE_SHUTDOWN },
+    { RUN_STATE_MIGRATE, RUN_STATE_GUEST_PANICKED },
+    { RUN_STATE_MIGRATE, RUN_STATE_DEBUG },
+    { RUN_STATE_MIGRATE, RUN_STATE_RUNNING },
+    { RUN_STATE_MIGRATE, RUN_STATE_INTERNAL_ERROR },
+    { RUN_STATE_MIGRATE, RUN_STATE_IO_ERROR },
+    { RUN_STATE_MIGRATE, RUN_STATE_WATCHDOG },
 
     { RUN_STATE_MAX, RUN_STATE_MAX },
 };
@@ -666,7 +691,9 @@ void runstate_set(RunState new_state)
 
 int runstate_is_running(void)
 {
-    return runstate_check(RUN_STATE_RUNNING);
+    return (runstate_check(RUN_STATE_RUNNING) ||
+            runstate_check(RUN_STATE_MIGRATE) ||
+            runstate_check(RUN_STATE_DUMP_BITMAP));
 }
 
 bool runstate_needs_reset(void)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 4/8] bitmap dump process with runstates

[Sanidhya Kashyap]

Introduced both runstates: RUN_STATE_MIGRATE and RUN_STATE_DUMP_BITMAP to
both migration and bitmap dump process.

I want the bitmap dump process to get canceled so whenever the state changes
from RUN_STATE_BITMAP to something else. But, this does not happen when I stop
the guest via stop qmp interface as the current_run_state variable is not updated.
Any thoughts on that? Do I need to make the changes there as well or is there any
simple way to do it?

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 migration.c |  7 +++++++
 savevm.c    | 26 +++++++++++++++++++-------
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/migration.c b/migration.c
index 3fc03d6..d91dd4c 100644
--- a/migration.c
+++ b/migration.c
@@ -436,6 +436,13 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         return;
     }
 
+    if (runstate_check(RUN_STATE_DUMP_BITMAP)) {
+        error_setg(errp, "bitmap dump in progress");
+        return;
+    }
+
+    runstate_set(RUN_STATE_MIGRATE);
+
     s = migrate_init(&params);
 
     if (strstart(uri, "tcp:", &p)) {
diff --git a/savevm.c b/savevm.c
index 525b388..675c8e5 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1163,7 +1163,8 @@ static void *bitmap_logging_thread(void *opaque)
      * using the FILE pointer f.
      */
     while (epoch_count < total_epochs) {
-        if (!runstate_is_running() || b->state != LOG_BITMAP_STATE_ACTIVE) {
+        if (!runstate_check(RUN_STATE_DUMP_BITMAP) ||
+            b->state != LOG_BITMAP_STATE_ACTIVE) {
             goto log_thread_end;
         }
         bitmap_zero(logging_bitmap, ram_bitmap_pages);
@@ -1193,6 +1194,7 @@ static void *bitmap_logging_thread(void *opaque)
         logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
                                     LOG_BITMAP_STATE_COMPLETED);
     }
+    runstate_set(RUN_STATE_RUNNING);
     return NULL;
 }
 
@@ -1203,18 +1205,26 @@ void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
     int fd = -1;
     BitmapLogState *b = logging_current_state();
     Error *local_err = NULL;
-    if (b->state == LOG_BITMAP_STATE_ACTIVE ||
-            b->state == LOG_BITMAP_STATE_SETUP ||
-            b->state == LOG_BITMAP_STATE_CANCELING) {
+
+    if (runstate_check(RUN_STATE_DUMP_BITMAP) ||
+        b->state == LOG_BITMAP_STATE_ACTIVE ||
+        b->state == LOG_BITMAP_STATE_SETUP ||
+        b->state == LOG_BITMAP_STATE_CANCELING) {
         b = NULL;
         error_setg(errp, "dirty bitmap dump in progress");
         return;
     }
 
-    if (b->state == LOG_BITMAP_STATE_COMPLETED) {
-        b->state = LOG_BITMAP_STATE_NONE;
+    if (!runstate_is_running()) {
+        b = NULL;
+        error_setg(errp, "Guest is not in a running state");
+        return;
     }
 
+    runstate_set(RUN_STATE_DUMP_BITMAP);
+
+    b->state = LOG_BITMAP_STATE_NONE;
+
     if (!has_epochs) {
         epochs = MIN_EPOCH_VALUE;
     }
@@ -1227,14 +1237,16 @@ void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
         if (local_err) {
             b = NULL;
             error_propagate(errp, local_err);
+            runstate_set(RUN_STATE_RUNNING);
             return;
         }
     }
 
     fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
     if (fd < 0) {
-        error_setg_file_open(errp, errno, filename);
         b = NULL;
+        error_setg_file_open(errp, errno, filename);
+        runstate_set(RUN_STATE_RUNNING);
         return;
     }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 5/8] hmp interface for dirty bitmap dump

[Sanidhya Kashyap]

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx | 16 ++++++++++++++++
 hmp.c           | 16 ++++++++++++++++
 hmp.h           |  1 +
 3 files changed, 33 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 2e462c0..1665587 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1780,6 +1780,22 @@ STEXI
 show available trace events and their state
 ETEXI
 
+     {
+        .name       = "ldb|log-dirty-bitmap",
+        .args_type  = "filename:s,epochs:i?,frequency:i?",
+        .params     = "filename epochs frequency",
+        .help       = "dumps the memory's dirty bitmap to file\n\t\t\t"
+                     "filename: name of the file in which the bitmap will be saved\n\t\t\t"
+                      "epochs: number of times, the memory will be logged\n\t\t\t"
+                      "frequency: time difference in milliseconds between each epoch",
+        .mhandler.cmd = hmp_log_dirty_bitmap,
+    },
+STEXI
+@item ldb or log-dirty-bitmap @var{filename}
+@findex log-dirty-bitmap
+dumps the writable working set of a VM's memory to a file
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index ccc35d4..a400825 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1314,6 +1314,22 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &err);
 }
 
+void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict)
+{
+    const char *filename = qdict_get_str(qdict, "filename");
+    int64_t epochs = qdict_get_try_int(qdict, "epochs", 3);
+    int64_t frequency = qdict_get_try_int(qdict, "frequency", 10);
+    Error *err = NULL;
+
+    qmp_log_dirty_bitmap(filename, !!epochs, epochs, !!frequency,
+                         frequency, &err);
+    if (err) {
+        monitor_printf(mon, "log-dirty-bitmap: %s\n", error_get_pretty(err));
+        error_free(err);
+        return;
+    }
+}
+
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index aba59e9..3a79a93 100644
--- a/hmp.h
+++ b/hmp.h
@@ -93,6 +93,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict);
 void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 6/8] cancel mechanism for an already running dump bitmap process

[Sanidhya Kashyap]

No particular functional changes. Rectified some previous mistakes.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx  | 14 ++++++++++++++
 hmp.c            |  5 +++++
 hmp.h            |  1 +
 qapi-schema.json |  8 ++++++++
 qmp-commands.hx  | 20 ++++++++++++++++++++
 savevm.c         | 19 +++++++++++++++++++
 6 files changed, 67 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1665587..501e011 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1796,6 +1796,20 @@ STEXI
 dumps the writable working set of a VM's memory to a file
 ETEXI
 
+	{
+	.name       = "ldbc|log-dirty-bitmap-cancel",
+	.args_type  = "",
+	.params     = "",
+	.help       = "cancel the current bitmap dump process",
+	.mhandler.cmd = hmp_log_dirty_bitmap_cancel,
+},
+
+STEXI
+@item ldbc or log-dirty-bitmap-cancel
+@findex log-dirty-bitmap-cancel
+Cancel the current bitmap dump process
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index a400825..fed8795 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1330,6 +1330,11 @@ void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict)
     }
 }
 
+void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict)
+{
+    qmp_log_dirty_bitmap_cancel(NULL);
+}
+
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 3a79a93..b600429 100644
--- a/hmp.h
+++ b/hmp.h
@@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index 2918fc4..9f9f097 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4744,3 +4744,11 @@
   'data'    : { 'filename'      : 'str',
                 '*epochs'       : 'int',
                 '*frequency'    : 'int' } }
+##
+# @log-dirty-bitmap-cancel
+#
+# cancel the dirty bitmap logging process
+#
+# Since 2.1
+##
+{ 'command': 'log-dirty-bitmap-cancel' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 183a636..2a8dacc 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3605,3 +3605,23 @@ value is 3 while that of frequency is 10.
 
 EQMP
 
+	{
+        .name       = "log-dirty-bitmap-cancel",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_cancel,
+    },
+
+SQMP
+log_bitmap_cancel
+--------------
+
+Cancel the current bitmap dump process.
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "log-dirty-bitmap-cancel" }
+<- { "return": {} }
+
+EQMP
diff --git a/savevm.c b/savevm.c
index 675c8e5..ff87254 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1260,6 +1260,25 @@ void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
     return;
 }
 
+static void logging_bitmap_cancel(BitmapLogState *b)
+{
+    int old_state;
+    do {
+        old_state = b->state;
+        if (old_state != LOG_BITMAP_STATE_SETUP &&
+            old_state != LOG_BITMAP_STATE_ACTIVE) {
+            break;
+        }
+        logging_state_set_status(b, old_state,
+                                 LOG_BITMAP_STATE_CANCELING);
+    } while (b->state != LOG_BITMAP_STATE_CANCELING);
+}
+
+void qmp_log_dirty_bitmap_cancel(Error **errp)
+{
+    logging_bitmap_cancel(logging_current_state());
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 7/8] set the frequency of the dump bitmap process

[Sanidhya Kashyap]

No particular functional change. Corrected some mistakes.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx  | 15 +++++++++++++++
 hmp.c            | 12 ++++++++++++
 hmp.h            |  1 +
 qapi-schema.json | 10 ++++++++++
 qmp-commands.hx  | 23 +++++++++++++++++++++++
 savevm.c         | 13 +++++++++++++
 6 files changed, 74 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 501e011..ce0d9b5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1810,6 +1810,21 @@ STEXI
 Cancel the current bitmap dump process
 ETEXI
 
+    {
+        .name       = "ldbsf|log-dirty-bitmap-set-frequency",
+        .args_type  = "frequency:i",
+        .params     = "frequency",
+        .help       = "set the frequency for bitmap dump process\n\t\t\t"
+                      "frequency: the new frequency value to replace the existing",
+        .mhandler.cmd = hmp_log_dirty_bitmap_set_frequency,
+    },
+
+STEXI
+@item ldbsf or log-dirty-bitmap-set-frequency @var{frequency}
+@findex log-dirty-bitmap-set-frequency
+Set the frequency to @var{frequency} (int) for bitmap dump process.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index fed8795..8765093 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1335,6 +1335,18 @@ void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict)
     qmp_log_dirty_bitmap_cancel(NULL);
 }
 
+void hmp_log_dirty_bitmap_set_frequency(Monitor *mon, const QDict *qdict)
+{
+    int64_t frequency = qdict_get_int(qdict, "frequency");
+    Error *err = NULL;
+    qmp_log_dirty_bitmap_set_frequency(frequency, &err);
+    if (err) {
+        monitor_printf(mon, "log-dirty-bitmap-set-frequency: %s\n",
+                       error_get_pretty(err));
+        error_free(err);
+    }
+}
+
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index b600429..991be02 100644
--- a/hmp.h
+++ b/hmp.h
@@ -95,6 +95,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
 void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap_set_frequency(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index 9f9f097..7b7e4de 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4752,3 +4752,13 @@
 # Since 2.1
 ##
 { 'command': 'log-dirty-bitmap-cancel' }
+
+## @log-dirty-bitmap-set-frequency
+#
+# sets the frequency of the dirty bitmap logging process
+# @frequency: the updated frequency value
+#
+# Since 2.1
+##
+{ 'command': 'log-dirty-bitmap-set-frequency',
+  'data': {'frequency': 'int' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2a8dacc..51a0ad8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3625,3 +3625,26 @@ Example:
 <- { "return": {} }
 
 EQMP
+
+    {
+        .name       = "log-dirty-bitmap-set-frequency",
+        .args_type  = "frequency:i",
+        .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_set_frequency,
+    },
+
+SQMP
+log-dirty-bitmap-set-frequency
+--------------------
+
+Update the frequency for the remaining epochs.
+
+Arguments:
+
+- "frequency": the updated frequency (json-int)
+
+Example:
+
+-> { "execute": "log-dirty-bitmap-set-frequency", "arguments": { "value": 1024 } }
+<- { "return": {} }
+
+EQMP
diff --git a/savevm.c b/savevm.c
index ff87254..cfa8dce 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1279,6 +1279,19 @@ void qmp_log_dirty_bitmap_cancel(Error **errp)
     logging_bitmap_cancel(logging_current_state());
 }
 
+void qmp_log_dirty_bitmap_set_frequency(int64_t frequency, Error **errp)
+{
+    BitmapLogState *b = logging_current_state();
+    Error *local_err = NULL;
+    if (!check_value(frequency, MIN_FREQUENCY_VALUE, "frequency", &local_err)) {
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+    b->current_frequency = frequency;
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 8/8] python script for extracting bitmap from a binary file

[Sanidhya Kashyap]

No particular functional change. This file does not need to be included in
the Makefile as it will be only useful once the user has generated the bitmap
file via bitmap dump process.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 scripts/extract-bitmap.py | 64 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100755 scripts/extract-bitmap.py

diff --git a/scripts/extract-bitmap.py b/scripts/extract-bitmap.py
new file mode 100755
index 0000000..f5ca341
--- /dev/null
+++ b/scripts/extract-bitmap.py
@@ -0,0 +1,64 @@
+#!/usr/bin/python
+# This python script helps in extracting the dirty bitmap present
+# in the file after executing the log-dirty-bitmap command either
+# from the qmp or hmp interface. This file only processes binary
+# file obtained via command.
+#
+# Copyright (C) 2014 Sanidhya Kashyap <sanidhya.iiith@gmail.com>
+#
+# Authors:
+#       Sanidhya Kashyap
+#
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+
+import struct
+import argparse
+from functools import partial
+
+long_bytes = 8
+complete_bitmap_list = []
+
+def get_unsigned_long_integer(value):
+    return struct.unpack('<Q', value)[0]
+
+def get_long_integer(value):
+    return struct.unpack('<q',value)[0]
+
+def dump_bitmap(infile, bitmap_length):
+    count = 1
+    bitmap_list = []
+    for value in iter(partial(infile.read, long_bytes), ''):
+        if (count % bitmap_length):
+            count += 1
+            bitmap_list.append(hex(get_unsigned_long_integer(value)))
+        else:
+            complete_bitmap_list.append(bitmap_list)
+            count = 1
+            bitmap_list = []
+    # currently, the complete list is printed. It is up to the user to decide about
+    # the usage of the bitmap as the bitmap provides the writable working set of the
+    # VM for a particular duration.
+    print complete_bitmap_list
+
+def main():
+    extracter = argparse.ArgumentParser(description='Extract dirty bitmap from binary file.')
+    extracter.add_argument('infile', help='Input file to extract the bitmap')
+    args = extracter.parse_args()
+    print 'The filename is {}'.format(args.infile)
+
+    infile = open(format(args.infile), 'rb')
+
+    ram_bitmap_pages = get_long_integer(infile.read(long_bytes))
+    print ram_bitmap_pages
+    bitmap_length = ram_bitmap_pages / long_bytes
+    if ram_bitmap_pages % long_bytes != 0:
+        bitmap_length += 1
+    print bitmap_length
+
+    dump_bitmap(infile, bitmap_length);
+
+    infile.close()
+
+if __name__ == '__main__':
+    main()
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 1/8] enable sharing of the function between migration and bitmap dump

[Juan Quintela]

Sanidhya Kashyap <sanidhya.iiith@gmail.com> wrote:
> As advised by Eric, I have enabled sharing of the function between of the
> function that syncs the dirty bitmap obtained via kvm ioctl. I have tried
> to make the least changes to the functions by concentrating only on the
> function definitions.
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  arch_init.c             | 19 +++++++++++--------
>  include/exec/ram_addr.h |  4 ++++
>  2 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 685ba0e..48eb90a 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -434,20 +434,22 @@ ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
>      return (next - base) << TARGET_PAGE_BITS;
>  }
>  
> -static inline bool migration_bitmap_set_dirty(ram_addr_t addr)
> +static inline bool bitmap_set_dirty(ram_addr_t addr, unsigned long *bitmap,
> +                                                     bool migration_flag)

Exporting a function without a prefix could be dangerous.

Later, Juan.

Re: [Qemu-devel] [PATCH v2 3/8] RunState: added two new flags for bitmap dump and migration process

[Dr. David Alan Gilbert]

* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> I have added two new flags - RUN_STATE_MIGRATE and RUN_STATE_DUMP_BITMAP.
> These both flags behave same as RUN_STATE_RUNNING flag. The purpose of
> introducing these flags is to avoid running both migration and dump bitmap
> process simultaneously.
> 
> I haven't added many transitions to the RUN_STATE_DUMP_BITMAP. I will try
> to include the transitions on the basis of discussions.
> 
> On the other hand, I have tried to add the transitions that might occur during
> the migration process. There is a possibility that some transitions can be
> redundant (as pointed by Chen, this is not my patch problem,  but I have tried
> to cover what I thought is necessary).

I find runstate to be very subtle; there are probably other things that
would need changing; e.g. I see the USB code has:
   if (state == RUN_STATE_RUNNING) {
and
   if (!runstate_check(RUN_STATE_RUNNING)) {

(it should probably be using runstate_is_running() )
Also virt-test tends to see the state (I don't know if libvirt etc do?)
so they may get confused.

Dave


> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  qapi-schema.json |  7 ++++++-
>  vl.c             | 29 ++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 17e5147..2918fc4 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -234,12 +234,17 @@
>  # @watchdog: the watchdog action is configured to pause and has been triggered
>  #
>  # @guest-panicked: guest has been panicked as a result of guest OS panic
> +#
> +# @migrate: migration process is being executed
> +#
> +# @dump-bitmap: dump the writable working set of the guest
> +#
>  ##
>  { 'enum': 'RunState',
>    'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>              'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
>              'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
> -            'guest-panicked' ] }
> +            'guest-panicked', 'migrate', 'dump-bitmap' ] }
>  
>  ##
>  # @SnapshotInfo
> diff --git a/vl.c b/vl.c
> index 709d8cd..a2ffd66 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -576,31 +576,39 @@ static const RunStateTransition runstate_transitions_def[] = {
>      /*     from      ->     to      */
>      { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
>      { RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_DEBUG, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
>  
>      { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
>      { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
>      { RUN_STATE_IO_ERROR, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_IO_ERROR, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
>      { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_PAUSED, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_POSTMIGRATE, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
>      { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
>      { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
> +    { RUN_STATE_PRELAUNCH, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
>  
>      { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
>  
> +    { RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING},
> +
>      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
>      { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
>      { RUN_STATE_RUNNING, RUN_STATE_IO_ERROR },
> @@ -611,6 +619,8 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
>      { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
>      { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
> +    { RUN_STATE_RUNNING, RUN_STATE_DUMP_BITMAP },
> +    { RUN_STATE_RUNNING, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>  
> @@ -621,12 +631,27 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
>      { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
>      { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_SUSPENDED, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_WATCHDOG, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
>      { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_MIGRATE },
> +
> +    { RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING },
> +
> +    { RUN_STATE_MIGRATE, RUN_STATE_POSTMIGRATE },
> +    { RUN_STATE_MIGRATE, RUN_STATE_PAUSED },
> +    { RUN_STATE_MIGRATE, RUN_STATE_SHUTDOWN },
> +    { RUN_STATE_MIGRATE, RUN_STATE_GUEST_PANICKED },
> +    { RUN_STATE_MIGRATE, RUN_STATE_DEBUG },
> +    { RUN_STATE_MIGRATE, RUN_STATE_RUNNING },
> +    { RUN_STATE_MIGRATE, RUN_STATE_INTERNAL_ERROR },
> +    { RUN_STATE_MIGRATE, RUN_STATE_IO_ERROR },
> +    { RUN_STATE_MIGRATE, RUN_STATE_WATCHDOG },
>  
>      { RUN_STATE_MAX, RUN_STATE_MAX },
>  };
> @@ -666,7 +691,9 @@ void runstate_set(RunState new_state)
>  
>  int runstate_is_running(void)
>  {
> -    return runstate_check(RUN_STATE_RUNNING);
> +    return (runstate_check(RUN_STATE_RUNNING) ||
> +            runstate_check(RUN_STATE_MIGRATE) ||
> +            runstate_check(RUN_STATE_DUMP_BITMAP));
>  }
>  
>  bool runstate_needs_reset(void)
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 2/8] bitmap dump code via QAPI framework

[Dr. David Alan Gilbert]

* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> Following are the changes made with respect to the previous version:
> Chen's advice

> +    if (qemu_write_full(fd, &ram_bitmap_pages, sizeof(int64_t)) < 0) {
> +        b->state = LOG_BITMAP_STATE_ERROR;
> +        goto log_thread_end;
> +    }
> +
> +    /*
> +     * sync the dirty bitmap along with saving it
> +     * using the FILE pointer f.
> +     */
> +    while (epoch_count < total_epochs) {
> +        if (!runstate_is_running() || b->state != LOG_BITMAP_STATE_ACTIVE) {
> +            goto log_thread_end;
> +        }
> +        bitmap_zero(logging_bitmap, ram_bitmap_pages);
> +        logging_lock();
> +        dirty_bitmap_sync();
> +        logging_unlock();
> +        if (qemu_write_full(fd, logging_bitmap, bitmap_size) < 0) {
> +            b->state = LOG_BITMAP_STATE_ERROR;
> +            goto log_thread_end;
> +        }
> +        g_usleep(b->current_frequency * 1000);
> +        epoch_count++;
> +    }

I wonder about adding two extra things to the file format:
  1) The block names/length/offset information - so that you can tell
that bitmap entry 'n' is from main ram or from video ram.
  2) A marker word between/after each bitmap with a known value - it would
help spot any error where the wrong length is being read in the scripts;
otherwise it would be easy to get misaligned bitmaps without really noticing.

Dave

> +
> +    /*
> +     * stop the logging period.
> +     */
> + log_thread_end:
> +    logging_bitmap_close(b);
> +    if (b->state == LOG_BITMAP_STATE_ACTIVE) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_ACTIVE,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    } else if (b->state == LOG_BITMAP_STATE_CANCELING) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_CANCELING,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    } else if (b->state == LOG_BITMAP_STATE_ERROR) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    }
> +    return NULL;
> +}
> +
> +void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
> +                          int64_t epochs, bool has_frequency,
> +                          int64_t frequency, Error **errp)
> +{
> +    int fd = -1;
> +    BitmapLogState *b = logging_current_state();
> +    Error *local_err = NULL;
> +    if (b->state == LOG_BITMAP_STATE_ACTIVE ||
> +            b->state == LOG_BITMAP_STATE_SETUP ||
> +            b->state == LOG_BITMAP_STATE_CANCELING) {
> +        b = NULL;
> +        error_setg(errp, "dirty bitmap dump in progress");
> +        return;
> +    }
> +
> +    if (b->state == LOG_BITMAP_STATE_COMPLETED) {
> +        b->state = LOG_BITMAP_STATE_NONE;
> +    }
> +
> +    if (!has_epochs) {
> +        epochs = MIN_EPOCH_VALUE;
> +    }
> +    if (!has_frequency) {
> +        frequency = MIN_FREQUENCY_VALUE;
> +    }
> +
> +    if (!check_value(epochs, MIN_EPOCH_VALUE, "epoch", &local_err) ||
> +        !check_value(frequency, MIN_FREQUENCY_VALUE, "frequency", &local_err)) {
> +        if (local_err) {
> +            b = NULL;
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
> +    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
> +    if (fd < 0) {
> +        error_setg_file_open(errp, errno, filename);
> +        b = NULL;
> +        return;
> +    }
> +
> +    b->total_epochs = epochs;
> +    b->current_frequency = frequency;
> +    b->fd = fd;
> +    qemu_thread_create(&b->thread, "dirty-bitmap-dump",
> +                       bitmap_logging_thread, b,
> +                       QEMU_THREAD_JOINABLE);
> +
> +    return;
> +}
> +
>  void qmp_xen_save_devices_state(const char *filename, Error **errp)
>  {
>      QEMUFile *f;
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 1/8] enable sharing of the function between migration and bitmap dump

[Sanidhya Kashyap]

>
> Exporting a function without a prefix could be dangerous.
>
> Later, Juan.

Any particular solution, as the function is being used at two places.

-- 

Sanidhya Kashyap

Re: [Qemu-devel] [PATCH v2 2/8] bitmap dump code via QAPI framework

[Juan Quintela]

Sanidhya Kashyap <sanidhya.iiith@gmail.com> wrote:
> Following are the changes made with respect to the previous version:
> Chen's advice
> 1) Replaced DIRTY_MEMORY_LOG_BITMAP with DIRTY_MEMORY_MIGRATION and
> completely removed the DIRTY_MEMORY_LOG_BITMAP flag.
>
> Eric's advice
> 2) Replaced FILE pointer with file descriptor.
> 3) Replaced fopen/fclose with qemu_open / qemu_close.
> 4) Removed text format, output only in machine-readable format.
> 5) Defined constants.



> +
> +static inline bool check_value(int64_t value, int64_t min_value,
> +                               const char *str, Error **errp)

If you pass min_value, you should also pass max_value, no?

> +{
> +    if (value < min_value) {
> +        error_setg(errp, "%s's value must be greater than %ld",
> +                         str, min_value);
> +        return false;
> +    }
> +    if (value > LOG_SIZE_MAX) {
> +        error_setg(errp, "%s's value must be less than %ld",
> +                         str, LOG_SIZE_MAX);

You use it for more things that LOG, right?

Name of the function can be changed:

   value_in_range()?


> +    g_free(logging_bitmap);
> +    qemu_close(b->fd);
> +    logging_bitmap = NULL;

Change orders of this line with previous one?

> +    b = NULL;

Why?  b is a local variable.
Do you mean:

	b->fd = -1;?

> +     */
> + log_thread_end:
> +    logging_bitmap_close(b);
> +    if (b->state == LOG_BITMAP_STATE_ACTIVE) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_ACTIVE,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    } else if (b->state == LOG_BITMAP_STATE_CANCELING) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_CANCELING,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    } else if (b->state == LOG_BITMAP_STATE_ERROR) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    }

can you use a switch here, please?

> +{
> +    int fd = -1;
> +    BitmapLogState *b = logging_current_state();
> +    Error *local_err = NULL;
> +    if (b->state == LOG_BITMAP_STATE_ACTIVE ||
> +            b->state == LOG_BITMAP_STATE_SETUP ||
> +            b->state == LOG_BITMAP_STATE_CANCELING) {
> +        b = NULL;

Not needed.  This is repeated in more places.

> +    b->total_epochs = epochs;
> +    b->current_frequency = frequency;

I would have written it as:

	if (has_epochs) {
             b->total_epochs = epochs;
        } else {
             b->total_epochs IN_EPOCH_VALUE;
        }

The same with current frequency.

Thanks, Juan.

Re: [Qemu-devel] [PATCH v2 1/8] enable sharing of the function between migration and bitmap dump

[Juan Quintela]

Sanidhya Kashyap <sanidhya.iiith@gmail.com> wrote:
>>
>> Exporting a function without a prefix could be dangerous.
>>
>> Later, Juan.
>
> Any particular solution, as the function is being used at two places.


bitmap_* prefix is taken for generic bitmap operations.

qemu_bitmap_range_sync()

migration_bitmap_range_sync()

Use a prefix that is known to be used only on qemu.

I can undrestand why you don't want migration, so qemu_ prefix should do
the trick?


Later, Juan.

Re: [Qemu-devel] [PATCH v2 1/8] enable sharing of the function between migration and bitmap dump

[Sanidhya Kashyap]

> bitmap_* prefix is taken for generic bitmap operations.
>
> qemu_bitmap_range_sync()
>
> migration_bitmap_range_sync()
>
> Use a prefix that is known to be used only on qemu.
>
> I can undrestand why you don't want migration, so qemu_ prefix should do
> the trick?
>
>
> Later, Juan.


Got it!

-- 

Sanidhya Kashyap