Re: [Xen-devel] [PATCH v2 6/6] libxl/save: Refactor libxl__domain_suspend_state

2015-06-16 Thread Ian Campbell 
On Wed, 2015-06-03 at 16:01 +0800, Yang Hongyang wrote:
 Currently struct libxl__domain_suspend_state contains 2 type of states,
 one is save state, another is suspend state. This patch separate it out.

This patch separates those two out.

 The motivation of this is that COLO will need to do suspend/resume
 continuesly, we need a more common suspend state.

continuously

 After this change, dss stands for libxl__domain_save_state,
 dsps stands for libxl__domain_suspend_state.
 
 Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
 CC: Ian Campbell ian.campb...@citrix.com
 CC: Ian Jackson ian.jack...@eu.citrix.com
 CC: Wei Liu wei.l...@citrix.com
 CC: Andrew Cooper andrew.coop...@citrix.com

I presume this is almost entirely mechanical and the compiler mostly
took care of identifying everywhere to be changed:

Acked-by: Ian Campbell ian.campb...@citrix.com

[...]
 @@ -2839,9 +2840,27 @@ typedef struct libxl__logdirty_switch {
  } libxl__logdirty_switch;
  
  struct libxl__domain_suspend_state {
[...]
 +struct libxl__domain_save_state {

Please could you add a little comment before each of these explaining
their scope for the hard of thinking (i.e. me). One is the live phase
and one is the final stop-and-copy phase, I think.

Ian.



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

Re: [Xen-devel] [PATCH v2 6/6] libxl/save: Refactor libxl__domain_suspend_state

2015-06-16 Thread Ian Jackson 
Yang Hongyang writes ([PATCH v2 6/6] libxl/save: Refactor 
libxl__domain_suspend_state):
 Currently struct libxl__domain_suspend_state contains 2 type of states,
 one is save state, another is suspend state. This patch separate it out.
 The motivation of this is that COLO will need to do suspend/resume
 continuesly, we need a more common suspend state.

Currently in libxl/libxc/etc.  suspend and save have referred to
the same thing, but different terminology has been used at different
layers.  Ie both suspend and save each refer to either or both of
save to disk or suspend for live migrate, or to the relevant
underlying mechanisms.

So I'm not sure introducing a distinction between those two terms in
libxl is really helpful.  If it is to be done there should be a clear
explanation of what the difference is.

On IRC you said:

14:21 yanghy Diziet, currently, in libxl, suspend is used as 2
   means, one is save(corrspond to libxc save), another is
   suspend the guest(related to suspend callback)

That's rather different, I think.  Or, at least, I'm not sure that I
understand this distinction as you are making it.  The suspend
callback is part of the implementation of what at higher layers we
save/suspend/restore/migration.

AIUI this callback is related to pausing the guest, or manipulating
its VCPUs ?  Perhaps we should rename this callback ?  Maybe Andrew
Cooper can suggest a name ?

Forgive me if I'm confused and going off in the wrong direction...

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH v2 6/6] libxl/save: Refactor libxl__domain_suspend_state

2015-06-16 Thread Andrew Cooper 
On 16/06/15 14:26, Ian Jackson wrote:
 Yang Hongyang writes ([PATCH v2 6/6] libxl/save: Refactor 
 libxl__domain_suspend_state):
 Currently struct libxl__domain_suspend_state contains 2 type of states,
 one is save state, another is suspend state. This patch separate it out.
 The motivation of this is that COLO will need to do suspend/resume
 continuesly, we need a more common suspend state.
 Currently in libxl/libxc/etc.  suspend and save have referred to
 the same thing, but different terminology has been used at different
 layers.  Ie both suspend and save each refer to either or both of
 save to disk or suspend for live migrate, or to the relevant
 underlying mechanisms.

 So I'm not sure introducing a distinction between those two terms in
 libxl is really helpful.  If it is to be done there should be a clear
 explanation of what the difference is.

 On IRC you said:

 14:21 yanghy Diziet, currently, in libxl, suspend is used as 2
means, one is save(corrspond to libxc save), another is
suspend the guest(related to suspend callback)

 That's rather different, I think.  Or, at least, I'm not sure that I
 understand this distinction as you are making it.  The suspend
 callback is part of the implementation of what at higher layers we
 save/suspend/restore/migration.

 AIUI this callback is related to pausing the guest, or manipulating
 its VCPUs ?  Perhaps we should rename this callback ?  Maybe Andrew
 Cooper can suggest a name ?

 Forgive me if I'm confused and going off in the wrong direction...

The terminology used by libxc is more consistent in this area.

suspend refers to quiescing the VM, so pausing qemu, making a
remote_shutdown(SHUTDOWN_suspend) hypercall etc.

save refers to the actions involved in actually shuffling the state of
the VM, so xc_domain_save() etc.

I believe that these are useful distinctions to maintain.  libxl
currently uses suspend to encapsulate both.

~Andrew

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

[Xen-devel] [PATCH v2 6/6] libxl/save: Refactor libxl__domain_suspend_state

2015-06-03 Thread Yang Hongyang 
Currently struct libxl__domain_suspend_state contains 2 type of states,
one is save state, another is suspend state. This patch separate it out.
The motivation of this is that COLO will need to do suspend/resume
continuesly, we need a more common suspend state.

After this change, dss stands for libxl__domain_save_state,
dsps stands for libxl__domain_suspend_state.

Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
CC: Ian Campbell ian.campb...@citrix.com
CC: Ian Jackson ian.jack...@eu.citrix.com
CC: Wei Liu wei.l...@citrix.com
CC: Andrew Cooper andrew.coop...@citrix.com
---
 tools/libxl/libxl.c  |  10 +--
 tools/libxl/libxl_dom_save.c |  76 ++
 tools/libxl/libxl_dom_suspend.c  | 166 ---
 tools/libxl/libxl_internal.h |  51 +++-
 tools/libxl/libxl_netbuffer.c|   2 +-
 tools/libxl/libxl_remus.c|  39 -
 tools/libxl/libxl_save_callout.c |   2 +-
 7 files changed, 185 insertions(+), 161 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0f9248e..ba2da92 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -793,7 +793,7 @@ out:
 }
 
 static void remus_failover_cb(libxl__egc *egc,
-  libxl__domain_suspend_state *dss, int rc);
+  libxl__domain_save_state *dss, int rc);
 
 /* TODO: Explicit Checkpoint acknowledgements via recv_fd. */
 int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
