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

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

[Sanidhya Kashyap]

Hi,

This is the first part of my gsoc work.

Following patches add the support of dirty bitmap logging and dumping
to a specified file. These patches provide the mechanism to get the
dirty bitmap if the user wants to log the VM besides migration. This
can be useful in case of learning about the writable working set of
the VM even when it is not being migrated. I know that it will incur
huge degradation but still, a user might want to do that. Besides this,
this mechanism can also be useful in case of learning about the pages
that can be stored in the cache while performing the xbzrle based live
migration or even before the beginning of the hybrid (precopy + postcopy)
live migration.

I have tried to provide both of the hmp and qmp commands interface to
obtain the dirty bitmap. Besides dumping the dirty bitmap, one can
also update the frequency with which the logging is taking place or
even can cancel the ongoing logging process. Thus, there are 3 hmp and
qmp commands interface provided to perform the task.


Sanidhya Kashyap (6):
  split dirty bitmap into four for dumping the bitmaps
  bitmap dump code via QAPI framework
  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

 exec.c                    |   4 +
 hmp-commands.hx           |  46 +++++++
 hmp.c                     |  36 +++++
 hmp.h                     |   3 +
 include/exec/memory.h     |   3 +-
 include/exec/ram_addr.h   |  16 ++-
 include/qapi/qmp/qerror.h |   3 +
 qapi-schema.json          |  23 ++++
 qmp-commands.hx           |  77 +++++++++++
 savevm.c                  | 344 ++++++++++++++++++++++++++++++++++++++++++++++
 scripts/extract-bitmap.py |  68 +++++++++
 11 files changed, 621 insertions(+), 2 deletions(-)
 create mode 100755 scripts/extract-bitmap.py

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/6] split dirty bitmap into four for dumping the bitmaps

[Sanidhya Kashyap]

Added another flag - DIRTY_MEMORY_LOG_BITMAP for the purpose of logging
the dirty bitmap. The dumping bitmap process will utilize this flag for
dumping the data in the file.

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

diff --git a/exec.c b/exec.c
index 4e179a6..658196c 100644
--- a/exec.c
+++ b/exec.c
@@ -1551,6 +1551,7 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
     }
     cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_MIGRATION);
     cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_VGA);
+    cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_LOG_BITMAP);
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (!cpu_physical_memory_is_clean(ram_addr)) {
@@ -1959,6 +1960,7 @@ static void invalidate_and_set_dirty(hwaddr addr,
         /* set dirty bit */
         cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_VGA);
         cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_MIGRATION);
+        cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_LOG_BITMAP);
     }
     xen_modified_memory(addr, length);
 }
@@ -2563,6 +2565,8 @@ void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val)
                 cpu_physical_memory_set_dirty_flag(addr1,
                                                    DIRTY_MEMORY_MIGRATION);
                 cpu_physical_memory_set_dirty_flag(addr1, DIRTY_MEMORY_VGA);
+                cpu_physical_memory_set_dirty_flag(addr1,
+                                                   DIRTY_MEMORY_LOG_BITMAP);
             }
         }
     }
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1d55ad9..3f24563 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -19,7 +19,8 @@
 #define DIRTY_MEMORY_VGA       0
 #define DIRTY_MEMORY_CODE      1
 #define DIRTY_MEMORY_MIGRATION 2
-#define DIRTY_MEMORY_NUM       3        /* num of dirty bits */
+#define DIRTY_MEMORY_LOG_BITMAP 3
+#define DIRTY_MEMORY_NUM       4        /* num of dirty bits */
 
 #include <stdint.h>
 #include <stdbool.h>
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 2edfa96..f1ca86f 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -56,7 +56,10 @@ static inline bool cpu_physical_memory_is_clean(ram_addr_t addr)
     bool code = cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_CODE);
     bool migration =
         cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_MIGRATION);
-    return !(vga && code && migration);
+    bool bitmap_log =
+        cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_LOG_BITMAP);
+
+    return !(vga && code && migration && bitmap_log);
 }
 
 static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
@@ -66,6 +69,13 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
     set_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
 }
 
+static inline void cpu_physical_memory_clear_dirty_flag(ram_addr_t addr,
+                                                        unsigned client)
+{
+    assert(client < DIRTY_MEMORY_NUM);
+    clear_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
+}
+
 static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
                                                        ram_addr_t length)
 {
@@ -76,6 +86,8 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
     bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page);
     bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page);
     bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_CODE], page, end - page);
+    bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_LOG_BITMAP], page,
+                                                               end - page);
     xen_modified_memory(start, length);
 }
 
@@ -105,6 +117,8 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
                 ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION][page + k] |= temp;
                 ram_list.dirty_memory[DIRTY_MEMORY_VGA][page + k] |= temp;
                 ram_list.dirty_memory[DIRTY_MEMORY_CODE][page + k] |= temp;
+                ram_list.dirty_memory[DIRTY_MEMORY_LOG_BITMAP][page + k] |=
+                                                                        temp;
             }
         }
         xen_modified_memory(start, pages);
-- 
1.8.3.1

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

[Sanidhya Kashyap]

This patch introduces the mechanism of dumping the dirty bitmap either
in an ascii format or binary format. The implementation is almost
similar to the migration one. A separate thread is created for the
dumping process.

The bitmap is obtained with the help of ramlist blocks. I have used almost
similar functions that have been used in the migration mechanism (in
arch_init.c file). The mechanism to save the dirty bitmap is based on
RamBlock rather than MemoryRegion. After having a discussion with Juan, there
are still some issues with MemoryRegion like
* memory regions can overlap as well as
* no port of TCG to MemoryRegion.

