Re: [Xen-devel] [PATCH v6 COLO 03/15] primary vm suspend/get_dirty_pfn/resume/checkpoint code

2015-06-16 Thread Ian Campbell
On Mon, 2015-06-08 at 11:45 +0800, Yang Hongyang wrote:
 diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
 index 86bcf9c..d5902a6 100644
 --- a/tools/libxc/include/xenguest.h
 +++ b/tools/libxc/include/xenguest.h
 @@ -75,6 +75,18 @@ struct save_callbacks {
   */
  int (*toolstack_save)(uint32_t domid, uint8_t **buf, uint32_t *len, void 
 *data);
  
 +/* Called after the guest is suspended.
 + *
 + * returns the list of dirty pfn:
 + *  struct {
 + *  uint64_t count;
 + *  uint64_t pfn[];
 + *  };

Seeing this comment and then a callback which returns a uint8_t* makes
me suspicious. Can we not do something a bit more typesafe here, like
returning a pointer to a suitable struct?

 + *
 + *  Note: the caller must free the return value.
 + */
 +uint8_t *(*get_dirty_pfn)(void *data);
 +
  /* to be provided as the last argument to each callback function */
  void* data;
  };
 diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
 index 10d3d82..1145ae4 100644
 --- a/tools/libxl/libxl.c
 +++ b/tools/libxl/libxl.c
 @@ -17,6 +17,7 @@
  #include libxl_osdeps.h
  
  #include libxl_internal.h
 +#include libxl_colo.h
  
  #define PAGE_TO_MEMKB(pages) ((pages) * 4)
  #define BACKEND_STRING_SIZE 5
 @@ -841,7 +842,10 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
 libxl_domain_remus_info *info,
  assert(info);
  
  /* Point of no return */
 -libxl__remus_setup(egc, dss-rs);
 +if (libxl_defbool_val(info-colo))

libxl code must arrange to have called libxl_defbool_setdefault before
using libxl_defbool_val, which I don't see here. There is a big block of
such settings near the top of this function which you should add to.

On the other hand -- is it possible for a caller to say they don't care
what kind of check pointing they want and have libxl decide? If not then
it doesn't make sense to use a defbool, a regular bool would be
appropriate.

I'm also wondering to what extent COLO could be considered an extension
to Remus, as opposed to an alternative -- iow I'm unsure if reusing
libxl_domain_remus_start as the API makes sense (the implementation
could still be shared where appropriate).

 diff --git a/tools/libxl/libxl_colo.h b/tools/libxl/libxl_colo.h
 index 91df275..26a2563 100644
 --- a/tools/libxl/libxl_colo.h
 +++ b/tools/libxl/libxl_colo.h
 @@ -35,4 +35,14 @@ extern void libxl__colo_restore_teardown(libxl__egc *egc,
   libxl__colo_restore_state *crs,
   int rc);
  
 +extern void libxl__colo_save_domain_suspend_callback(void *data);
 +extern void libxl__colo_save_domain_resume_callback(void *data);
 +extern void libxl__colo_save_domain_checkpoint_callback(void *data);
 +extern void libxl__colo_save_get_dirty_pfn_callback(void *data);
 +extern void libxl__colo_save_setup(libxl__egc *egc,
 +   libxl__colo_save_state *css);
 +extern void libxl__colo_save_teardown(libxl__egc *egc,
 +  libxl__colo_save_state *css,
 +  int rc);

Should all be marked _hidden I think?

[...]


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


[Xen-devel] [PATCH v6 COLO 03/15] primary vm suspend/get_dirty_pfn/resume/checkpoint code

2015-06-07 Thread Yang Hongyang
From: Wen Congyang we...@cn.fujitsu.com

We will do the following things again and again:
1. Suspend primary vm
   a. Suspend primary vm
   b. do postsuspend
   c. Read LIBXL_COLO_SVM_SUSPENDED sent by secondary
   d. Read secondary vm's dirty page information to master(count + pfn list)
2. Get dirty pfn list callback, used by libxc
   a. Return secondary vm's dirty pfn list
3. Resume primary vm
   a. Read LIBXL_COLO_SVM_READY from slave
   b. Do presume
   c. Resume primary vm
   d. Read LIBXL_COLO_SVM_RESUMED from slave
