[Qemu-devel] [PATCH 0/4] Xen, introducing dirty log for migration.

[Qemu-devel] [PATCH 0/4] Xen, introducing dirty log for migration.

[Anthony PERARD]

Hi,

This patch set will fix live migration under Xen. For this I introduce a new
QMP command to switch global-dirty log and few calls (in exec.c and memory.c)
to xen set_dirty function.

It unfortunatly necessary to adds new calls because
cpu_physical_memory_set_dirty is not call if the memory is already consider as
dirty by QEMU. But Xen does not handle the same dirty bitmap.

Thanks for your reviews,


Anthony PERARD (4):
  QMP, Introduce set-global-dirty-log command.
  xen: Introduce xen_modified_memory.
  exec, memory: Call to xen_modified_memory.
  xen: Always set the vram dirty during migration.

 exec.c           |    4 ++++
 hw/xen.h         |    1 +
 memory.c         |    2 ++
 qapi-schema.json |   13 +++++++++++++
 qmp-commands.hx  |   24 ++++++++++++++++++++++++
 xen-all.c        |   41 +++++++++++++++++++++++++++++++++++++++++
 xen-stub.c       |    9 +++++++++
 7 files changed, 94 insertions(+), 0 deletions(-)

-- 
Anthony PERARD

[Qemu-devel] [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Anthony PERARD]

This command is used during a migration of a guest under Xen. It calls
memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
argument pass to the command.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 qapi-schema.json |   13 +++++++++++++
 qmp-commands.hx  |   24 ++++++++++++++++++++++++
 xen-all.c        |   15 +++++++++++++++
 xen-stub.c       |    5 +++++
 4 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 1ab5dbd..e8d17b5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1765,6 +1765,19 @@
 { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
 
 ##
+# @xen-set-global-dirty-log
+#
+# Enable or disable the global dirty log mode.
+#
+# @enable: true to enable, false to disable.
+#
+# Returns: nothing
+#
+# Since: 1.2
+##
+{ 'command': 'xen-set-global-dirty-log', 'data': { 'enable': 'bool' } }
+
+##
 # @device_del:
 #
 # Remove a device from a guest
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2e1a38e..c92decd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -468,6 +468,30 @@ Example:
 EQMP
 
     {
+        .name       = "xen-set-global-dirty-log",
+        .args_type  = "enable:b",
+        .mhandler.cmd_new = qmp_marshal_input_xen_set_global_dirty_log,
+    },
+
+SQMP
+xen-set-global-dirty-log
+-------
+
+Enable or disable the global dirty log mode.
+
+Arguments:
+
+- "enable": Enable it or disable it.
+
+Example:
+
+-> { "execute": "xen-set-global-dirty-log",
+     "arguments": { "enable": true } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
         .mhandler.cmd_new = qmp_marshal_input_migrate,
diff --git a/xen-all.c b/xen-all.c
index 59f2323..8995d43 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -14,6 +14,7 @@
 #include "hw/pc.h"
 #include "hw/xen_common.h"
 #include "hw/xen_backend.h"
+#include "qmp-commands.h"
 
 #include "range.h"
 #include "xen-mapcache.h"
@@ -36,6 +37,7 @@
 
 static MemoryRegion ram_memory, ram_640k, ram_lo, ram_hi;
 static MemoryRegion *framebuffer;
+static bool xen_in_migration;
 
 /* Compatibility with older version */
 #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
@@ -552,10 +554,14 @@ static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
 
 static void xen_log_global_start(MemoryListener *listener)
 {
+    if (xen_enabled()) {
+        xen_in_migration = true;
+    }
 }
 
 static void xen_log_global_stop(MemoryListener *listener)
 {
+    xen_in_migration = false;
 }
 
 static void xen_eventfd_add(MemoryListener *listener,
@@ -586,6 +592,15 @@ static MemoryListener xen_memory_listener = {
     .priority = 10,
 };
 
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+    if (enable) {
+        memory_global_dirty_log_start();
+    } else {
+        memory_global_dirty_log_stop();
+    }
+}
+
 /* VCPU Operations, MMIO, IO ring ... */
 
 static void xen_reset_vcpu(void *opaque)
diff --git a/xen-stub.c b/xen-stub.c
index 8ff2b79..5e66ba8 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -11,6 +11,7 @@
 #include "qemu-common.h"
 #include "hw/xen.h"
 #include "memory.h"
+#include "qmp-commands.h"
 
 void xenstore_store_pv_console_info(int i, CharDriverState *chr)
 {
@@ -54,3 +55,7 @@ int xen_init(void)
 void xen_register_framebuffer(MemoryRegion *mr)
 {
 }
+
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+}
-- 
Anthony PERARD

[PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Anthony PERARD]

This command is used during a migration of a guest under Xen. It calls
memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
argument pass to the command.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 qapi-schema.json |   13 +++++++++++++
 qmp-commands.hx  |   24 ++++++++++++++++++++++++
 xen-all.c        |   15 +++++++++++++++
 xen-stub.c       |    5 +++++
 4 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 1ab5dbd..e8d17b5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1765,6 +1765,19 @@
 { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
 
 ##
+# @xen-set-global-dirty-log
+#
+# Enable or disable the global dirty log mode.
+#
+# @enable: true to enable, false to disable.
+#
+# Returns: nothing
+#
+# Since: 1.2
+##
+{ 'command': 'xen-set-global-dirty-log', 'data': { 'enable': 'bool' } }
+
+##
 # @device_del:
 #
 # Remove a device from a guest
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2e1a38e..c92decd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -468,6 +468,30 @@ Example:
 EQMP
 
     {
+        .name       = "xen-set-global-dirty-log",
+        .args_type  = "enable:b",
+        .mhandler.cmd_new = qmp_marshal_input_xen_set_global_dirty_log,
+    },
+
+SQMP
+xen-set-global-dirty-log
+-------
+
+Enable or disable the global dirty log mode.
+
+Arguments:
+
+- "enable": Enable it or disable it.
+
+Example:
+
+-> { "execute": "xen-set-global-dirty-log",
+     "arguments": { "enable": true } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
         .mhandler.cmd_new = qmp_marshal_input_migrate,
diff --git a/xen-all.c b/xen-all.c
index 59f2323..8995d43 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -14,6 +14,7 @@
 #include "hw/pc.h"
 #include "hw/xen_common.h"
 #include "hw/xen_backend.h"
+#include "qmp-commands.h"
 
 #include "range.h"
 #include "xen-mapcache.h"
@@ -36,6 +37,7 @@
 
 static MemoryRegion ram_memory, ram_640k, ram_lo, ram_hi;
 static MemoryRegion *framebuffer;
+static bool xen_in_migration;
 
 /* Compatibility with older version */
 #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
@@ -552,10 +554,14 @@ static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
 
 static void xen_log_global_start(MemoryListener *listener)
 {
+    if (xen_enabled()) {
+        xen_in_migration = true;
+    }
 }
 
 static void xen_log_global_stop(MemoryListener *listener)
 {
+    xen_in_migration = false;
 }
 
 static void xen_eventfd_add(MemoryListener *listener,
@@ -586,6 +592,15 @@ static MemoryListener xen_memory_listener = {
     .priority = 10,
 };
 
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+    if (enable) {
+        memory_global_dirty_log_start();
+    } else {
+        memory_global_dirty_log_stop();
+    }
+}
+
 /* VCPU Operations, MMIO, IO ring ... */
 
 static void xen_reset_vcpu(void *opaque)
diff --git a/xen-stub.c b/xen-stub.c
index 8ff2b79..5e66ba8 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -11,6 +11,7 @@
 #include "qemu-common.h"
 #include "hw/xen.h"
 #include "memory.h"
+#include "qmp-commands.h"
 
 void xenstore_store_pv_console_info(int i, CharDriverState *chr)
 {
@@ -54,3 +55,7 @@ int xen_init(void)
 void xen_register_framebuffer(MemoryRegion *mr)
 {
 }
+
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+}
-- 
Anthony PERARD

[Qemu-devel] [PATCH 2/4] xen: Introduce xen_modified_memory.

[Anthony PERARD]

This function is to be used during live migration. Every write access to the
guest memory should call this funcion so the Xen tools knows which pages are
dirty.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/xen.h   |    1 +
 xen-all.c  |   21 +++++++++++++++++++++
 xen-stub.c |    4 ++++
 3 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/hw/xen.h b/hw/xen.h
index e5926b7..d14e92d 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -48,6 +48,7 @@ void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
 struct MemoryRegion;
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
                    struct MemoryRegion *mr);
+void xen_modified_memory(ram_addr_t start, ram_addr_t length);
 #endif
 
 struct MemoryRegion;
diff --git a/xen-all.c b/xen-all.c
index 8995d43..498883b 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -1218,3 +1218,24 @@ void xen_shutdown_fatal_error(const char *fmt, ...)
     /* destroy the domain */
     qemu_system_shutdown_request();
 }
+
+void xen_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+    if (unlikely(xen_in_migration)) {
+        int rc;
+        ram_addr_t start_pfn, nb_pages;
+
+        if (length == 0) {
+            length = TARGET_PAGE_SIZE;
+        }
+        start_pfn = start >> TARGET_PAGE_BITS;
+        nb_pages = ((start + length + TARGET_PAGE_SIZE - 1) >> TARGET_PAGE_BITS)
+            - start_pfn;
+        rc = xc_hvm_modified_memory(xen_xc, xen_domid, start_pfn, nb_pages);
+        if (rc) {
+            fprintf(stderr, "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT
+                    "): %i, %s\n", __func__,
+                    start, nb_pages, rc, strerror(-rc));
+        }
+    }
+}
diff --git a/xen-stub.c b/xen-stub.c
index 5e66ba8..9214392 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -59,3 +59,7 @@ void xen_register_framebuffer(MemoryRegion *mr)
 void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
 {
 }
+
+void xen_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+}
-- 
Anthony PERARD