Another point to be noted is that other instance of the bitmap dump process
will not execute if there is an already running process.

In this patch, I have introduced log-dirty-bitmap qmp interface to dump the
bitmap array.

The logging process has four variables:
1) filename - To dump the data in the file.
2) epochs - It is the number of times, the bitmaps will be logged and dumped
in the file. The default value is 3.
3) frequency - Time difference between each epochs. Currently, the default
value is 40ms.
4) readable - Its a boolean variable whose default value is false. That is
the data is save in human readable format. The first line is the total number
of pages followed by size of the bitmap and then the hex dump of the bitmap
array.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 include/qapi/qmp/qerror.h |   3 +
 qapi-schema.json          |  10 ++
 qmp-commands.hx           |  34 +++++
 savevm.c                  | 318 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 365 insertions(+)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 902d1a7..8b5b7b3 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -118,6 +118,9 @@ void qerror_report_err(Error *err);
 #define QERR_MIGRATION_ACTIVE \
     ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
 
+#define QERR_LOG_DIRTY_BITMAP_ACTIVE \
+    ERROR_CLASS_GENERIC_ERROR, "Dirty bitmap dump already in progress"
+
 #define QERR_MISSING_PARAMETER \
     ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 36cb964..c1dba64 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4700,3 +4700,13 @@
               'btn'     : 'InputBtnEvent',
               'rel'     : 'InputMoveEvent',
               'abs'     : 'InputMoveEvent' } }
