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

[Qemu-devel] [PATCH V2 0/5] 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.

Change since the first patch set:
  - New patch to set dirty if not, in exec.c
    => only one place to add the xen call in exec.c


Thanks for your reviews,


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

 exec.c           |   43 +++++++++++++++----------------------------
 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, 105 insertions(+), 28 deletions(-)

-- 
Anthony PERARD

[Qemu-devel] [PATCH V2 1/5] QMP, Introduce xen-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 a92adb1..e521ea4 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 e3cf3c5..961f5c0 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 61def2e..e9c7ebf 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,
@@ -588,6 +594,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 V2 2/5] 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 e9c7ebf..54e54cb 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -1220,3 +1220,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 V2 3/5] exec: Introduce helper to set dirty flags.

[Anthony PERARD]

This new helper/hook is used in the next patch to add an extra call in a single
place.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 exec.c |   42 ++++++++++++++----------------------------
 1 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/exec.c b/exec.c
index feb4795..b24a03a 100644
--- a/exec.c
+++ b/exec.c
@@ -3386,6 +3386,16 @@ int cpu_memory_rw_debug(CPUArchState *env, target_ulong addr,
 }
 
 #else
+static void set_dirty_helper(target_phys_addr_t addr, target_phys_addr_t length)
+{
+    if (!cpu_physical_memory_is_dirty(addr)) {
+        /* invalidate code */
+        tb_invalidate_phys_page_range(addr, addr + length, 0);
+        /* set dirty bit */
+        cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
+    }
+}
+
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write)
 {
@@ -3431,13 +3441,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                 /* RAM case */
                 ptr = qemu_get_ram_ptr(addr1);
                 memcpy(ptr, buf, l);
-                if (!cpu_physical_memory_is_dirty(addr1)) {
-                    /* invalidate code */
-                    tb_invalidate_phys_page_range(addr1, addr1 + l, 0);
-                    /* set dirty bit */
-                    cpu_physical_memory_set_dirty_flags(
-                        addr1, (0xff & ~CODE_DIRTY_FLAG));
-                }
+                set_dirty_helper(addr1, l);
                 qemu_put_ram_ptr(ptr);
             }
         } else {
@@ -3628,13 +3632,7 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
                 l = TARGET_PAGE_SIZE;
                 if (l > access_len)
                     l = access_len;
-                if (!cpu_physical_memory_is_dirty(addr1)) {
-                    /* invalidate code */
-                    tb_invalidate_phys_page_range(addr1, addr1 + l, 0);
-                    /* set dirty bit */
-                    cpu_physical_memory_set_dirty_flags(
-                        addr1, (0xff & ~CODE_DIRTY_FLAG));
-                }
+                set_dirty_helper(addr1, l);
                 addr1 += l;
                 access_len -= l;
             }
@@ -3940,13 +3938,7 @@ static inline void stl_phys_internal(target_phys_addr_t addr, uint32_t val,
             stl_p(ptr, val);
             break;
         }
-        if (!cpu_physical_memory_is_dirty(addr1)) {
-            /* invalidate code */
-            tb_invalidate_phys_page_range(addr1, addr1 + 4, 0);
-            /* set dirty bit */
-            cpu_physical_memory_set_dirty_flags(addr1,
-                (0xff & ~CODE_DIRTY_FLAG));
-        }
+        set_dirty_helper(addr1, 4);
     }
 }
 
@@ -4013,13 +4005,7 @@ static inline void stw_phys_internal(target_phys_addr_t addr, uint32_t val,
             stw_p(ptr, val);
             break;
         }
-        if (!cpu_physical_memory_is_dirty(addr1)) {
-            /* invalidate code */
-            tb_invalidate_phys_page_range(addr1, addr1 + 2, 0);
-            /* set dirty bit */
-            cpu_physical_memory_set_dirty_flags(addr1,
-                (0xff & ~CODE_DIRTY_FLAG));
-        }
+        set_dirty_helper(addr1, 2);
     }
 }
 
-- 
Anthony PERARD

[Qemu-devel] [PATCH V2 4/5] 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   |    1 +
 memory.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/exec.c b/exec.c
index b24a03a..84f9557 100644
--- a/exec.c
+++ b/exec.c
@@ -3394,6 +3394,7 @@ static void set_dirty_helper(target_phys_addr_t addr, target_phys_addr_t length)
         /* set dirty bit */
         cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
     }
+    xen_modified_memory(addr, length);
 }
 
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
diff --git a/memory.c b/memory.c
index 643871b..1b91330 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 V2 5/5] 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 54e54cb..207182e 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 V2 1/5] QMP, Introduce xen-set-global-dirty-log command.

[Luiz Capitulino]

On Fri, 20 Jul 2012 15:34:38 +0100
Anthony PERARD <anthony.perard@citrix.com> 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.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.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 a92adb1..e521ea4 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 e3cf3c5..961f5c0 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 61def2e..e9c7ebf 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,
> @@ -588,6 +594,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)
> +{
> +}

Re: [Qemu-devel] [PATCH V2 3/5] exec: Introduce helper to set dirty flags.

[Avi Kivity]

On 07/20/2012 05:34 PM, Anthony PERARD wrote:
> This new helper/hook is used in the next patch to add an extra call in a single
> place.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  exec.c |   42 ++++++++++++++----------------------------
>  1 files changed, 14 insertions(+), 28 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index feb4795..b24a03a 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3386,6 +3386,16 @@ int cpu_memory_rw_debug(CPUArchState *env, target_ulong addr,
>  }
>  
>  #else

<blank line>

> +static void set_dirty_helper(target_phys_addr_t addr, target_phys_addr_t length)

More meaningful name please.  Practically every function can be name
something_helper().

> +{
> +    if (!cpu_physical_memory_is_dirty(addr)) {
> +        /* invalidate code */
> +        tb_invalidate_phys_page_range(addr, addr + length, 0);
> +        /* set dirty bit */
> +        cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
> +    }
> +}
> +

Otherwise looks good; a nice cleanup in its own right.



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