[PATCH 2/4] xen: Introduce xen_modified_memory.

[Anthony PERARD]

This function is to be used during live migration. Every write access to the
guest memory should call this funcion so the Xen tools knows which pages are
dirty.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/xen.h   |    1 +
 xen-all.c  |   21 +++++++++++++++++++++
 xen-stub.c |    4 ++++
 3 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/hw/xen.h b/hw/xen.h
index e5926b7..d14e92d 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -48,6 +48,7 @@ void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
 struct MemoryRegion;
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
                    struct MemoryRegion *mr);
+void xen_modified_memory(ram_addr_t start, ram_addr_t length);
 #endif
 
 struct MemoryRegion;
diff --git a/xen-all.c b/xen-all.c
index 8995d43..498883b 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -1218,3 +1218,24 @@ void xen_shutdown_fatal_error(const char *fmt, ...)
     /* destroy the domain */
     qemu_system_shutdown_request();
 }
+
+void xen_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+    if (unlikely(xen_in_migration)) {
+        int rc;
+        ram_addr_t start_pfn, nb_pages;
+
+        if (length == 0) {
+            length = TARGET_PAGE_SIZE;
+        }
+        start_pfn = start >> TARGET_PAGE_BITS;
+        nb_pages = ((start + length + TARGET_PAGE_SIZE - 1) >> TARGET_PAGE_BITS)
+            - start_pfn;
+        rc = xc_hvm_modified_memory(xen_xc, xen_domid, start_pfn, nb_pages);
+        if (rc) {
+            fprintf(stderr, "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT
+                    "): %i, %s\n", __func__,
+                    start, nb_pages, rc, strerror(-rc));
+        }
+    }
+}
diff --git a/xen-stub.c b/xen-stub.c
index 5e66ba8..9214392 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -59,3 +59,7 @@ void xen_register_framebuffer(MemoryRegion *mr)
 void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
 {
 }
+
+void xen_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+}
-- 
Anthony PERARD

[Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

This patch add some calls to xen_modified_memory to notify Xen about dirtybits
during migration.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 exec.c   |    4 ++++
 memory.c |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/exec.c b/exec.c
index c9fa17d..9f7a4f7 100644
--- a/exec.c
+++ b/exec.c
@@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                     cpu_physical_memory_set_dirty_flags(
                         addr1, (0xff & ~CODE_DIRTY_FLAG));
                 }
+                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
                 qemu_put_ram_ptr(ptr);
             }
         } else {
@@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
     if (buffer != bounce.buffer) {
         if (is_write) {
             ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
+            xen_modified_memory(addr1, access_len);
             while (access_len) {
                 unsigned l;
                 l = TARGET_PAGE_SIZE;
@@ -3947,6 +3949,7 @@ static inline void stl_phys_internal(target_phys_addr_t addr, uint32_t val,
             cpu_physical_memory_set_dirty_flags(addr1,
                 (0xff & ~CODE_DIRTY_FLAG));
         }
+        xen_modified_memory(addr1, 4);
     }
 }
 
@@ -4020,6 +4023,7 @@ static inline void stw_phys_internal(target_phys_addr_t addr, uint32_t val,
             cpu_physical_memory_set_dirty_flags(addr1,
                 (0xff & ~CODE_DIRTY_FLAG));
         }
+        xen_modified_memory(addr1, 2);
     }
 }
 
diff --git a/memory.c b/memory.c
index aab4a31..4d004e2 100644
--- a/memory.c
+++ b/memory.c
@@ -19,6 +19,7 @@
 #include "bitops.h"
 #include "kvm.h"
 #include <assert.h>
+#include "hw/xen.h"
 
 #define WANT_EXEC_OBSOLETE
 #include "exec-obsolete.h"
@@ -1085,6 +1086,7 @@ void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr,
                              target_phys_addr_t size)
 {
     assert(mr->terminates);
+    xen_modified_memory(mr->ram_addr + addr, size);
     return cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, -1);
 }
 
-- 
Anthony PERARD

[PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

This patch add some calls to xen_modified_memory to notify Xen about dirtybits
during migration.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 exec.c   |    4 ++++
 memory.c |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/exec.c b/exec.c
index c9fa17d..9f7a4f7 100644
--- a/exec.c
+++ b/exec.c
@@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                     cpu_physical_memory_set_dirty_flags(
                         addr1, (0xff & ~CODE_DIRTY_FLAG));
                 }
+                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
                 qemu_put_ram_ptr(ptr);
             }
         } else {
@@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
     if (buffer != bounce.buffer) {
         if (is_write) {
             ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
+            xen_modified_memory(addr1, access_len);
             while (access_len) {
                 unsigned l;
                 l = TARGET_PAGE_SIZE;
@@ -3947,6 +3949,7 @@ static inline void stl_phys_internal(target_phys_addr_t addr, uint32_t val,
             cpu_physical_memory_set_dirty_flags(addr1,
                 (0xff & ~CODE_DIRTY_FLAG));
         }
+        xen_modified_memory(addr1, 4);
     }
 }
 
@@ -4020,6 +4023,7 @@ static inline void stw_phys_internal(target_phys_addr_t addr, uint32_t val,
             cpu_physical_memory_set_dirty_flags(addr1,
                 (0xff & ~CODE_DIRTY_FLAG));
         }
+        xen_modified_memory(addr1, 2);
     }
 }
 
diff --git a/memory.c b/memory.c
index aab4a31..4d004e2 100644
--- a/memory.c
+++ b/memory.c
@@ -19,6 +19,7 @@
 #include "bitops.h"
 #include "kvm.h"
 #include <assert.h>
+#include "hw/xen.h"
 
 #define WANT_EXEC_OBSOLETE
 #include "exec-obsolete.h"
@@ -1085,6 +1086,7 @@ void memory_region_set_dirty(MemoryRegion *mr, target_phys_addr_t addr,
                              target_phys_addr_t size)
 {
     assert(mr->terminates);
+    xen_modified_memory(mr->ram_addr + addr, size);
     return cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, -1);
 }
 
-- 
Anthony PERARD

[Qemu-devel] [PATCH 4/4] xen: Always set the vram dirty during migration.

[Anthony PERARD]

Because the call to track the dirty bit in the video ram during migration won't
work (it returns -1), we set dirtybit on the all video ram.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen-all.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index 498883b..00bdb50 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
         return;
     }
 
+    if (unlikely(xen_in_migration)) {
+        /* track_dirty_vram does not work during migration */
+        memory_region_set_dirty(framebuffer, 0, size);
+        return;
+    }
     rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
                                  start_addr >> TARGET_PAGE_BITS, npages,
                                  bitmap);
-- 
Anthony PERARD

[PATCH 4/4] xen: Always set the vram dirty during migration.

[Anthony PERARD]

Because the call to track the dirty bit in the video ram during migration won't
work (it returns -1), we set dirtybit on the all video ram.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen-all.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index 498883b..00bdb50 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
         return;
     }
 
+    if (unlikely(xen_in_migration)) {
+        /* track_dirty_vram does not work during migration */
+        memory_region_set_dirty(framebuffer, 0, size);
+        return;
+    }
     rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
                                  start_addr >> TARGET_PAGE_BITS, npages,
                                  bitmap);
-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Avi Kivity]

On 07/17/2012 04:30 PM, Anthony PERARD wrote:
> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
> during migration.
> 
> diff --git a/exec.c b/exec.c
> index c9fa17d..9f7a4f7 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>                      cpu_physical_memory_set_dirty_flags(
>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
>                  }
> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
>                  qemu_put_ram_ptr(ptr);
>              }
>          } else {

This is pretty ugly.  An alternative is to set up a periodic bitmap scan
that looks at the qemu dirty bitmap and calls xen_modified_memory() for
dirty page ranges, and clears the bitmap for the next pass.  Is it workable?

(is xen_modified_memory a hypercall, or does it maintain an in-memory
structure?)


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Avi Kivity]

On 07/17/2012 04:30 PM, Anthony PERARD wrote:
> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
> during migration.
> 
> diff --git a/exec.c b/exec.c
> index c9fa17d..9f7a4f7 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>                      cpu_physical_memory_set_dirty_flags(
>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
>                  }
> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
>                  qemu_put_ram_ptr(ptr);
>              }
>          } else {

This is pretty ugly.  An alternative is to set up a periodic bitmap scan
that looks at the qemu dirty bitmap and calls xen_modified_memory() for
dirty page ranges, and clears the bitmap for the next pass.  Is it workable?

(is xen_modified_memory a hypercall, or does it maintain an in-memory
structure?)


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Eric Blake]

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

On 07/17/2012 07:30 AM, Anthony PERARD wrote:

In the subject, s/set-global/xen-&/, so that searching git log for the
full monitor command name will hit this commit.

> This command is used during a migration of a guest under Xen. It calls
> memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
> argument pass to the command.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  qapi-schema.json |   13 +++++++++++++
>  qmp-commands.hx  |   24 ++++++++++++++++++++++++
>  xen-all.c        |   15 +++++++++++++++
>  xen-stub.c       |    5 +++++
>  4 files changed, 57 insertions(+), 0 deletions(-)
> 

-- 
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: 620 bytes --]

Re: [Qemu-devel] [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 775 bytes --]

On 07/17/2012 07:30 AM, Anthony PERARD wrote:

In the subject, s/set-global/xen-&/, so that searching git log for the
full monitor command name will hit this commit.

> This command is used during a migration of a guest under Xen. It calls
> memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
> argument pass to the command.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  qapi-schema.json |   13 +++++++++++++
>  qmp-commands.hx  |   24 ++++++++++++++++++++++++
>  xen-all.c        |   15 +++++++++++++++
>  xen-stub.c       |    5 +++++
>  4 files changed, 57 insertions(+), 0 deletions(-)
> 

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


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

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Avi Kivity]

On 07/17/2012 04:30 PM, Anthony PERARD wrote:
> This command is used during a migration of a guest under Xen. It calls
> memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
> argument pass to the command.

Is the command truly needed?  Can't it come from the xen library you
link to?


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Avi Kivity]

On 07/17/2012 04:30 PM, Anthony PERARD wrote:
> This command is used during a migration of a guest under Xen. It calls
> memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
> argument pass to the command.

Is the command truly needed?  Can't it come from the xen library you
link to?


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