+##
+# @log-dirty-bitmap
+#
+# provides the dirty bitmap for time t if specified.
+##
+{ 'command' : 'log-dirty-bitmap',
+  'data'    : { 'filename'      : 'str',
+                '*epochs'       : 'int',
+                '*frequency'    : 'int',
+                '*readable'     : 'bool' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cae890e..33e7719 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3569,3 +3569,37 @@ 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
+- "readable": dumps the bitmap in hex format (non-binary)
+
+Examples:
+-> { "execute" : "log-dirty-bitmap",
+     "arguments" : {
+         "filename" : "/tmp/fileXXX",
+         "epochs" : 100,
+         "frequency" : 100 } }
+
+<- { "return": {} }
+
+Note: The epochs, frequency and readable are optional. epochs default
+value is 3 while that of frequency is 40 and readable defaults to false.
+
+EQMP
+
diff --git a/savevm.c b/savevm.c
index da8aa24..25d399a 100644
--- a/savevm.c
+++ b/savevm.c
@@ -41,6 +41,10 @@
 #include "qemu/iov.h"
 #include "block/snapshot.h"
 #include "block/qapi.h"
+#include "exec/address-spaces.h"
+#include "exec/cpu-all.h"
+#include "exec/ram_addr.h"
+#include "qemu/bitmap.h"
 
 #define SELF_ANNOUNCE_ROUNDS 5
 
@@ -1002,6 +1006,320 @@ 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_CANCELLED,
+    LOG_BITMAP_STATE_CANCELING,
+    LOG_BITMAP_STATE_COMPLETED
+};
+
+typedef struct BitmapLogState BitmapLogState;
+static unsigned long *logging_bitmap;
+static int64_t LOG_SIZE_MAX = 100000;
+
+struct BitmapLogState {
+    int state;
+    int64_t current_frequency;
+    int64_t total_epochs;
+    bool readable;
+    QemuThread thread;
+    FILE *bitmap_dump_fp;
+};
+
+/*
+ * 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 void check_frequency_value(int64_t *frequency, Error **errp)
+{
+    if (*frequency < 10) {
+        error_setg(errp, "frequency should be greater "
+                        "than 10 milliseconds\n");
+        *frequency = 10;
+    }
+
+    if (*frequency > LOG_SIZE_MAX) {
+        error_setg(errp, "frequency greater than %ld (LOG_SIZE_MAX)\n",
+                   LOG_SIZE_MAX);
+        *frequency = LOG_SIZE_MAX;
+    }
+}
+
+static inline void check_epochs_value(int64_t *epochs, Error **errp)
+{
+    if (*epochs <= 0) {
+        error_setg(errp, "epoch size must be greater than 0, setting"
+                        " a value of 3\n");
+        *epochs = 3;
+    }
+
+    if (*epochs > LOG_SIZE_MAX) {
+        error_setg(errp, "epoch size greater than %ld (LOG_SIZE_MAX)\n",
+                   LOG_SIZE_MAX);
+        *epochs = LOG_SIZE_MAX;
+    }
+}
+
+static inline long sizeof_bitmap(long nbits)
+{
+    return BITS_TO_LONGS(nbits) * sizeof(unsigned long);
+}
+
+/*
+ * inspired from migration mechanism
+ */
+
+static BitmapLogState *logging_current_state(void)
+{
+    static BitmapLogState current_bitmaplogstate = {
+        .state = LOG_BITMAP_STATE_NONE,
+        .readable = false,
+    };
+
+    return &current_bitmaplogstate;
+}
+
+/*
+ * syncing the logging_bitmap with the ram_list dirty bitmap
+ */
+
+/*
+ * copied from arch_init.c file (below function)
+ */
+
+static void dirty_bitmap_logging_sync_range(ram_addr_t start,
+                                            ram_addr_t length)
+{
+    ram_addr_t addr;
+    unsigned long long page = BIT_WORD(start >> TARGET_PAGE_BITS);
+
+    if (((page * BITS_PER_LONG) << TARGET_PAGE_BITS) == start) {
+        int k;
+        int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS);
+        unsigned long *src = ram_list.dirty_memory[DIRTY_MEMORY_LOG_BITMAP];
+        for (k = page; k < page + nr; k++) {
+            if (src[k]) {
+                unsigned long new_dirty;
+                new_dirty = ~logging_bitmap[k];
+                logging_bitmap[k] |= src[k];
+                new_dirty &= src[k];
+                src[k] = 0;
+            }
+        }
+    } else {
+        for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
+            if (cpu_physical_memory_get_dirty(start + addr,
+                        TARGET_PAGE_SIZE,
+                        DIRTY_MEMORY_LOG_BITMAP)) {
+                cpu_physical_memory_reset_dirty(start + addr,
+                        TARGET_PAGE_SIZE,
+                        DIRTY_MEMORY_LOG_BITMAP);
+                logging_bitmap_set_dirty(start + addr);
+            }
+        }
+    }
+}
+
+static void dirty_bitmap_sync(void)
+{
+    RAMBlock *block;
+    address_space_sync_dirty_bitmap(&address_space_memory);
+    QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+        dirty_bitmap_logging_sync_range(block->mr->ram_addr, block->length);
+    }
+}
+
+static void write_file(FILE *dump_fp, int64_t ram_bitmap_pages, bool readable)
+{
+    int64_t i = 0;
+    int64_t bitmap_length = BITS_TO_LONGS(ram_bitmap_pages);
+
+    if (readable) {
+        for (; i < bitmap_length - 1; i++) {
+            fprintf(dump_fp, "%lx ", logging_bitmap[i]);
+        }
+        fprintf(dump_fp, "%lx\n", logging_bitmap[i]);
+    } else {
+        fwrite(logging_bitmap, sizeof(unsigned long), bitmap_length, dump_fp);
+    }
+}
+
+static inline void logging_bitmap_close(BitmapLogState *b)
+{
+    logging_lock();
+    memory_global_dirty_log_stop();
+    logging_unlock();
+
+    g_free(logging_bitmap);
+    fclose(b->bitmap_dump_fp);
+    b->bitmap_dump_fp = NULL;
+    logging_bitmap = NULL;
+    b = NULL;
+}
+
+static void *bitmap_logging_thread(void *opaque)
+{
+    /*
+     * setup basic structures
+     */
+
+    BitmapLogState *b = opaque;
+    int64_t curr_epoch = 0;
+    int64_t total_epochs = b->total_epochs;
+    FILE *dump_fp = b->bitmap_dump_fp;
+    bool readable = b->readable;
+    int64_t ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
+    int64_t bitmap_length = BITS_TO_LONGS(ram_bitmap_pages);
+
+    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 (readable) {
+        fprintf(dump_fp, "Total RAM pages: %ld\n"
+                         "Bitmap length: %ld\n",
+                         ram_bitmap_pages, bitmap_length);
+    } else {
+        fwrite(&ram_bitmap_pages, sizeof(int64_t), 1, dump_fp);
+        fwrite(&bitmap_length, sizeof(int64_t), 1, dump_fp);
+    }
+
+    /*
+     * sync the dirty bitmap along with saving it
+     * using the FILE pointer f.
+     */
+    while (curr_epoch < 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();
+        write_file(dump_fp, ram_bitmap_pages, readable);
+        g_usleep(b->current_frequency * 1000);
+        curr_epoch++;
+    }
+
+    /*
+     * 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_CANCELLED);
+    }
+    return NULL;
+}
+
+void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
+                          int64_t epochs, bool has_frequency,
+                          int64_t frequency, bool has_readable,
+                          bool readable, Error **errp)
+{
+    FILE *f;
+    BitmapLogState *b = logging_current_state();
+
+    if (b->state == LOG_BITMAP_STATE_ACTIVE ||
+            b->state == LOG_BITMAP_STATE_SETUP ||
+            b->state == LOG_BITMAP_STATE_CANCELING) {
+        error_set(errp, QERR_LOG_DIRTY_BITMAP_ACTIVE);
+        return;
+    }
+
+    if (b->state == LOG_BITMAP_STATE_COMPLETED) {
+        logging_state_set_status(b, LOG_BITMAP_STATE_COMPLETED,
+                                    LOG_BITMAP_STATE_NONE);
+    } else if (b->state == LOG_BITMAP_STATE_CANCELLED) {
+        logging_state_set_status(b, LOG_BITMAP_STATE_CANCELLED,
+                                    LOG_BITMAP_STATE_NONE);
+    }
+
+    if (readable) {
+        f = fopen(filename, "w");
+    } else {
+        f = fopen(filename, "wb");
+    }
+    if (!f) {
+        error_setg_file_open(errp, errno, filename);
+        goto no_bitmap_end;
+    }
+
+    if (!has_epochs) {
+        epochs = 3;
+    }
+    if (!has_frequency) {
+        frequency = 40;
+    }
+    check_epochs_value(&epochs, errp);
+    check_frequency_value(&frequency, errp);
+    b->total_epochs = epochs;
+    b->current_frequency = frequency;
+    b->bitmap_dump_fp = f;
+    b->readable = readable;
+    qemu_thread_create(&b->thread, "dirty-bitmap-logging",
+                       bitmap_logging_thread, b,
+                       QEMU_THREAD_JOINABLE);
+
+ no_bitmap_end:
+    return;
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;
-- 
1.8.3.1

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 6155 bytes --]

On 05/20/2014 11:47 AM, Sanidhya Kashyap wrote:
> This patch introduces the mechanism of dumping the dirty bitmap either
> in an ascii format or binary format. The implementation is almost
> similar to the migration one. A separate thread is created for the
> dumping process.
> 
> The bitmap is obtained with the help of ramlist blocks. I have used almost
> similar functions that have been used in the migration mechanism (in
> arch_init.c file). The mechanism to save the dirty bitmap is based on
> RamBlock rather than MemoryRegion. After having a discussion with Juan, there
> are still some issues with MemoryRegion like
> * memory regions can overlap as well as
> * no port of TCG to MemoryRegion.
> 

> 
> +++ b/include/qapi/qmp/qerror.h
> @@ -118,6 +118,9 @@ void qerror_report_err(Error *err);
>  #define QERR_MIGRATION_ACTIVE \
>      ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
>  
> +#define QERR_LOG_DIRTY_BITMAP_ACTIVE \
> +    ERROR_CLASS_GENERIC_ERROR, "Dirty bitmap dump already in progress"
> +

Please don't add new ERROR_CLASS macros.  Instead, just use error_setg()
and directly output the error message at the call site that produces the
error.

> +++ b/qapi-schema.json
> @@ -4700,3 +4700,13 @@
>                'btn'     : 'InputBtnEvent',
>                'rel'     : 'InputMoveEvent',
>                'abs'     : 'InputMoveEvent' } }
> +##
> +# @log-dirty-bitmap
> +#
> +# provides the dirty bitmap for time t if specified.

Too light on the specification.  You should document all 4 parameters,
and the default value for when the parameters are omitted.

'time t' isn't mentioned in any of the parameter names, so I'm assuming
the docs should mention its significance.

Missing a 'Since: 2.1' designation.

> +##
> +{ 'command' : 'log-dirty-bitmap',
> +  'data'    : { 'filename'      : 'str',
> +                '*epochs'       : 'int',
> +                '*frequency'    : 'int',
> +                '*readable'     : 'bool' } }

Seems okay.  As long as you use qemu_open(), it should also allow
management to pass in an fd with the add-fd mechanism.

> +
> +Arguments:
> +
> +- "filename": name of the file, in which the bitmap will be saved
> +- "epochs": number of times, the memory will be logged

s/times,/times/

> +- "frequency": time difference in milliseconds between each epoch
> +- "readable": dumps the bitmap in hex format (non-binary)

Possibly a poor choice of naming (binary output IS machine-readable; in
fact, programs prefer parsing binary output over decoding human-readable
text); maybe 'pretty' or 'text' would have been better; or even invert
the sense and have it be 'binary' and default to true.  I also wonder if
we even need it - I'd rather have the QMP command always output raw
binary, and then rely on post-processing to turn it into whatever format
the user wants to see (od works wonders), than to bloat qemu with having
to generate a human-readable variant.

> +
> +Examples:
> +-> { "execute" : "log-dirty-bitmap",
> +     "arguments" : {
> +         "filename" : "/tmp/fileXXX",
> +         "epochs" : 100,
> +         "frequency" : 100 } }
> +
> +<- { "return": {} }
> +
> +Note: The epochs, frequency and readable are optional. epochs default
> +value is 3 while that of frequency is 40 and readable defaults to false.

Mention the defaults alongside the description of each parameter, not in
a footnote.


> +static inline void check_epochs_value(int64_t *epochs, Error **errp)
> +{
> +    if (*epochs <= 0) {
> +        error_setg(errp, "epoch size must be greater than 0, setting"
> +                        " a value of 3\n");
> +        *epochs = 3;

This error reporting won't work.  Since this function returns void, if
errp is set at all, the caller must assume failure, rather than hoping
that you replaced the user's bad input with a saner default value.  If
it was worth reporting the error, then it is not worth trying to fix up
the value.

> +/*
> + * copied from arch_init.c file (below function)
> + */
> +
> +static void dirty_bitmap_logging_sync_range(ram_addr_t start,
> +                                            ram_addr_t length)
> +{

Rather than copying a function, can you make the original non-static and
share it between both callers?  Otherwise, the two will get out of sync
if a bug is fixed in one.

> +void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
> +                          int64_t epochs, bool has_frequency,
> +                          int64_t frequency, bool has_readable,
> +                          bool readable, Error **errp)
> +{
> +    FILE *f;
> +    BitmapLogState *b = logging_current_state();
> +
> +    if (b->state == LOG_BITMAP_STATE_ACTIVE ||
> +            b->state == LOG_BITMAP_STATE_SETUP ||
> +            b->state == LOG_BITMAP_STATE_CANCELING) {
> +        error_set(errp, QERR_LOG_DIRTY_BITMAP_ACTIVE);
> +        return;
> +    }
> +
> +    if (b->state == LOG_BITMAP_STATE_COMPLETED) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_COMPLETED,
> +                                    LOG_BITMAP_STATE_NONE);
> +    } else if (b->state == LOG_BITMAP_STATE_CANCELLED) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_CANCELLED,
> +                                    LOG_BITMAP_STATE_NONE);
> +    }
> +
> +    if (readable) {

readable is undefined if has_readable is false.  You are missing
something like:

if (!has_readable) {
    readable = false;
}

> +        f = fopen(filename, "w");
> +    } else {
> +        f = fopen(filename, "wb");

Ouch. You should be using qemu_open, not fopen, so that you can
automatically support things like /dev/fdset/NNN when coupled with the
'add-fd' command for passing open file descriptors.

> +    if (!has_epochs) {
> +        epochs = 3;
> +    }
> +    if (!has_frequency) {
> +        frequency = 40;

Magic numbers; use a named constant.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Sanidhya Kashyap]

>> +#define QERR_LOG_DIRTY_BITMAP_ACTIVE \
>> +    ERROR_CLASS_GENERIC_ERROR, "Dirty bitmap dump already in progress"
>> +
>
> Please don't add new ERROR_CLASS macros.  Instead, just use error_setg()
> and directly output the error message at the call site that produces the
> error.
>

will keep in mind in my next version of the patch.

>> +++ b/qapi-schema.json
>> @@ -4700,3 +4700,13 @@
>>                'btn'     : 'InputBtnEvent',
>>                'rel'     : 'InputMoveEvent',
>>                'abs'     : 'InputMoveEvent' } }
>> +##
>> +# @log-dirty-bitmap
>> +#
>> +# provides the dirty bitmap for time t if specified.
>
> Too light on the specification.  You should document all 4 parameters,
> and the default value for when the parameters are omitted.
>
> 'time t' isn't mentioned in any of the parameter names, so I'm assuming
> the docs should mention its significance.
>