4. Wait a new checkpoint
   a. Wait a new checkpoint(not implemented)
   b. Send LIBXL_COLO_NEW_CHECKPOINT to slave

Signed-off-by: Wen Congyang we...@cn.fujitsu.com
Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
---
 tools/libxc/include/xenguest.h |  12 +
 tools/libxl/Makefile   |   2 +-
 tools/libxl/libxl.c|   6 +-
 tools/libxl/libxl_colo.h   |  10 +
 tools/libxl/libxl_colo_save.c  | 643 +
 tools/libxl/libxl_dom_save.c   |  15 +-
 tools/libxl/libxl_internal.h   |  31 +-
 tools/libxl/libxl_save_msgs_gen.pl |   1 +
 tools/libxl/libxl_types.idl|   1 +
 9 files changed, 712 insertions(+), 9 deletions(-)
 create mode 100644 tools/libxl/libxl_colo_save.c

diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index 86bcf9c..d5902a6 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -75,6 +75,18 @@ struct save_callbacks {
  */
 int (*toolstack_save)(uint32_t domid, uint8_t **buf, uint32_t *len, void 
*data);
 
+/* Called after the guest is suspended.
+ *
+ * returns the list of dirty pfn:
+ *  struct {
+ *  uint64_t count;
+ *  uint64_t pfn[];
+ *  };
+ *
+ *  Note: the caller must free the return value.
+ */
+uint8_t *(*get_dirty_pfn)(void *data);
+
 /* to be provided as the last argument to each callback function */
 void* data;
 };
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 82cc4c2..88c5426 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -57,7 +57,7 @@ LIBXL_OBJS-y += libxl_nonetbuffer.o
 endif
 
 LIBXL_OBJS-y += libxl_remus.o libxl_checkpoint_device.o libxl_remus_disk_drbd.o
-LIBXL_OBJS-y += libxl_colo_restore.o
+LIBXL_OBJS-y += libxl_colo_restore.o libxl_colo_save.o
 
 LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
 LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 10d3d82..1145ae4 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -17,6 +17,7 @@
 #include libxl_osdeps.h
 
 #include libxl_internal.h
+#include libxl_colo.h
 
 #define PAGE_TO_MEMKB(pages) ((pages) * 4)
 #define BACKEND_STRING_SIZE 5
@@ -841,7 +842,10 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
libxl_domain_remus_info *info,
 assert(info);
 
 /* Point of no return */
-libxl__remus_setup(egc, dss-rs);
+if (libxl_defbool_val(info-colo))
+libxl__colo_save_setup(egc, dss-css);
+else
+libxl__remus_setup(egc, dss-rs);
 return AO_INPROGRESS;
 
  out:
diff --git a/tools/libxl/libxl_colo.h b/tools/libxl/libxl_colo.h
index 91df275..26a2563 100644
--- a/tools/libxl/libxl_colo.h
+++ b/tools/libxl/libxl_colo.h
@@ -35,4 +35,14 @@ extern void libxl__colo_restore_teardown(libxl__egc *egc,
  libxl__colo_restore_state *crs,
  int rc);
 
+extern void libxl__colo_save_domain_suspend_callback(void *data);
+extern void libxl__colo_save_domain_resume_callback(void *data);
+extern void libxl__colo_save_domain_checkpoint_callback(void *data);
+extern void libxl__colo_save_get_dirty_pfn_callback(void *data);
+extern void libxl__colo_save_setup(libxl__egc *egc,
+   libxl__colo_save_state *css);
+extern void libxl__colo_save_teardown(libxl__egc *egc,
+  libxl__colo_save_state *css,
+  int rc);
+
 #endif
diff --git a/tools/libxl/libxl_colo_save.c b/tools/libxl/libxl_colo_save.c
new file mode 100644
index 000..153ec57
--- /dev/null
+++ b/tools/libxl/libxl_colo_save.c
@@ -0,0 +1,643 @@
+/*
+ * Copyright (C) 2014 FUJITSU LIMITED
+ * Author: Wen Congyang we...@cn.fujitsu.com
+ * Yang Hongyang yan...@cn.fujitsu.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * 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 Lesser General Public License for more details.
+ */
+