On 17/07/12 14:37, Avi Kivity wrote:
> On 07/17/2012 04:30 PM, Anthony PERARD wrote:
>> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
>> during migration.
>>
>> diff --git a/exec.c b/exec.c
>> index c9fa17d..9f7a4f7 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>>                       cpu_physical_memory_set_dirty_flags(
>>                           addr1, (0xff & ~CODE_DIRTY_FLAG));
>>                   }
>> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
>>                   qemu_put_ram_ptr(ptr);
>>               }
>>           } else {
>
> This is pretty ugly.  An alternative is to set up a periodic bitmap scan
> that looks at the qemu dirty bitmap and calls xen_modified_memory() for
> dirty page ranges, and clears the bitmap for the next pass.  Is it workable?

I don't think a periodic scan can do anything useful, unfortunately.

> (is xen_modified_memory a hypercall, or does it maintain an in-memory
> structure?)

It's an hypercall. The function do something (call the hypercall) only 
during migration, otherwise it return immediately.

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

On 17/07/12 14:37, Avi Kivity wrote:
> On 07/17/2012 04:30 PM, Anthony PERARD wrote:
>> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
>> during migration.
>>
>> diff --git a/exec.c b/exec.c
>> index c9fa17d..9f7a4f7 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>>                       cpu_physical_memory_set_dirty_flags(
>>                           addr1, (0xff & ~CODE_DIRTY_FLAG));
>>                   }
>> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
>>                   qemu_put_ram_ptr(ptr);
>>               }
>>           } else {
>
> This is pretty ugly.  An alternative is to set up a periodic bitmap scan
> that looks at the qemu dirty bitmap and calls xen_modified_memory() for
> dirty page ranges, and clears the bitmap for the next pass.  Is it workable?

I don't think a periodic scan can do anything useful, unfortunately.

> (is xen_modified_memory a hypercall, or does it maintain an in-memory
> structure?)

It's an hypercall. The function do something (call the hypercall) only 
during migration, otherwise it return immediately.

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Avi Kivity]

On 07/17/2012 04:59 PM, Anthony PERARD wrote:
>>
>> This is pretty ugly.  An alternative is to set up a periodic bitmap scan
>> that looks at the qemu dirty bitmap and calls xen_modified_memory() for
>> dirty page ranges, and clears the bitmap for the next pass.  Is it
>> workable?
> 
> I don't think a periodic scan can do anything useful, unfortunately.

Why not?

>> (is xen_modified_memory a hypercall, or does it maintain an in-memory
>> structure?)
> 
> It's an hypercall. The function do something (call the hypercall) only
> during migration, otherwise it return immediately.

I see.  I guess it isn't expensive for you because there isn't much dma
done by qemu usually with xen (unlike kvm where pv block devices are
implemented in qemu).

How about pushing the call into cpu_physical_memory_set_dirty_flags()?
Would that reduce the number of call sites?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Avi Kivity]

On 07/17/2012 04:59 PM, Anthony PERARD wrote:
>>
>> This is pretty ugly.  An alternative is to set up a periodic bitmap scan
>> that looks at the qemu dirty bitmap and calls xen_modified_memory() for
>> dirty page ranges, and clears the bitmap for the next pass.  Is it
>> workable?
> 
> I don't think a periodic scan can do anything useful, unfortunately.

Why not?

>> (is xen_modified_memory a hypercall, or does it maintain an in-memory
>> structure?)
> 
> It's an hypercall. The function do something (call the hypercall) only
> during migration, otherwise it return immediately.

I see.  I guess it isn't expensive for you because there isn't much dma
done by qemu usually with xen (unlike kvm where pv block devices are
implemented in qemu).

How about pushing the call into cpu_physical_memory_set_dirty_flags()?
Would that reduce the number of call sites?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Avi Kivity wrote:
> On 07/17/2012 04:30 PM, Anthony PERARD wrote:
> > This command is used during a migration of a guest under Xen. It calls
> > memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
> > argument pass to the command.
> 
> Is the command truly needed?  Can't it come from the xen library you
> link to?

I thought that a while ago we decided to use QMP rather than xenstore to
issue Xen commands to QEMU.
The only xenstore stuff left are the PV protocols and few parameters.

Of course we could use xenstore to issue commands again, but it goes
against the last year and an half of development :-)

Re: [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Avi Kivity wrote:
> On 07/17/2012 04:30 PM, Anthony PERARD wrote:
> > This command is used during a migration of a guest under Xen. It calls
> > memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
> > argument pass to the command.
> 
> Is the command truly needed?  Can't it come from the xen library you
> link to?

I thought that a while ago we decided to use QMP rather than xenstore to
issue Xen commands to QEMU.
The only xenstore stuff left are the PV protocols and few parameters.

Of course we could use xenstore to issue commands again, but it goes
against the last year and an half of development :-)

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Anthony PERARD wrote:
> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
> during migration.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  exec.c   |    4 ++++
>  memory.c |    2 ++
>  2 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index c9fa17d..9f7a4f7 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>                      cpu_physical_memory_set_dirty_flags(
>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
>                  }
> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
>                  qemu_put_ram_ptr(ptr);
>              }
>          } else {
> @@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
>      if (buffer != bounce.buffer) {
>          if (is_write) {
>              ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
> +            xen_modified_memory(addr1, access_len);
>              while (access_len) {
>                  unsigned l;
>                  l = TARGET_PAGE_SIZE;

You need to add xen_modified_memory in cpu_physical_memory_map, rather
than cpu_physical_memory_unmap.

Re: [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Anthony PERARD wrote:
> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
> during migration.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  exec.c   |    4 ++++
>  memory.c |    2 ++
>  2 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index c9fa17d..9f7a4f7 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>                      cpu_physical_memory_set_dirty_flags(
>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
>                  }
> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
>                  qemu_put_ram_ptr(ptr);
>              }
>          } else {
> @@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
>      if (buffer != bounce.buffer) {
>          if (is_write) {
>              ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
> +            xen_modified_memory(addr1, access_len);
>              while (access_len) {
>                  unsigned l;
>                  l = TARGET_PAGE_SIZE;

You need to add xen_modified_memory in cpu_physical_memory_map, rather
than cpu_physical_memory_unmap.

Re: [Qemu-devel] [PATCH 4/4] xen: Always set the vram dirty during migration.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Anthony PERARD wrote:
> Because the call to track the dirty bit in the video ram during migration won't
> work (it returns -1), we set dirtybit on the all video ram.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  xen-all.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/xen-all.c b/xen-all.c
> index 498883b..00bdb50 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>          return;
>      }
>  
> +    if (unlikely(xen_in_migration)) {
> +        /* track_dirty_vram does not work during migration */
> +        memory_region_set_dirty(framebuffer, 0, size);
> +        return;
> +    }
>      rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
>                                   start_addr >> TARGET_PAGE_BITS, npages,
>                                   bitmap);

Why are you setting the entire framebuffer dirty?
We should set dirty only the actualy region that is supposed to be
dirty.

Re: [PATCH 4/4] xen: Always set the vram dirty during migration.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Anthony PERARD wrote:
> Because the call to track the dirty bit in the video ram during migration won't
> work (it returns -1), we set dirtybit on the all video ram.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  xen-all.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/xen-all.c b/xen-all.c
> index 498883b..00bdb50 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>          return;
>      }
>  
> +    if (unlikely(xen_in_migration)) {
> +        /* track_dirty_vram does not work during migration */
> +        memory_region_set_dirty(framebuffer, 0, size);
> +        return;
> +    }
>      rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
>                                   start_addr >> TARGET_PAGE_BITS, npages,
>                                   bitmap);

Why are you setting the entire framebuffer dirty?
We should set dirty only the actualy region that is supposed to be
dirty.

Re: [Qemu-devel] [PATCH 4/4] xen: Always set the vram dirty during migration.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Anthony PERARD wrote:
> > Because the call to track the dirty bit in the video ram during migration won't
> > work (it returns -1), we set dirtybit on the all video ram.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > ---
> >  xen-all.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/xen-all.c b/xen-all.c
> > index 498883b..00bdb50 100644
> > --- a/xen-all.c
> > +++ b/xen-all.c
> > @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
> >          return;
> >      }
> >  
> > +    if (unlikely(xen_in_migration)) {
> > +        /* track_dirty_vram does not work during migration */
> > +        memory_region_set_dirty(framebuffer, 0, size);
> > +        return;
> > +    }
> >      rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
> >                                   start_addr >> TARGET_PAGE_BITS, npages,
> >                                   bitmap);
> 
> Why are you setting the entire framebuffer dirty?
> We should set dirty only the actualy region that is supposed to be
> dirty.
> 

Also memory_region_set_dirty calls cpu_physical_memory_set_dirty_range,
in which you didn't add a call to xen_modified_memory.

Re: [PATCH 4/4] xen: Always set the vram dirty during migration.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Anthony PERARD wrote:
> > Because the call to track the dirty bit in the video ram during migration won't
> > work (it returns -1), we set dirtybit on the all video ram.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > ---
> >  xen-all.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/xen-all.c b/xen-all.c
> > index 498883b..00bdb50 100644
> > --- a/xen-all.c
> > +++ b/xen-all.c
> > @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
> >          return;
> >      }
> >  
> > +    if (unlikely(xen_in_migration)) {
> > +        /* track_dirty_vram does not work during migration */
> > +        memory_region_set_dirty(framebuffer, 0, size);
> > +        return;
> > +    }
> >      rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
> >                                   start_addr >> TARGET_PAGE_BITS, npages,
> >                                   bitmap);
> 
> Why are you setting the entire framebuffer dirty?
> We should set dirty only the actualy region that is supposed to be
> dirty.
> 