ohh!, a bit messy statement, will rectify it.

> Missing a 'Since: 2.1' designation.
>
>> +##
>> +{ 'command' : 'log-dirty-bitmap',
>> +  'data'    : { 'filename'      : 'str',
>> +                '*epochs'       : 'int',
>> +                '*frequency'    : 'int',
>> +                '*readable'     : 'bool' } }
>
> Seems okay.  As long as you use qemu_open(), it should also allow
> management to pass in an fd with the add-fd mechanism.
>

haven't used qemu_open, will make appropriate changes.

>> +
>> +Arguments:
>> +
>> +- "filename": name of the file, in which the bitmap will be saved
>> +- "epochs": number of times, the memory will be logged
>
> s/times,/times/
>
>> +- "frequency": time difference in milliseconds between each epoch
>> +- "readable": dumps the bitmap in hex format (non-binary)
>
> Possibly a poor choice of naming (binary output IS machine-readable; in
> fact, programs prefer parsing binary output over decoding human-readable
> text); maybe 'pretty' or 'text' would have been better; or even invert
> the sense and have it be 'binary' and default to true.  I also wonder if
> we even need it - I'd rather have the QMP command always output raw
> binary, and then rely on post-processing to turn it into whatever format
> the user wants to see (od works wonders), than to bloat qemu with having
> to generate a human-readable variant.
>