@@ -801,7 +801,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
libxl_domain_remus_info *info,
  const libxl_asyncop_how *ao_how)
 {
 AO_CREATE(ctx, domid, ao_how);
-libxl__domain_suspend_state *dss;
+libxl__domain_save_state *dss;
 int rc;
 
 libxl_domain_type type = libxl__domain_type(gc, domid);
@@ -849,7 +849,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
libxl_domain_remus_info *info,
 }
 
 static void remus_failover_cb(libxl__egc *egc,
-  libxl__domain_suspend_state *dss, int rc)
+  libxl__domain_save_state *dss, int rc)
 {
 STATE_AO_GC(dss-ao);
 /*
@@ -861,7 +861,7 @@ static void remus_failover_cb(libxl__egc *egc,
 }
 
 static void domain_suspend_cb(libxl__egc *egc,
-  libxl__domain_suspend_state *dss, int rc)
+  libxl__domain_save_state *dss, int rc)
 {
 STATE_AO_GC(dss-ao);
 libxl__ao_complete(egc,ao,rc);
@@ -880,7 +880,7 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, 
int fd, int flags,
 goto out_err;
 }
 
-libxl__domain_suspend_state *dss;
+libxl__domain_save_state *dss;
 GCNEW(dss);
 
 dss-ao = ao;
diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
index 2081892..74a6bae 100644
--- a/tools/libxl/libxl_dom_save.c
+++ b/tools/libxl/libxl_dom_save.c
@@ -44,7 +44,7 @@ static void switch_logdirty_timeout(libxl__egc *egc, 
libxl__ev_time *ev,
 static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch*,
 const char *watch_path, const char *event_path);
 static void switch_logdirty_done(libxl__egc *egc,
- libxl__domain_suspend_state *dss, int ok);
+ libxl__domain_save_state *dss, int ok);
 
 static void logdirty_init(libxl__logdirty_switch *lds)
 {
@@ -58,7 +58,7 @@ static void 
domain_suspend_switch_qemu_xen_traditional_logdirty
 libxl__save_helper_state *shs)
 {
 libxl__egc *egc = shs-egc;
-libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+libxl__domain_save_state *dss = CONTAINER_OF(shs, *dss, shs);
 libxl__logdirty_switch *lds = dss-logdirty;
 STATE_AO_GC(dss-ao);
 int rc;
@@ -130,7 +130,7 @@ static void domain_suspend_switch_qemu_xen_logdirty
 libxl__save_helper_state *shs)
 {
 libxl__egc *egc = shs-egc;
-libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+libxl__domain_save_state *dss = CONTAINER_OF(shs, *dss, shs);
 STATE_AO_GC(dss-ao);
 int rc;
 
@@ -148,7 +148,7 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
 {
 libxl__save_helper_state *shs = user;
 libxl__egc *egc = shs-egc;
-libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+libxl__domain_save_state *dss = CONTAINER_OF(shs, *dss, shs);
 STATE_AO_GC(dss-ao);
 
 switch (libxl__device_model_version_running(gc, domid)) {
@@ -167,7 +167,7 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
 static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
 const struct timeval *requested_abs)
 {
-libxl__domain_suspend_state *dss = CONTAINER_OF(ev, *dss, 
logdirty.timeout);
+libxl__domain_save_state *dss = CONTAINER_OF(ev, *dss, logdirty.timeout);
 STATE_AO_GC(dss-ao);