Also memory_region_set_dirty calls cpu_physical_memory_set_dirty_range,
in which you didn't add a call to xen_modified_memory.

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Avi Kivity wrote:
> On 07/17/2012 04:59 PM, Anthony PERARD wrote:
> >>
> >> This is pretty ugly.  An alternative is to set up a periodic bitmap scan
> >> that looks at the qemu dirty bitmap and calls xen_modified_memory() for
> >> dirty page ranges, and clears the bitmap for the next pass.  Is it
> >> workable?
> > 
> > I don't think a periodic scan can do anything useful, unfortunately.
> 
> Why not?

I vaguely remember that we used to have a bitmap years ago, but, aside from
making the code much more complicated, it caused blue screens on
intensive disk accesses.


> >> (is xen_modified_memory a hypercall, or does it maintain an in-memory
> >> structure?)
> > 
> > It's an hypercall. The function do something (call the hypercall) only
> > during migration, otherwise it return immediately.
> 
> I see.  I guess it isn't expensive for you because there isn't much dma
> done by qemu usually with xen (unlike kvm where pv block devices are
> implemented in qemu).
> 
> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
> Would that reduce the number of call sites?

Pushing the calls to cpu_physical_memory_set_dirty_flags and
cpu_physical_memory_set_dirty_range would make the code much nicer.
However being these functions in exec-obsolete.h, are they at risk of
removal?

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Stefano Stabellini]

On Tue, 17 Jul 2012, Avi Kivity wrote:
> On 07/17/2012 04:59 PM, Anthony PERARD wrote:
> >>
> >> This is pretty ugly.  An alternative is to set up a periodic bitmap scan
> >> that looks at the qemu dirty bitmap and calls xen_modified_memory() for
> >> dirty page ranges, and clears the bitmap for the next pass.  Is it
> >> workable?
> > 
> > I don't think a periodic scan can do anything useful, unfortunately.
> 
> Why not?

I vaguely remember that we used to have a bitmap years ago, but, aside from
making the code much more complicated, it caused blue screens on
intensive disk accesses.


> >> (is xen_modified_memory a hypercall, or does it maintain an in-memory
> >> structure?)
> > 
> > It's an hypercall. The function do something (call the hypercall) only
> > during migration, otherwise it return immediately.
> 
> I see.  I guess it isn't expensive for you because there isn't much dma
> done by qemu usually with xen (unlike kvm where pv block devices are
> implemented in qemu).
> 
> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
> Would that reduce the number of call sites?

Pushing the calls to cpu_physical_memory_set_dirty_flags and
cpu_physical_memory_set_dirty_range would make the code much nicer.
However being these functions in exec-obsolete.h, are they at risk of
removal?

Re: [Qemu-devel] [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Avi Kivity]

On 07/17/2012 08:58 PM, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Avi Kivity wrote:
>> On 07/17/2012 04:30 PM, Anthony PERARD wrote:
>> > This command is used during a migration of a guest under Xen. It calls
>> > memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
>> > argument pass to the command.
>> 
>> Is the command truly needed?  Can't it come from the xen library you
>> link to?
> 
> I thought that a while ago we decided to use QMP rather than xenstore to
> issue Xen commands to QEMU.
> The only xenstore stuff left are the PV protocols and few parameters.
> 
> Of course we could use xenstore to issue commands again, but it goes
> against the last year and an half of development :-)

This particular command is weird.  You enable dirty logging via an
external interface, but all output goes through the internal interface.
 It doesn't make much sense.

But let's not reopen the issue, if it was decided to go with qmp command
for those things then so be it.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/4] QMP, Introduce set-global-dirty-log command.

[Avi Kivity]

On 07/17/2012 08:58 PM, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Avi Kivity wrote:
>> On 07/17/2012 04:30 PM, Anthony PERARD wrote:
>> > This command is used during a migration of a guest under Xen. It calls
>> > memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
>> > argument pass to the command.
>> 
>> Is the command truly needed?  Can't it come from the xen library you
>> link to?
> 
> I thought that a while ago we decided to use QMP rather than xenstore to
> issue Xen commands to QEMU.
> The only xenstore stuff left are the PV protocols and few parameters.
> 
> Of course we could use xenstore to issue commands again, but it goes
> against the last year and an half of development :-)

This particular command is weird.  You enable dirty logging via an
external interface, but all output goes through the internal interface.
 It doesn't make much sense.

But let's not reopen the issue, if it was decided to go with qmp command
for those things then so be it.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Avi Kivity]