will remove the readable (poor naming convention) part from the code and will
only dump the data in machine-readable format.

>
>> +static inline void check_epochs_value(int64_t *epochs, Error **errp)
>> +{
>> +    if (*epochs <= 0) {
>> +        error_setg(errp, "epoch size must be greater than 0, setting"
>> +                        " a value of 3\n");
>> +        *epochs = 3;
>
> This error reporting won't work.  Since this function returns void, if
> errp is set at all, the caller must assume failure, rather than hoping
> that you replaced the user's bad input with a saner default value.  If
> it was worth reporting the error, then it is not worth trying to fix up
> the value.
>

will correct it.

>> +/*
>> + * copied from arch_init.c file (below function)
>> + */
>> +
>> +static void dirty_bitmap_logging_sync_range(ram_addr_t start,
>> +                                            ram_addr_t length)
>> +{
>
> Rather than copying a function, can you make the original non-static and
> share it between both callers?  Otherwise, the two will get out of sync
> if a bug is fixed in one.
>

will have to make certain changes in both files in order for the file
to be generic. will do that.


>> +    if (readable) {
>
> readable is undefined if has_readable is false.  You are missing
> something like:
>
> if (!has_readable) {
>     readable = false;
> }

missed it. will keep this in mind.


>> +    if (!has_epochs) {
>> +        epochs = 3;
>> +    }
>> +    if (!has_frequency) {
>> +        frequency = 40;
>
> Magic numbers; use a named constant.
>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

Thanks for the quick response. Will address all the above issues in my
second version of the patch.

--

Sanidhya

[Qemu-devel] [PATCH 3/6] hmp interface for dirty bitmap dump

[Sanidhya Kashyap]

Added the log-dirty-bitmap or ldb hmp interface.

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8971f1b..032a320 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1774,6 +1774,23 @@ STEXI
 show available trace events and their state
 ETEXI
 
+     {
+        .name       = "ldb|log-dirty-bitmap",
+        .args_type  = "readable:-r,filename:s,epochs:i?,frequency:i?",
+        .params     = "[-r] 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\n\t\t\t"
+                      "-r: dumps the bitmap in hex format (non-binary)",
+        .mhandler.cmd = hmp_log_dirty_bitmap,
+    },
+STEXI
+@item ldb or log-dirty-bitmap [-r] @var{filename}
+@findex log-dirty-bitmap
+umps the writable working set of the VM's memoryto a file
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index 5c4d612..cc54988 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1309,6 +1309,25 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &err);
 }
 
+void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict)
+{
+    printf("called\n");
+    int readable = qdict_get_try_bool(qdict, "readable", 0);
+    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", 40);
+    Error *err = NULL;
+
+    printf("%s %ld %ld %d\n", filename, epochs, frequency, readable);
+    qmp_log_dirty_bitmap(filename, !!epochs, epochs, !!frequency,
+                        frequency, !!readable, readable, &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 20ef454..bf6cc2a 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 4/6] cancel mechanism for an already running dump bitmap process

[Sanidhya Kashyap]

This patch introduces both hmp and qmp (log-dirty-bitmap-cancel) interface
to cancel an already executing dump process.

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 032a320..a8e13b3 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1791,6 +1791,20 @@ STEXI
 umps the writable working set of the VM's memoryto a file
 ETEXI
 
+	{
+	.name       = "ldbc|log-dirty-bitmap-cancel",
+	.args_type  = "",
+	.params     = "",
+	.help       = "cancel the current VM logging",
+	.mhandler.cmd = hmp_log_dirty_bitmap_cancel,
+},
+
+STEXI
+@item ldbc or log-dirty-bitmap-cancel
+@findex log-dirty-bitmap-cancel
+Cancel the current VM migration.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index cc54988..7bbbaad 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1328,6 +1328,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 bf6cc2a..d98b2ae 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 c1dba64..e897adb 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4710,3 +4710,8 @@
                 '*epochs'       : 'int',
                 '*frequency'    : 'int',
                 '*readable'     : 'bool' } }
+## @log-dirty-bitmap-cancel
+#
+# cancel the dirty bitmap logging process
+##
+{ 'command': 'log-dirty-bitmap-cancel' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 33e7719..55b2251 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3603,3 +3603,23 @@ value is 3 while that of frequency is 40 and readable defaults to false.
 
 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 logging of the dirty bitmap.
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "log-bitmap-cancel" }
+<- { "return": {} }
+
+EQMP
diff --git a/savevm.c b/savevm.c
index 25d399a..1859f57 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1320,6 +1320,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

Re: [Qemu-devel] [PATCH 4/6] cancel mechanism for an already running dump bitmap process

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

On 05/20/2014 11:47 AM, Sanidhya Kashyap wrote:
> This patch introduces both hmp and qmp (log-dirty-bitmap-cancel) interface
> to cancel an already executing dump process.
> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---

> +++ b/qapi-schema.json
> @@ -4710,3 +4710,8 @@
>                  '*epochs'       : 'int',
>                  '*frequency'    : 'int',
>                  '*readable'     : 'bool' } }
> +## @log-dirty-bitmap-cancel
> +#
> +# cancel the dirty bitmap logging process
> +##

Missing 'Since: 2.1'; also, the formatting doesn't quite match other
commands where ## is on a line by itself at the start.

> +{ 'command': 'log-dirty-bitmap-cancel' }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 33e7719..55b2251 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3603,3 +3603,23 @@ value is 3 while that of frequency is 40 and readable defaults to false.
>  
>  EQMP
>  
> +	{
> +        .name       = "log-dirty-bitmap-cancel",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_cancel,
> +    },
> +
> +SQMP
> +log_bitmap_cancel

Incorrect command name.

> +--------------
> +
> +Cancel the current logging of the dirty bitmap.
> +
> +Arguments: None.
> +
> +Example:
> +
> +-> { "execute": "log-bitmap-cancel" }

And yet another spelling of an incorrect command name.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 5/6] set the frequency of the dump bitmap process

[Sanidhya Kashyap]

This patch introduces both qmp and hmp (log-dirty-bitmap-set-frequency)
interface to update the value of the frequency.

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index a8e13b3..38c1b9d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1805,6 +1805,21 @@ STEXI
 Cancel the current VM migration.
 ETEXI
 
+    {
+        .name       = "ldbsf|log-dirty-bitmap-set-frequency",
+        .args_type  = "frequency:i",
+        .params     = "frequency",
+        .help       = "set the frequency for memory logging\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 memory logging.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index 7bbbaad..297db3d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1333,6 +1333,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 d98b2ae..21d5e88 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 e897adb..ccef4d8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4715,3 +4715,11 @@
 # cancel the dirty bitmap logging process
 ##
 { 'command': 'log-dirty-bitmap-cancel' }
+
+## @log-dirty-bitmap-set-frequency
+#
+# sets the frequency of the dirty bitmap logging process
+##
+{ 'command': 'log-dirty-bitmap-set-frequency',
+  'data': {'frequency': 'int' } }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 55b2251..fe12d05 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3623,3 +3623,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 1859f57..7551891 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1339,6 +1339,13 @@ 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();
+    check_frequency_value(&frequency, errp);
+    b->current_frequency = frequency;
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 5/6] set the frequency of the dump bitmap process

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On 05/20/2014 11:47 AM, Sanidhya Kashyap wrote:
> This patch introduces both qmp and hmp (log-dirty-bitmap-set-frequency)
> interface to update the value of the frequency.
> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---

> +++ b/qapi-schema.json
> @@ -4715,3 +4715,11 @@
>  # cancel the dirty bitmap logging process
>  ##
>  { 'command': 'log-dirty-bitmap-cancel' }
> +
> +## @log-dirty-bitmap-set-frequency
> +#
> +# sets the frequency of the dirty bitmap logging process
> +##

Same comments as in other patches about missing documentation.

> +{ 'command': 'log-dirty-bitmap-set-frequency',
> +  'data': {'frequency': 'int' } }
> +
> diff --git a/qmp-commands.hx b/qmp-commands.hx

Please don't end files with an extra blank line.

> +Arguments:
> +
> +- "frequency": the updated frequency (json-int)