On 07/17/2012 09:36 PM, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Avi Kivity wrote:
>> On 07/17/2012 04:59 PM, Anthony PERARD wrote:
>> >>
>> >> This is pretty ugly.  An alternative is to set up a periodic bitmap scan
>> >> that looks at the qemu dirty bitmap and calls xen_modified_memory() for
>> >> dirty page ranges, and clears the bitmap for the next pass.  Is it
>> >> workable?
>> > 
>> > I don't think a periodic scan can do anything useful, unfortunately.
>> 
>> Why not?
> 
> I vaguely remember that we used to have a bitmap years ago, but, aside from
> making the code much more complicated, it caused blue screens on
> intensive disk accesses.

Surely it was some bug, not the scan itself.

> 
> 
>> >> (is xen_modified_memory a hypercall, or does it maintain an in-memory
>> >> structure?)
>> > 
>> > It's an hypercall. The function do something (call the hypercall) only
>> > during migration, otherwise it return immediately.
>> 
>> I see.  I guess it isn't expensive for you because there isn't much dma
>> done by qemu usually with xen (unlike kvm where pv block devices are
>> implemented in qemu).
>> 
>> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
>> Would that reduce the number of call sites?
> 
> Pushing the calls to cpu_physical_memory_set_dirty_flags and
> cpu_physical_memory_set_dirty_range would make the code much nicer.
> However being these functions in exec-obsolete.h, are they at risk of
> removal?

exec-obsolete.h just means don't add new call sites.  The functions
won't be removed, instead they'll be absorbed into the memory code with
different names and different implementations.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Avi Kivity]

On 07/17/2012 09:36 PM, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Avi Kivity wrote:
>> On 07/17/2012 04:59 PM, Anthony PERARD wrote:
>> >>
>> >> This is pretty ugly.  An alternative is to set up a periodic bitmap scan
>> >> that looks at the qemu dirty bitmap and calls xen_modified_memory() for
>> >> dirty page ranges, and clears the bitmap for the next pass.  Is it
>> >> workable?
>> > 
>> > I don't think a periodic scan can do anything useful, unfortunately.
>> 
>> Why not?
> 
> I vaguely remember that we used to have a bitmap years ago, but, aside from
> making the code much more complicated, it caused blue screens on
> intensive disk accesses.

Surely it was some bug, not the scan itself.

> 
> 
>> >> (is xen_modified_memory a hypercall, or does it maintain an in-memory
>> >> structure?)
>> > 
>> > It's an hypercall. The function do something (call the hypercall) only
>> > during migration, otherwise it return immediately.
>> 
>> I see.  I guess it isn't expensive for you because there isn't much dma
>> done by qemu usually with xen (unlike kvm where pv block devices are
>> implemented in qemu).
>> 
>> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
>> Would that reduce the number of call sites?
> 
> Pushing the calls to cpu_physical_memory_set_dirty_flags and
> cpu_physical_memory_set_dirty_range would make the code much nicer.
> However being these functions in exec-obsolete.h, are they at risk of
> removal?

exec-obsolete.h just means don't add new call sites.  The functions
won't be removed, instead they'll be absorbed into the memory code with
different names and different implementations.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

On 17/07/12 19:36, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Avi Kivity wrote:
>> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
>> Would that reduce the number of call sites?
>
> Pushing the calls to cpu_physical_memory_set_dirty_flags and
> cpu_physical_memory_set_dirty_range would make the code much nicer.
> However being these functions in exec-obsolete.h, are they at risk of
> removal?

I thought about it, but when I saw that set_dirty were called only when 
it was not already set as dirty where the call seams to be necessary.

I just try to call xen_modified_mem only within 
cpu_phy_mem_set_dirty_flags but it does not work, even when I tried to 
clear the dirtybits. But I maybe don't do the right thing yet to clear 
the dirty bits

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

On 17/07/12 19:36, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Avi Kivity wrote:
>> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
>> Would that reduce the number of call sites?
>
> Pushing the calls to cpu_physical_memory_set_dirty_flags and
> cpu_physical_memory_set_dirty_range would make the code much nicer.
> However being these functions in exec-obsolete.h, are they at risk of
> removal?

I thought about it, but when I saw that set_dirty were called only when 
it was not already set as dirty where the call seams to be necessary.

I just try to call xen_modified_mem only within 
cpu_phy_mem_set_dirty_flags but it does not work, even when I tried to 
clear the dirtybits. But I maybe don't do the right thing yet to clear 
the dirty bits

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Avi Kivity]

On 07/19/2012 02:41 PM, Anthony PERARD wrote:
> On 17/07/12 19:36, Stefano Stabellini wrote:
>> On Tue, 17 Jul 2012, Avi Kivity wrote:
>>> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
>>> Would that reduce the number of call sites?
>>
>> Pushing the calls to cpu_physical_memory_set_dirty_flags and
>> cpu_physical_memory_set_dirty_range would make the code much nicer.
>> However being these functions in exec-obsolete.h, are they at risk of
>> removal?
> 
> I thought about it, but when I saw that set_dirty were called only when
> it was not already set as dirty where the call seams to be necessary.
> 
> I just try to call xen_modified_mem only within
> cpu_phy_mem_set_dirty_flags but it does not work, even when I tried to
> clear the dirtybits. But I maybe don't do the right thing yet to clear
> the dirty bits

You can wrap the if (not dirty) make_it_dirty() sequence in a helper,
and insert your hypercall in the helper, unconditionally.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Avi Kivity]

On 07/19/2012 02:41 PM, Anthony PERARD wrote:
> On 17/07/12 19:36, Stefano Stabellini wrote:
>> On Tue, 17 Jul 2012, Avi Kivity wrote:
>>> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
>>> Would that reduce the number of call sites?
>>
>> Pushing the calls to cpu_physical_memory_set_dirty_flags and
>> cpu_physical_memory_set_dirty_range would make the code much nicer.
>> However being these functions in exec-obsolete.h, are they at risk of
>> removal?
> 
> I thought about it, but when I saw that set_dirty were called only when
> it was not already set as dirty where the call seams to be necessary.
> 
> I just try to call xen_modified_mem only within
> cpu_phy_mem_set_dirty_flags but it does not work, even when I tried to
> clear the dirtybits. But I maybe don't do the right thing yet to clear
> the dirty bits

You can wrap the if (not dirty) make_it_dirty() sequence in a helper,
and insert your hypercall in the helper, unconditionally.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Paolo Bonzini]