In what unit? I'm assuming milliseconds based on the rest of the series,
but spelling it out will help readers.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Sanidhya Kashyap]

This script extracts the bitmap only from a binary file. It takes only
a single input as file. This will be useful to users for exploring the
writable working set for the VM.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 scripts/extract-bitmap.py | 68 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 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..64ce136
--- /dev/null
+++ b/scripts/extract-bitmap.py
@@ -0,0 +1,68 @@
+#!/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) Sanidhya Kashyap <sanidhya.iiith@gmail.com>
+#
+# Authors:
+#       Sanidhya Kashyap
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, see <http://www.gnu.org/licenses/>.
+#
+
+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 = get_long_integer(infile.read(long_bytes))
+    print bitmap_length
+
+    dump_bitmap(infile, bitmap_length);
+
+    infile.close()
+
+if __name__ == '__main__':
+    main()
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 6/6] python script for extracting bitmap from a binary file

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

On 05/20/2014 11:47 AM, Sanidhya Kashyap wrote:
> This script extracts the bitmap only from a binary file. It takes only
> a single input as file. This will be useful to users for exploring the
> writable working set for the VM.
> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  scripts/extract-bitmap.py | 68 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 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..64ce136
> --- /dev/null
> +++ b/scripts/extract-bitmap.py
> @@ -0,0 +1,68 @@
> +#!/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) Sanidhya Kashyap <sanidhya.iiith@gmail.com>

Worth listing 2014?

> +#
> +# Authors:
> +#       Sanidhya Kashyap
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License along
> +# with this program; if not, see <http://www.gnu.org/licenses/>.

You refer to the GPL, but didn't explicitly state what license this file
is under.  Did you mean for it to be GPLv2+?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 6/6] python script for extracting bitmap from a binary file

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

On 05/20/2014 01:38 PM, Eric Blake wrote:
> On 05/20/2014 11:47 AM, Sanidhya Kashyap wrote:
>> This script extracts the bitmap only from a binary file. It takes only
>> a single input as file. This will be useful to users for exploring the
>> writable working set for the VM.
>>
>> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
>> ---
>>  scripts/extract-bitmap.py | 68 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>  create mode 100755 scripts/extract-bitmap.py

Does this file need to be mentioned in any Makefile to ensure that it
gets distributed as part of the qemu tarball?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 6/6] python script for extracting bitmap from a binary file

[Sanidhya Kashyap]

>>> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
>>> ---
>>>  scripts/extract-bitmap.py | 68 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 68 insertions(+)
>>>  create mode 100755 scripts/extract-bitmap.py
>
> Does this file need to be mentioned in any Makefile to ensure that it
> gets distributed as part of the qemu tarball?
>

No, the file does not need to be mentioned in nay Makefile.

I will add all the required changes in the next version.

> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

-- 
Sanidhya

Re: [Qemu-devel] [PATCH 0/6] Obtain dirty bitmap via VM logging

[ChenLiang]

Hi,
Nice job. We should avoid running migration_thread and bitmap_logging_thread simultaneously.

Best regards:
ChenLiang

> Hi,
> 
> This is the first part of my gsoc work.
> 
> Following patches add the support of dirty bitmap logging and dumping
> to a specified file. These patches provide the mechanism to get the
> dirty bitmap if the user wants to log the VM besides migration. This
> can be useful in case of learning about the writable working set of
> the VM even when it is not being migrated. I know that it will incur
> huge degradation but still, a user might want to do that. Besides this,
> this mechanism can also be useful in case of learning about the pages
> that can be stored in the cache while performing the xbzrle based live
> migration or even before the beginning of the hybrid (precopy + postcopy)
> live migration.
> 
> I have tried to provide both of the hmp and qmp commands interface to
> obtain the dirty bitmap. Besides dumping the dirty bitmap, one can
> also update the frequency with which the logging is taking place or
> even can cancel the ongoing logging process. Thus, there are 3 hmp and
> qmp commands interface provided to perform the task.
> 
> 
> Sanidhya Kashyap (6):
>   split dirty bitmap into four for dumping the bitmaps
>   bitmap dump code via QAPI framework
>   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
> 
>  exec.c                    |   4 +
>  hmp-commands.hx           |  46 +++++++
>  hmp.c                     |  36 +++++
>  hmp.h                     |   3 +
>  include/exec/memory.h     |   3 +-
>  include/exec/ram_addr.h   |  16 ++-
>  include/qapi/qmp/qerror.h |   3 +
>  qapi-schema.json          |  23 ++++
>  qmp-commands.hx           |  77 +++++++++++
>  savevm.c                  | 344 ++++++++++++++++++++++++++++++++++++++++++++++
>  scripts/extract-bitmap.py |  68 +++++++++
>  11 files changed, 621 insertions(+), 2 deletions(-)
>  create mode 100755 scripts/extract-bitmap.py
> 

Re: [Qemu-devel] [PATCH 0/6] Obtain dirty bitmap via VM logging

[Sanidhya Kashyap]

On Wed, May 21, 2014 at 9:43 AM, ChenLiang <chenliang88@huawei.com> wrote:
> Hi,
> Nice job. We should avoid running migration_thread and bitmap_logging_thread simultaneously.
>
Any particular suggestion to avoid running simultaneous execution of
the threads?

--
Sanidhya

Re: [Qemu-devel] [PATCH 0/6] Obtain dirty bitmap via VM logging

[ChenLiang]

On 2014/5/21 12:56, Sanidhya Kashyap wrote:

> On Wed, May 21, 2014 at 9:43 AM, ChenLiang <chenliang88@huawei.com> wrote:
>> Hi,
>> Nice job. We should avoid running migration_thread and bitmap_logging_thread simultaneously.
>>
> Any particular suggestion to avoid running simultaneous execution of
> the threads?
> 
> --
> Sanidhya
> 
> 

We can do it like this:
https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg02185.html

Re: [Qemu-devel] [PATCH 0/6] Obtain dirty bitmap via VM logging

[Sanidhya Kashyap]

>
> We can do it like this:
> https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg02185.html
>

Thanks.

If we are using only one of the threads, then I think we do not
require the splitting of the dirty bitmap (
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04041.html ).
I can directly use the DIRTY_MEMORY_MIGRATION flag. What is your
suggestion?



-- 

Sanidhya Kashyap

Re: [Qemu-devel] [PATCH 0/6] Obtain dirty bitmap via VM logging

[Sanidhya Kashyap]

On Wed, May 21, 2014 at 12:15 PM, ChenLiang <chenliang88@huawei.com> wrote:
> On 2014/5/21 12:56, Sanidhya Kashyap wrote:
>
>> On Wed, May 21, 2014 at 9:43 AM, ChenLiang <chenliang88@huawei.com> wrote:
>>> Hi,
>>> Nice job. We should avoid running migration_thread and bitmap_logging_thread simultaneously.
>>>
>> Any particular suggestion to avoid running simultaneous execution of
>> the threads?
>>
>
> We can do it like this:
> https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg02185.html
>

Hi Chen,

I have some questions which I wanted  to get clarified before
implementing the above part of avoiding the simultaneous execution.

As the migration process is going on, the RUN_STATE_RUNNING state is
being used, which is the same case in bitmap dump process. In order to
use the concept of RUN_STATE_* states, should I create two new states
as RUN_STATE_LOGBITMAP AND RUN_STATE_OUTMIGRATE? The
RUN_STATE_OUTMIGRATE state will have all the transitions that
RUN_STATE_RUNNING supports except the ones being used by migration.
Similarly, RUN_STATE_MIGRATE will support all the transitions of
RUN_STATE_RUNNING except RUN_STATE_LOGBITMAP.

We might also have to update the runstate_is_running function, since
RUN_STATE_LOGBITMAP and RUN_STATE_MIGRATE are almost similar in nature
to the RUN_STATE_RUNNING. What is your opinion about that?

Is it the only approach to do it or are there any simple and efficient
approaches?

Thanks,
Sanidhya

Re: [Qemu-devel] [PATCH 0/6] Obtain dirty bitmap via VM logging

[ChenLiang]

On 2014/5/22 19:21, Sanidhya Kashyap wrote:

> On Wed, May 21, 2014 at 12:15 PM, ChenLiang <chenliang88@huawei.com> wrote:
>> On 2014/5/21 12:56, Sanidhya Kashyap wrote:
>>
>>> On Wed, May 21, 2014 at 9:43 AM, ChenLiang <chenliang88@huawei.com> wrote:
>>>> Hi,
>>>> Nice job. We should avoid running migration_thread and bitmap_logging_thread simultaneously.
>>>>
>>> Any particular suggestion to avoid running simultaneous execution of
>>> the threads?
>>>
>>
>> We can do it like this:
>> https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg02185.html
>>
> 
> Hi Chen,
> 
> I have some questions which I wanted  to get clarified before
> implementing the above part of avoiding the simultaneous execution.
> 
> As the migration process is going on, the RUN_STATE_RUNNING state is
> being used, which is the same case in bitmap dump process. In order to
> use the concept of RUN_STATE_* states, should I create two new states
> as RUN_STATE_LOGBITMAP AND RUN_STATE_OUTMIGRATE? The
> RUN_STATE_OUTMIGRATE state will have all the transitions that
> RUN_STATE_RUNNING supports except the ones being used by migration.
> Similarly, RUN_STATE_MIGRATE will support all the transitions of
> RUN_STATE_RUNNING except RUN_STATE_LOGBITMAP.
> 
> We might also have to update the runstate_is_running function, since
> RUN_STATE_LOGBITMAP and RUN_STATE_MIGRATE are almost similar in nature
> to the RUN_STATE_RUNNING. What is your opinion about that?
> 
> Is it the only approach to do it or are there any simple and efficient
> approaches?
> 
> Thanks,
> Sanidhya
> 
>


Hmm, I think we need the two flags. Although it is little hard to do it. Because the vm can't
do something like hotplug vcpu or device during migrate. But for now qemu don't guarantee it(in my reading).
So we should change the vm state  RUN_STATE_RUNNING to RUN_STATE_MIGRATE when migrate.
BTW, it is not your patches problem.

cc:  Juan

Best regards
ChenLiang

Re: [Qemu-devel] [PATCH 0/6] Obtain dirty bitmap via VM logging

[Sanidhya Kashyap]

> Hmm, I think we need the two flags. Although it is little hard to do it. Because the vm can't
> do something like hotplug vcpu or device during migrate. But for now qemu don't guarantee it(in my reading).
> So we should change the vm state  RUN_STATE_RUNNING to RUN_STATE_MIGRATE when migrate.
> BTW, it is not your patches problem.
>

Then, I'll go ahead and implement this part using two flags in my
second version of the patches.

> cc:  Juan
>
> Best regards
> ChenLiang
>



-- 

Sanidhya Kashyap