Il 17/07/2012 20:06, Stefano Stabellini ha scritto:
> On Tue, 17 Jul 2012, Anthony PERARD wrote:
>> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
>> during migration.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>  exec.c   |    4 ++++
>>  memory.c |    2 ++
>>  2 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index c9fa17d..9f7a4f7 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>>                      cpu_physical_memory_set_dirty_flags(
>>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
>>                  }
>> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
>>                  qemu_put_ram_ptr(ptr);
>>              }
>>          } else {
>> @@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
>>      if (buffer != bounce.buffer) {
>>          if (is_write) {
>>              ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
>> +            xen_modified_memory(addr1, access_len);
>>              while (access_len) {
>>                  unsigned l;
>>                  l = TARGET_PAGE_SIZE;
> 
> You need to add xen_modified_memory in cpu_physical_memory_map, rather
> than cpu_physical_memory_unmap.

No, adding it to map is wrong, because the RAM save routine can migrate
(and mark as non-dirty) the read buffers _before_ the device models have
written to them.

Paolo

Re: [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Paolo Bonzini]

Il 17/07/2012 20:06, Stefano Stabellini ha scritto:
> On Tue, 17 Jul 2012, Anthony PERARD wrote:
>> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
>> during migration.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>  exec.c   |    4 ++++
>>  memory.c |    2 ++
>>  2 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index c9fa17d..9f7a4f7 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>>                      cpu_physical_memory_set_dirty_flags(
>>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
>>                  }
>> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
>>                  qemu_put_ram_ptr(ptr);
>>              }
>>          } else {
>> @@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
>>      if (buffer != bounce.buffer) {
>>          if (is_write) {
>>              ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
>> +            xen_modified_memory(addr1, access_len);
>>              while (access_len) {
>>                  unsigned l;
>>                  l = TARGET_PAGE_SIZE;
> 
> You need to add xen_modified_memory in cpu_physical_memory_map, rather
> than cpu_physical_memory_unmap.

No, adding it to map is wrong, because the RAM save routine can migrate
(and mark as non-dirty) the read buffers _before_ the device models have
written to them.

Paolo

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

On 19/07/12 12:50, Avi Kivity wrote:
> On 07/19/2012 02:41 PM, Anthony PERARD wrote:
>> On 17/07/12 19:36, Stefano Stabellini wrote:
>>> On Tue, 17 Jul 2012, Avi Kivity wrote:
>>>> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
>>>> Would that reduce the number of call sites?
>>>
>>> Pushing the calls to cpu_physical_memory_set_dirty_flags and
>>> cpu_physical_memory_set_dirty_range would make the code much nicer.
>>> However being these functions in exec-obsolete.h, are they at risk of
>>> removal?
>>
>> I thought about it, but when I saw that set_dirty were called only when
>> it was not already set as dirty where the call seams to be necessary.
>>
>> I just try to call xen_modified_mem only within
>> cpu_phy_mem_set_dirty_flags but it does not work, even when I tried to
>> clear the dirtybits. But I maybe don't do the right thing yet to clear
>> the dirty bits
>
> You can wrap the if (not dirty) make_it_dirty() sequence in a helper,
> and insert your hypercall in the helper, unconditionally.

Ok, I'll do that.

Thanks,

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

On 19/07/12 12:50, Avi Kivity wrote:
> On 07/19/2012 02:41 PM, Anthony PERARD wrote:
>> On 17/07/12 19:36, Stefano Stabellini wrote:
>>> On Tue, 17 Jul 2012, Avi Kivity wrote:
>>>> How about pushing the call into cpu_physical_memory_set_dirty_flags()?
>>>> Would that reduce the number of call sites?
>>>
>>> Pushing the calls to cpu_physical_memory_set_dirty_flags and
>>> cpu_physical_memory_set_dirty_range would make the code much nicer.
>>> However being these functions in exec-obsolete.h, are they at risk of
>>> removal?
>>
>> I thought about it, but when I saw that set_dirty were called only when
>> it was not already set as dirty where the call seams to be necessary.
>>
>> I just try to call xen_modified_mem only within
>> cpu_phy_mem_set_dirty_flags but it does not work, even when I tried to
>> clear the dirtybits. But I maybe don't do the right thing yet to clear
>> the dirty bits
>
> You can wrap the if (not dirty) make_it_dirty() sequence in a helper,
> and insert your hypercall in the helper, unconditionally.

Ok, I'll do that.

Thanks,

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Stefano Stabellini]

On Thu, 19 Jul 2012, Paolo Bonzini wrote:
> Il 17/07/2012 20:06, Stefano Stabellini ha scritto:
> > On Tue, 17 Jul 2012, Anthony PERARD wrote:
> >> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
> >> during migration.
> >>
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >> ---
> >>  exec.c   |    4 ++++
> >>  memory.c |    2 ++
> >>  2 files changed, 6 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/exec.c b/exec.c
> >> index c9fa17d..9f7a4f7 100644
> >> --- a/exec.c
> >> +++ b/exec.c
> >> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
> >>                      cpu_physical_memory_set_dirty_flags(
> >>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
> >>                  }
> >> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
> >>                  qemu_put_ram_ptr(ptr);
> >>              }
> >>          } else {
> >> @@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
> >>      if (buffer != bounce.buffer) {
> >>          if (is_write) {
> >>              ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
> >> +            xen_modified_memory(addr1, access_len);
> >>              while (access_len) {
> >>                  unsigned l;
> >>                  l = TARGET_PAGE_SIZE;
> > 
> > You need to add xen_modified_memory in cpu_physical_memory_map, rather
> > than cpu_physical_memory_unmap.
> 
> No, adding it to map is wrong, because the RAM save routine can migrate
> (and mark as non-dirty) the read buffers _before_ the device models have
> written to them.

You are correct, in fact this looks like a bug in the current qemu-xen
(non-upstream) codebase too!

What I think that we should do is only mark the memory as dirty
if(is_write) in cpu_physical_memory_unmap, like you are doing in this
patch.

Anthony, can you write a patch to change the behavior in
qemu-xen-traditional too?

Re: [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Stefano Stabellini]

On Thu, 19 Jul 2012, Paolo Bonzini wrote:
> Il 17/07/2012 20:06, Stefano Stabellini ha scritto:
> > On Tue, 17 Jul 2012, Anthony PERARD wrote:
> >> This patch add some calls to xen_modified_memory to notify Xen about dirtybits
> >> during migration.
> >>
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >> ---
> >>  exec.c   |    4 ++++
> >>  memory.c |    2 ++
> >>  2 files changed, 6 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/exec.c b/exec.c
> >> index c9fa17d..9f7a4f7 100644
> >> --- a/exec.c
> >> +++ b/exec.c
> >> @@ -3438,6 +3438,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
> >>                      cpu_physical_memory_set_dirty_flags(
> >>                          addr1, (0xff & ~CODE_DIRTY_FLAG));
> >>                  }
> >> +                xen_modified_memory(addr1, TARGET_PAGE_SIZE);
> >>                  qemu_put_ram_ptr(ptr);
> >>              }
> >>          } else {
> >> @@ -3623,6 +3624,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
> >>      if (buffer != bounce.buffer) {
> >>          if (is_write) {
> >>              ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
> >> +            xen_modified_memory(addr1, access_len);
> >>              while (access_len) {
> >>                  unsigned l;
> >>                  l = TARGET_PAGE_SIZE;
> > 
> > You need to add xen_modified_memory in cpu_physical_memory_map, rather
> > than cpu_physical_memory_unmap.
> 
> No, adding it to map is wrong, because the RAM save routine can migrate
> (and mark as non-dirty) the read buffers _before_ the device models have
> written to them.

You are correct, in fact this looks like a bug in the current qemu-xen
(non-upstream) codebase too!

What I think that we should do is only mark the memory as dirty
if(is_write) in cpu_physical_memory_unmap, like you are doing in this
patch.

Anthony, can you write a patch to change the behavior in
qemu-xen-traditional too?

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

On Thu, Jul 19, 2012 at 4:37 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
>
> Anthony, can you write a patch to change the behavior in
> qemu-xen-traditional too?

Yes.

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 3/4] exec, memory: Call to xen_modified_memory.

[Anthony PERARD]

On Thu, Jul 19, 2012 at 4:37 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
>
> Anthony, can you write a patch to change the behavior in
> qemu-xen-traditional too?

Yes.

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 4/4] xen: Always set the vram dirty during migration.

[Anthony PERARD]

On 17/07/12 19:30, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Stefano Stabellini wrote:
>> On Tue, 17 Jul 2012, Anthony PERARD wrote:
>>> Because the call to track the dirty bit in the video ram during migration won't
>>> work (it returns -1), we set dirtybit on the all video ram.
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>> ---
>>>   xen-all.c |    5 +++++
>>>   1 files changed, 5 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/xen-all.c b/xen-all.c
>>> index 498883b..00bdb50 100644
>>> --- a/xen-all.c
>>> +++ b/xen-all.c
>>> @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>>>           return;
>>>       }
>>>
>>> +    if (unlikely(xen_in_migration)) {
>>> +        /* track_dirty_vram does not work during migration */
>>> +        memory_region_set_dirty(framebuffer, 0, size);
>>> +        return;
>>> +    }
>>>       rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
>>>                                    start_addr >> TARGET_PAGE_BITS, npages,
>>>                                    bitmap);
>>
>> Why are you setting the entire framebuffer dirty?
>> We should set dirty only the actualy region that is supposed to be
>> dirty.

I set the dirty bit on the all framebuffer because the track dirty call 
fail, so I don't which bits are dirty. This one is for QEMU to know 
which part of the screen need to be updated.

> Also memory_region_set_dirty calls cpu_physical_memory_set_dirty_range,
> in which you didn't add a call to xen_modified_memory.



-- 
Anthony PERARD

Re: [PATCH 4/4] xen: Always set the vram dirty during migration.

[Anthony PERARD]

On 17/07/12 19:30, Stefano Stabellini wrote:
> On Tue, 17 Jul 2012, Stefano Stabellini wrote:
>> On Tue, 17 Jul 2012, Anthony PERARD wrote:
>>> Because the call to track the dirty bit in the video ram during migration won't
>>> work (it returns -1), we set dirtybit on the all video ram.
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>> ---
>>>   xen-all.c |    5 +++++
>>>   1 files changed, 5 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/xen-all.c b/xen-all.c
>>> index 498883b..00bdb50 100644
>>> --- a/xen-all.c
>>> +++ b/xen-all.c
>>> @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>>>           return;
>>>       }
>>>
>>> +    if (unlikely(xen_in_migration)) {
>>> +        /* track_dirty_vram does not work during migration */
>>> +        memory_region_set_dirty(framebuffer, 0, size);
>>> +        return;
>>> +    }
>>>       rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
>>>                                    start_addr >> TARGET_PAGE_BITS, npages,
>>>                                    bitmap);
>>
>> Why are you setting the entire framebuffer dirty?
>> We should set dirty only the actualy region that is supposed to be
>> dirty.

I set the dirty bit on the all framebuffer because the track dirty call 
fail, so I don't which bits are dirty. This one is for QEMU to know 
which part of the screen need to be updated.

> Also memory_region_set_dirty calls cpu_physical_memory_set_dirty_range,
> in which you didn't add a call to xen_modified_memory.



-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH 4/4] xen: Always set the vram dirty during migration.

[Stefano Stabellini]

On Fri, 20 Jul 2012, Anthony PERARD wrote:
> On 17/07/12 19:30, Stefano Stabellini wrote:
> > On Tue, 17 Jul 2012, Stefano Stabellini wrote:
> >> On Tue, 17 Jul 2012, Anthony PERARD wrote:
> >>> Because the call to track the dirty bit in the video ram during migration won't
> >>> work (it returns -1), we set dirtybit on the all video ram.
> >>>
> >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >>> ---
> >>>   xen-all.c |    5 +++++
> >>>   1 files changed, 5 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/xen-all.c b/xen-all.c
> >>> index 498883b..00bdb50 100644
> >>> --- a/xen-all.c
> >>> +++ b/xen-all.c
> >>> @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
> >>>           return;
> >>>       }
> >>>
> >>> +    if (unlikely(xen_in_migration)) {
> >>> +        /* track_dirty_vram does not work during migration */
> >>> +        memory_region_set_dirty(framebuffer, 0, size);
> >>> +        return;
> >>> +    }
> >>>       rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
> >>>                                    start_addr >> TARGET_PAGE_BITS, npages,
> >>>                                    bitmap);
> >>
> >> Why are you setting the entire framebuffer dirty?
> >> We should set dirty only the actualy region that is supposed to be
> >> dirty.
> 
> I set the dirty bit on the all framebuffer because the track dirty call 
> fail, so I don't which bits are dirty. This one is for QEMU to know 
> which part of the screen need to be updated.

In that case it would be better to set the entire bitmap to 0xff in case
xc_hvm_track_dirty_vram fails, right?
So that we don't need to special case live migration.

Re: [PATCH 4/4] xen: Always set the vram dirty during migration.

[Stefano Stabellini]

On Fri, 20 Jul 2012, Anthony PERARD wrote:
> On 17/07/12 19:30, Stefano Stabellini wrote:
> > On Tue, 17 Jul 2012, Stefano Stabellini wrote:
> >> On Tue, 17 Jul 2012, Anthony PERARD wrote:
> >>> Because the call to track the dirty bit in the video ram during migration won't
> >>> work (it returns -1), we set dirtybit on the all video ram.
> >>>
> >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >>> ---
> >>>   xen-all.c |    5 +++++
> >>>   1 files changed, 5 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/xen-all.c b/xen-all.c
> >>> index 498883b..00bdb50 100644
> >>> --- a/xen-all.c
> >>> +++ b/xen-all.c
> >>> @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
> >>>           return;
> >>>       }
> >>>
> >>> +    if (unlikely(xen_in_migration)) {
> >>> +        /* track_dirty_vram does not work during migration */
> >>> +        memory_region_set_dirty(framebuffer, 0, size);
> >>> +        return;
> >>> +    }
> >>>       rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
> >>>                                    start_addr >> TARGET_PAGE_BITS, npages,
> >>>                                    bitmap);
> >>
> >> Why are you setting the entire framebuffer dirty?
> >> We should set dirty only the actualy region that is supposed to be
> >> dirty.
> 
> I set the dirty bit on the all framebuffer because the track dirty call 
> fail, so I don't which bits are dirty. This one is for QEMU to know 
> which part of the screen need to be updated.

In that case it would be better to set the entire bitmap to 0xff in case
xc_hvm_track_dirty_vram fails, right?
So that we don't need to special case live migration.