Xen devel - Jan 2013 - [PATCH] tools: set migration constraints from cmdline

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1359394320 -3600
# Node ID 577b051fca174be9f7b3d6590c62735cdf214021
# Parent  6727070b4129cf852199b66b6a81042ee6966a98
tools: set migration constraints from cmdline

Add new options to xm/xl migrate to control the process of migration.
The intention is to optionally abort the migration if it takes too long
to migrate a busy guest due to the high number of dirty pages. Currently
the guest is suspended to transfer the remaining dirty pages. This
transfer can take too long, which can confuse the guest if its suspended
for too long.

-M <number>   Number of iterations before final suspend (default: 30)
--max_iters <number>

-m <factor>   Max amount of memory to transfer before final suspend
(default: 3*RAM)
--max_factor <factor>

-N            Abort migration instead of doing final suspend.
--no_suspend


A variant of this change has been tested with xend, the patch below is
only compile tested. The changes to libxl change the API, is that
approach acceptable?

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 6727070b4129 -r 577b051fca17 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -43,6 +43,8 @@
 */
 #define DEF_MAX_ITERS   29   /* limit us to 30 times round loop   */
 #define DEF_MAX_FACTOR   3   /* never send more than 3x p2m_size  */
+/* Suspend guest for final transit once number of dirty pages is that low */
+#define DEF_MIN_REMAINING 50
 
 struct save_ctx {
     unsigned long hvirt_start; /* virtual starting address of the hypervisor */
@@ -804,9 +806,11 @@ int xc_domain_save(xc_interface *xch, in
     int rc = 1, frc, i, j, last_iter = 0, iter = 0;
     int live  = (flags & XCFLAGS_LIVE);
     int debug = (flags & XCFLAGS_DEBUG);
+    int nosuspend = (flags & XCFLAGS_DOMSAVE_NOSUSPEND);
     int superpages = !!hvm;
     int race = 0, sent_last_iter, skip_this_iter = 0;
     unsigned int sent_this_iter = 0;
+    int min_reached;
     int tmem_saved = 0;
 
     /* The new domain''s shared-info frame number. */
@@ -1525,10 +1529,20 @@ int xc_domain_save(xc_interface *xch, in
 
         if ( live )
         {
+            min_reached = sent_this_iter + skip_this_iter <
DEF_MIN_REMAINING;
             if ( (iter >= max_iters) ||
-                 (sent_this_iter+skip_this_iter < 50) ||
+                 min_reached ||
                  (total_sent > dinfo->p2m_size*max_factor) )
             {
+                if ( min_reached && nosuspend )
+                {
+                    ERROR("Live migration aborted, as requested. (guest
too busy?)"
+                     " total_sent %lu iter %d, max_iters %u max_factor
%u",
+                      total_sent, iter, max_iters, max_factor);
+                    rc = 1;
+                    goto out;
+                }
+
                 DPRINTF("Start last iteration\n");
                 last_iter = 1;
 
diff -r 6727070b4129 -r 577b051fca17 tools/libxc/xenguest.h
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -28,6 +28,8 @@
 #define XCFLAGS_HVM       4
 #define XCFLAGS_STDVGA    8
 #define XCFLAGS_CHECKPOINT_COMPRESS    16
+#define XCFLAGS_DOMSAVE_NOSUSPEND      32
+
 #define X86_64_B_SIZE   64 
 #define X86_32_B_SIZE   32
 
diff -r 6727070b4129 -r 577b051fca17 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -757,6 +757,7 @@ static void domain_suspend_cb(libxl__egc
 }
 
 int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
+                         int max_iters, int max_factor,
                          const libxl_asyncop_how *ao_how)
 {
     AO_CREATE(ctx, domid, ao_how);
@@ -779,6 +780,9 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     dss->type = type;
     dss->live = flags & LIBXL_SUSPEND_LIVE;
     dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+    dss->max_iters = max_iters;
+    dss->max_factor = max_factor;
+    dss->xlflags = flags;
 
     libxl__domain_suspend(egc, dss);
     return AO_INPROGRESS;
diff -r 6727070b4129 -r 577b051fca17 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -502,10 +502,12 @@ void libxl_domain_config_dispose(libxl_d
 
 int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
                          int flags, /* LIBXL_SUSPEND_* */
+                         int max_iters, int max_factor,
                          const libxl_asyncop_how *ao_how)
                          LIBXL_EXTERNAL_CALLERS_ONLY;
 #define LIBXL_SUSPEND_DEBUG 1
 #define LIBXL_SUSPEND_LIVE 2
+#define LIBXL_SUSPEND_NO_FINAL_SUSPEND 4
 
 /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
  *   If this parameter is true, use co-operative resume. The guest
diff -r 6727070b4129 -r 577b051fca17 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
 
     dss->xcflags = (live) ? XCFLAGS_LIVE : 0
           | (debug) ? XCFLAGS_DEBUG : 0
+          | (dss->xlflags & LIBXL_SUSPEND_NO_FINAL_SUSPEND) ?
XCFLAGS_DOMSAVE_NOSUSPEND : 0
           | (dss->hvm) ? XCFLAGS_HVM : 0;
 
     dss->suspend_eventchn = -1;
diff -r 6727070b4129 -r 577b051fca17 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2253,6 +2253,9 @@ struct libxl__domain_suspend_state {
     xc_evtchn *xce; /* event channel handle */
     int suspend_eventchn;
     int hvm;
+    int max_iters;
+    int max_factor;
+    int xlflags;
     int xcflags;
     int guest_responded;
     const char *dm_savefile;
diff -r 6727070b4129 -r 577b051fca17 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
 
     save_domain_core_writeconfig(fd, filename, config_data, config_len);
 
-    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
+    int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0, NULL);
     close(fd);
 
     if (rc < 0)
@@ -3332,6 +3332,8 @@ static void migrate_do_preamble(int send
 }
 
 static void migrate_domain(uint32_t domid, const char *rune,
+                           int max_iters, int max_factor,
+                           int no_suspend,
                            const char *override_config_file)
 {
     pid_t child = -1;
@@ -3341,6 +3343,7 @@ static void migrate_domain(uint32_t domi
     char rc_buf;
     uint8_t *config_data;
     int config_len;
+    int flags = LIBXL_SUSPEND_LIVE;
 
     save_domain_core_begin(domid, override_config_file,
                            &config_data, &config_len);
@@ -3358,7 +3361,10 @@ static void migrate_domain(uint32_t domi
 
     xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS, 0);
 
-    rc = libxl_domain_suspend(ctx, domid, send_fd, LIBXL_SUSPEND_LIVE, NULL);
+    if (no_suspend)
+        flags |= LIBXL_SUSPEND_NO_FINAL_SUSPEND;
+
+    rc = libxl_domain_suspend(ctx, domid, send_fd, flags, max_iters,
max_factor, NULL);
     if (rc) {
         fprintf(stderr, "migration sender: libxl_domain_suspend
failed"
                 " (rc=%d)\n", rc);
@@ -3751,8 +3757,9 @@ int main_migrate(int argc, char **argv)
     char *rune = NULL;
     char *host;
     int opt, daemonize = 1, monitor = 1, debug = 0;
-
-    SWITCH_FOREACH_OPT(opt, "FC:s:ed", NULL, "migrate", 2)
{
+    int max_iters = 0, max_factor = 0, no_suspend = 0;
+
+    SWITCH_FOREACH_OPT(opt, "FC:s:edM:m:N", NULL,
"migrate", 2) {
     case ''C'':
         config_filename = optarg;
         break;
@@ -3769,6 +3776,15 @@ int main_migrate(int argc, char **argv)
     case ''d'':
         debug = 1;
         break;
+    case ''M'':
+        max_iters = atoi(optarg);
+        break;
+    case ''m'':
+        max_factor = atoi(optarg);
+        break;
+    case ''N'':
+        no_suspend = 1;
+        break;
     }
 
     domid = find_domain(argv[optind]);
@@ -3784,7 +3800,7 @@ int main_migrate(int argc, char **argv)
             return 1;
     }
 
-    migrate_domain(domid, rune, config_filename);
+    migrate_domain(domid, rune, max_iters, max_factor, no_suspend,
config_filename);
     return 0;
 }
 
diff -r 6727070b4129 -r 577b051fca17 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -152,6 +152,9 @@ struct cmd_spec cmd_table[] = {
       "-s <sshcommand> Use <sshcommand> instead of ssh. 
String will be passed\n"
       "                to sh. If empty, run <host> instead of ssh
<host> xl\n"
       "                migrate-receive [-d -e]\n"
+      "-M --max_iters <number>  Number of iterations before final
suspend (default: 30)\n"
+      "-m --max_factor <factor> Max amount of memory to transfer
before final suspend (default: 3*RAM).\n"
+      "-N --no_suspend Abort migration instead of doing final
suspend.\n"
       "-e              Do not wait in the background (on <host>) for
the death\n"
       "                of the domain."
     },
diff -r 6727070b4129 -r 577b051fca17 tools/python/xen/xend/XendCheckpoint.py
--- a/tools/python/xen/xend/XendCheckpoint.py
+++ b/tools/python/xen/xend/XendCheckpoint.py
@@ -118,9 +118,19 @@ def save(fd, dominfo, network, live, dst
         # enabled. Passing "0" simply uses the defaults compiled into
         # libxenguest; see the comments and/or code in xc_linux_save() for
         # more information.
+        max_iters = dominfo.info.get(''max_iters'',
"0")
+        max_factor = dominfo.info.get(''max_factor'',
"0")
+        no_suspend = dominfo.info.get(''no_suspend'', None)
+        if max_iters == "None":
+            max_iters = "0"
+        if max_factor == "None":
+            max_factor = "0"
+        if no_suspend == "None":
+            no_suspend = "0"
         cmd = [xen.util.auxbin.pathTo(XC_SAVE), str(fd),
-               str(dominfo.getDomid()), "0", "0", 
-               str(int(live) | (int(hvm) << 2)) ]
+               str(dominfo.getDomid()),
+               max_iters, max_factor,
+               str(int(live) | (int(hvm) << 2) | (int(no_suspend)
<< 5) ]
         log.debug("[xc_save]: %s", string.join(cmd))
 
         def saveInputHandler(line, tochild):
diff -r 6727070b4129 -r 577b051fca17 tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py
+++ b/tools/python/xen/xend/XendDomain.py
@@ -1832,6 +1832,18 @@ class XendDomain:
             log.exception(ex)
             raise XendError(str(ex))
 
+    def domain_migrate_constraints_set((self, domid, max_iters, max_factor,
no_suspend):
+        """Set the Migrate Constraints of this domain.
+        @param domid: Domain ID or Name
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param no_suspend: Abort migration instead of doing final suspend
+        """
+        dominfo = self.domain_lookup_nr(domid)
+        if not dominfo:
+            raise XendInvalidDomain(str(domid))
+        dominfo.setMigrateConstraints(max_iters, max_factor, no_suspend)
+
     def domain_maxmem_set(self, domid, mem):
         """Set the memory limit for a domain.
 
diff -r 6727070b4129 -r 577b051fca17 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -1459,6 +1459,18 @@ class XendDomainInfo:
         pci_conf = self.info[''devices''][dev_uuid][1]
         return map(pci_dict_to_bdf_str, pci_conf[''devs''])
 
+    def setMigrateConstraints(self, max_iters, max_factor, no_suspend):
+        """Set the Migrate Constraints of this domain.
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param no_suspend: Abort migration instead of doing final suspend
+        """
+        log.debug("Setting setMigrateConstraints of domain %s (%s) to
''%s'' ''%s'' ''%s''.",
+                  self.info[''name_label''], str(self.domid),
max_iters, max_factor, no_suspend)
+        self.info[''max_iters''] = str(max_iters)
+        self.info[''max_factor''] = str(max_factor)
+        self.info[''no_suspend''] = str(no_suspend)
+
     def setMemoryTarget(self, target):
         """Set the memory target of this domain.
         @param target: In MiB.
diff -r 6727070b4129 -r 577b051fca17 tools/python/xen/xm/migrate.py
--- a/tools/python/xen/xm/migrate.py
+++ b/tools/python/xen/xm/migrate.py
@@ -55,6 +55,18 @@ gopts.opt(''change_home_server'',
short=''c
           fn=set_true, default=0,
           use="Change home server for managed domains.")
 
+gopts.opt(''max_iters'', short=''M'',
val=''max_iters'',
+          fn=set_int, default=None,
+          use="Number of iterations before final suspend (default:
30).")
+
+gopts.opt(''max_factor'', short=''m'',
val=''max_factor'',
+          fn=set_int, default=None,
+          use="Max amount of memory to transfer before final suspend
(default: 3*RAM).")
+
+gopts.opt(''no_suspend'', short=''S'',
+          fn=set_true, default=None,
+          use="Abort migration instead of doing final suspend.")
+
 def help():
     return str(gopts)
     
@@ -80,6 +92,10 @@ def main(argv):
         server.xenapi.VM.migrate(vm_ref, dst, bool(opts.vals.live),
                                  other_config)
     else:
+        server.xend.domain.migrate_constraints_set(dom,
+                                                   opts.vals.max_iters,
+                                                   opts.vals.max_factor,
+                                                   opts.vals.no_suspend)
         server.xend.domain.migrate(dom, dst, opts.vals.live,
                                    opts.vals.port,
                                    opts.vals.node,

On Mon, 2013-01-28 at 17:32 +0000, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1359394320 -3600
> # Node ID 577b051fca174be9f7b3d6590c62735cdf214021
> # Parent  6727070b4129cf852199b66b6a81042ee6966a98
> tools: set migration constraints from cmdline
> 
> Add new options to xm/xl migrate to control the process of migration.
> The intention is to optionally abort the migration if it takes too long
> to migrate a busy guest due to the high number of dirty pages. Currently
> the guest is suspended to transfer the remaining dirty pages. This
> transfer can take too long, which can confuse the guest if its suspended
> for too long.
> 
> -M <number>   Number of iterations before final suspend (default: 30)
> --max_iters <number>
> 
> -m <factor>   Max amount of memory to transfer before final suspend
(default: 3*RAM)
> --max_factor <factor>
> 
> -N            Abort migration instead of doing final suspend.
> --no_suspend
> 
> 
> A variant of this change has been tested with xend, the patch below is
> only compile tested. The changes to libxl change the API, is that
> approach acceptable?
I''m afraid not, the compatibility requirements are covered in the
comment near the top of libxl.h.

So you either need a new function or to leverage the LIBXL_API_VERSION
define (which the user must supply) such that people providing 0x040200
see the current interface and people providing 0x040300 (or nothing) see
the new one.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r 6727070b4129 -r 577b051fca17 tools/libxc/xc_domain_save.c
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
> @@ -43,6 +43,8 @@
>  */
>  #define DEF_MAX_ITERS   29   /* limit us to 30 times round loop   */
>  #define DEF_MAX_FACTOR   3   /* never send more than 3x p2m_size  */
> +/* Suspend guest for final transit once number of dirty pages is that low
*/
> +#define DEF_MIN_REMAINING 50
> 
>  struct save_ctx {
>      unsigned long hvirt_start; /* virtual starting address of the
hypervisor */
> @@ -804,9 +806,11 @@ int xc_domain_save(xc_interface *xch, in
>      int rc = 1, frc, i, j, last_iter = 0, iter = 0;
>      int live  = (flags & XCFLAGS_LIVE);
>      int debug = (flags & XCFLAGS_DEBUG);
> +    int nosuspend = (flags & XCFLAGS_DOMSAVE_NOSUSPEND);
>      int superpages = !!hvm;
>      int race = 0, sent_last_iter, skip_this_iter = 0;
>      unsigned int sent_this_iter = 0;
> +    int min_reached;
>      int tmem_saved = 0;
> 
>      /* The new domain''s shared-info frame number. */
> @@ -1525,10 +1529,20 @@ int xc_domain_save(xc_interface *xch, in
> 
>          if ( live )
>          {
> +            min_reached = sent_this_iter + skip_this_iter <
DEF_MIN_REMAINING;
>              if ( (iter >= max_iters) ||
> -                 (sent_this_iter+skip_this_iter < 50) ||
> +                 min_reached ||
>                   (total_sent > dinfo->p2m_size*max_factor) )
>              {
> +                if ( min_reached && nosuspend )
> +                {
> +                    ERROR("Live migration aborted, as requested.
(guest too busy?)"
> +                     " total_sent %lu iter %d, max_iters %u
max_factor %u",
> +                      total_sent, iter, max_iters, max_factor);
> +                    rc = 1;
> +                    goto out;
> +                }
> +
>                  DPRINTF("Start last iteration\n");
>                  last_iter = 1;
> 
> diff -r 6727070b4129 -r 577b051fca17 tools/libxc/xenguest.h
> --- a/tools/libxc/xenguest.h
> +++ b/tools/libxc/xenguest.h
> @@ -28,6 +28,8 @@
>  #define XCFLAGS_HVM       4
>  #define XCFLAGS_STDVGA    8
>  #define XCFLAGS_CHECKPOINT_COMPRESS    16
> +#define XCFLAGS_DOMSAVE_NOSUSPEND      32
> +
>  #define X86_64_B_SIZE   64
>  #define X86_32_B_SIZE   32
> 
> diff -r 6727070b4129 -r 577b051fca17 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -757,6 +757,7 @@ static void domain_suspend_cb(libxl__egc
>  }
> 
>  int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int
flags,
> +                         int max_iters, int max_factor,
>                           const libxl_asyncop_how *ao_how)
>  {
>      AO_CREATE(ctx, domid, ao_how);
> @@ -779,6 +780,9 @@ int libxl_domain_suspend(libxl_ctx *ctx,
>      dss->type = type;
>      dss->live = flags & LIBXL_SUSPEND_LIVE;
>      dss->debug = flags & LIBXL_SUSPEND_DEBUG;
> +    dss->max_iters = max_iters;
> +    dss->max_factor = max_factor;
> +    dss->xlflags = flags;
> 
>      libxl__domain_suspend(egc, dss);
>      return AO_INPROGRESS;
> diff -r 6727070b4129 -r 577b051fca17 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -502,10 +502,12 @@ void libxl_domain_config_dispose(libxl_d
> 
>  int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
>                           int flags, /* LIBXL_SUSPEND_* */
> +                         int max_iters, int max_factor,
>                           const libxl_asyncop_how *ao_how)
>                           LIBXL_EXTERNAL_CALLERS_ONLY;
>  #define LIBXL_SUSPEND_DEBUG 1
>  #define LIBXL_SUSPEND_LIVE 2
> +#define LIBXL_SUSPEND_NO_FINAL_SUSPEND 4
> 
>  /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
>   *   If this parameter is true, use co-operative resume. The guest
> diff -r 6727070b4129 -r 577b051fca17 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
> 
>      dss->xcflags = (live) ? XCFLAGS_LIVE : 0
>            | (debug) ? XCFLAGS_DEBUG : 0
> +          | (dss->xlflags & LIBXL_SUSPEND_NO_FINAL_SUSPEND) ?
XCFLAGS_DOMSAVE_NOSUSPEND : 0
>            | (dss->hvm) ? XCFLAGS_HVM : 0;
> 
>      dss->suspend_eventchn = -1;
> diff -r 6727070b4129 -r 577b051fca17 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2253,6 +2253,9 @@ struct libxl__domain_suspend_state {
>      xc_evtchn *xce; /* event channel handle */
>      int suspend_eventchn;
>      int hvm;
> +    int max_iters;
> +    int max_factor;
> +    int xlflags;
>      int xcflags;
>      int guest_responded;
>      const char *dm_savefile;
> diff -r 6727070b4129 -r 577b051fca17 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
> 
>      save_domain_core_writeconfig(fd, filename, config_data, config_len);
> 
> -    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
> +    int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0, NULL);
>      close(fd);
> 
>      if (rc < 0)
> @@ -3332,6 +3332,8 @@ static void migrate_do_preamble(int send
>  }
> 
>  static void migrate_domain(uint32_t domid, const char *rune,
> +                           int max_iters, int max_factor,
> +                           int no_suspend,
>                             const char *override_config_file)
>  {
>      pid_t child = -1;
> @@ -3341,6 +3343,7 @@ static void migrate_domain(uint32_t domi
>      char rc_buf;
>      uint8_t *config_data;
>      int config_len;
> +    int flags = LIBXL_SUSPEND_LIVE;
> 
>      save_domain_core_begin(domid, override_config_file,
>                             &config_data, &config_len);
> @@ -3358,7 +3361,10 @@ static void migrate_domain(uint32_t domi
> 
>      xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS,
0);
> 
> -    rc = libxl_domain_suspend(ctx, domid, send_fd, LIBXL_SUSPEND_LIVE,
NULL);
> +    if (no_suspend)
> +        flags |= LIBXL_SUSPEND_NO_FINAL_SUSPEND;
> +
> +    rc = libxl_domain_suspend(ctx, domid, send_fd, flags, max_iters,
max_factor, NULL);
>      if (rc) {
>          fprintf(stderr, "migration sender: libxl_domain_suspend
failed"
>                  " (rc=%d)\n", rc);
> @@ -3751,8 +3757,9 @@ int main_migrate(int argc, char **argv)
>      char *rune = NULL;
>      char *host;
>      int opt, daemonize = 1, monitor = 1, debug = 0;
> -
> -    SWITCH_FOREACH_OPT(opt, "FC:s:ed", NULL,
"migrate", 2) {
> +    int max_iters = 0, max_factor = 0, no_suspend = 0;
> +
> +    SWITCH_FOREACH_OPT(opt, "FC:s:edM:m:N", NULL,
"migrate", 2) {
>      case ''C'':
>          config_filename = optarg;
>          break;
> @@ -3769,6 +3776,15 @@ int main_migrate(int argc, char **argv)
>      case ''d'':
>          debug = 1;
>          break;
> +    case ''M'':
> +        max_iters = atoi(optarg);
> +        break;
> +    case ''m'':
> +        max_factor = atoi(optarg);
> +        break;
> +    case ''N'':
> +        no_suspend = 1;
> +        break;
>      }
> 
>      domid = find_domain(argv[optind]);
> @@ -3784,7 +3800,7 @@ int main_migrate(int argc, char **argv)
>              return 1;
>      }
> 
> -    migrate_domain(domid, rune, config_filename);
> +    migrate_domain(domid, rune, max_iters, max_factor, no_suspend,
config_filename);
>      return 0;
>  }
> 
> diff -r 6727070b4129 -r 577b051fca17 tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -152,6 +152,9 @@ struct cmd_spec cmd_table[] = {
>        "-s <sshcommand> Use <sshcommand> instead of ssh. 
String will be passed\n"
>        "                to sh. If empty, run <host> instead of
ssh <host> xl\n"
>        "                migrate-receive [-d -e]\n"
> +      "-M --max_iters <number>  Number of iterations before
final suspend (default: 30)\n"
> +      "-m --max_factor <factor> Max amount of memory to
transfer before final suspend (default: 3*RAM).\n"
> +      "-N --no_suspend Abort migration instead of doing final
suspend.\n"
>        "-e              Do not wait in the background (on
<host>) for the death\n"
>        "                of the domain."
>      },
> diff -r 6727070b4129 -r 577b051fca17
tools/python/xen/xend/XendCheckpoint.py
> --- a/tools/python/xen/xend/XendCheckpoint.py
> +++ b/tools/python/xen/xend/XendCheckpoint.py
> @@ -118,9 +118,19 @@ def save(fd, dominfo, network, live, dst
>          # enabled. Passing "0" simply uses the defaults compiled
into
>          # libxenguest; see the comments and/or code in xc_linux_save() for
>          # more information.
> +        max_iters = dominfo.info.get(''max_iters'',
"0")
> +        max_factor = dominfo.info.get(''max_factor'',
"0")
> +        no_suspend = dominfo.info.get(''no_suspend'',
None)
> +        if max_iters == "None":
> +            max_iters = "0"
> +        if max_factor == "None":
> +            max_factor = "0"
> +        if no_suspend == "None":
> +            no_suspend = "0"
>          cmd = [xen.util.auxbin.pathTo(XC_SAVE), str(fd),
> -               str(dominfo.getDomid()), "0", "0",
> -               str(int(live) | (int(hvm) << 2)) ]
> +               str(dominfo.getDomid()),
> +               max_iters, max_factor,
> +               str(int(live) | (int(hvm) << 2) | (int(no_suspend)
<< 5) ]
>          log.debug("[xc_save]: %s", string.join(cmd))
> 
>          def saveInputHandler(line, tochild):
> diff -r 6727070b4129 -r 577b051fca17 tools/python/xen/xend/XendDomain.py
> --- a/tools/python/xen/xend/XendDomain.py
> +++ b/tools/python/xen/xend/XendDomain.py
> @@ -1832,6 +1832,18 @@ class XendDomain:
>              log.exception(ex)
>              raise XendError(str(ex))
> 
> +    def domain_migrate_constraints_set((self, domid, max_iters,
max_factor, no_suspend):
> +        """Set the Migrate Constraints of this domain.
> +        @param domid: Domain ID or Name
> +        @param max_iters: Number of iterations before final suspend
> +        @param max_factor: Max amount of memory to transfer before final
suspend
> +        @param no_suspend: Abort migration instead of doing final suspend
> +        """
> +        dominfo = self.domain_lookup_nr(domid)
> +        if not dominfo:
> +            raise XendInvalidDomain(str(domid))
> +        dominfo.setMigrateConstraints(max_iters, max_factor, no_suspend)
> +
>      def domain_maxmem_set(self, domid, mem):
>          """Set the memory limit for a domain.
> 
> diff -r 6727070b4129 -r 577b051fca17
tools/python/xen/xend/XendDomainInfo.py
> --- a/tools/python/xen/xend/XendDomainInfo.py
> +++ b/tools/python/xen/xend/XendDomainInfo.py
> @@ -1459,6 +1459,18 @@ class XendDomainInfo:
>          pci_conf = self.info[''devices''][dev_uuid][1]
>          return map(pci_dict_to_bdf_str,
pci_conf[''devs''])
> 
> +    def setMigrateConstraints(self, max_iters, max_factor, no_suspend):
> +        """Set the Migrate Constraints of this domain.
> +        @param max_iters: Number of iterations before final suspend
> +        @param max_factor: Max amount of memory to transfer before final
suspend
> +        @param no_suspend: Abort migration instead of doing final suspend
> +        """
> +        log.debug("Setting setMigrateConstraints of domain %s (%s) to
''%s'' ''%s'' ''%s''.",
> +                  self.info[''name_label''],
str(self.domid), max_iters, max_factor, no_suspend)
> +        self.info[''max_iters''] = str(max_iters)
> +        self.info[''max_factor''] = str(max_factor)
> +        self.info[''no_suspend''] = str(no_suspend)
> +
>      def setMemoryTarget(self, target):
>          """Set the memory target of this domain.
>          @param target: In MiB.
> diff -r 6727070b4129 -r 577b051fca17 tools/python/xen/xm/migrate.py
> --- a/tools/python/xen/xm/migrate.py
> +++ b/tools/python/xen/xm/migrate.py
> @@ -55,6 +55,18 @@ gopts.opt(''change_home_server'',
short=''c
>            fn=set_true, default=0,
>            use="Change home server for managed domains.")
> 
> +gopts.opt(''max_iters'', short=''M'',
val=''max_iters'',
> +          fn=set_int, default=None,
> +          use="Number of iterations before final suspend (default:
30).")
> +
> +gopts.opt(''max_factor'', short=''m'',
val=''max_factor'',
> +          fn=set_int, default=None,
> +          use="Max amount of memory to transfer before final suspend
(default: 3*RAM).")
> +
> +gopts.opt(''no_suspend'', short=''S'',
> +          fn=set_true, default=None,
> +          use="Abort migration instead of doing final suspend.")
> +
>  def help():
>      return str(gopts)
> 
> @@ -80,6 +92,10 @@ def main(argv):
>          server.xenapi.VM.migrate(vm_ref, dst, bool(opts.vals.live),
>                                   other_config)
>      else:
> +        server.xend.domain.migrate_constraints_set(dom,
> +                                                   opts.vals.max_iters,
> +                                                   opts.vals.max_factor,
> +                                                   opts.vals.no_suspend)
>          server.xend.domain.migrate(dom, dst, opts.vals.live,
>                                     opts.vals.port,
>                                     opts.vals.node,
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1359563423 -3600
# Node ID e44917a64f31036dccb81864e942c49e3a36cab5
# Parent  6727070b4129cf852199b66b6a81042ee6966a98
tools: set migration constraints from cmdline

Add new options to xm/xl migrate to control the process of migration.
The intention is to optionally abort the migration if it takes too long
to migrate a busy guest due to the high number of dirty pages. Currently
the guest is suspended to transfer the remaining dirty pages. This
transfer can take too long, which can confuse the guest if its suspended
for too long.

-M <number>   Number of iterations before final suspend (default: 30)
--max_iters <number>

-m <factor>   Max amount of memory to transfer before final suspend
(default: 3*RAM)
--max_factor <factor>

-N            Abort migration instead of doing final suspend.
--no_suspend


A variant of this change has been tested with xend, the patch below is
only compile tested. The changes to libxl change the API, is that
approach acceptable?

v2:
 - use LIBXL_API_VERSION and define libxl_domain_suspend_0x040200
 - fix logic error in min_reached check in xc_domain_save
 - add longopts
 - update --help text
 - correct description of migrate --help text

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 6727070b4129 -r e44917a64f31 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -43,6 +43,8 @@
 */
 #define DEF_MAX_ITERS   29   /* limit us to 30 times round loop   */
 #define DEF_MAX_FACTOR   3   /* never send more than 3x p2m_size  */
+/* Suspend guest for final transit once number of dirty pages is that low */
+#define DEF_MIN_REMAINING 50
 
 struct save_ctx {
     unsigned long hvirt_start; /* virtual starting address of the hypervisor */
@@ -804,6 +806,7 @@ int xc_domain_save(xc_interface *xch, in
     int rc = 1, frc, i, j, last_iter = 0, iter = 0;
     int live  = (flags & XCFLAGS_LIVE);
     int debug = (flags & XCFLAGS_DEBUG);
+    int no_suspend = (flags & XCFLAGS_DOMSAVE_NOSUSPEND);
     int superpages = !!hvm;
     int race = 0, sent_last_iter, skip_this_iter = 0;
     unsigned int sent_this_iter = 0;
@@ -1525,10 +1528,20 @@ int xc_domain_save(xc_interface *xch, in
 
         if ( live )
         {
+            int min_reached = sent_this_iter + skip_this_iter <
DEF_MIN_REMAINING;
             if ( (iter >= max_iters) ||
-                 (sent_this_iter+skip_this_iter < 50) ||
+                 min_reached ||
                  (total_sent > dinfo->p2m_size*max_factor) )
             {
+                if ( !min_reached && no_suspend )
+                {
+                    ERROR("Live migration aborted, as requested. (guest
too busy?)"
+                     " total_sent %lu iter %d, max_iters %u max_factor
%u",
+                      total_sent, iter, max_iters, max_factor);
+                    rc = 1;
+                    goto out;
+                }
+
                 DPRINTF("Start last iteration\n");
                 last_iter = 1;
 
diff -r 6727070b4129 -r e44917a64f31 tools/libxc/xenguest.h
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -28,6 +28,8 @@
 #define XCFLAGS_HVM       4
 #define XCFLAGS_STDVGA    8
 #define XCFLAGS_CHECKPOINT_COMPRESS    16
+#define XCFLAGS_DOMSAVE_NOSUSPEND      32
+
 #define X86_64_B_SIZE   64 
 #define X86_32_B_SIZE   32
 
diff -r 6727070b4129 -r e44917a64f31 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -756,7 +756,8 @@ static void domain_suspend_cb(libxl__egc
 
 }
 
-int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
+static int __libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, int max_iters, int max_factor,
                          const libxl_asyncop_how *ao_how)
 {
     AO_CREATE(ctx, domid, ao_how);
@@ -779,6 +780,9 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     dss->type = type;
     dss->live = flags & LIBXL_SUSPEND_LIVE;
     dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+    dss->max_iters = max_iters;
+    dss->max_factor = max_factor;
+    dss->xlflags = flags;
 
     libxl__domain_suspend(egc, dss);
     return AO_INPROGRESS;
@@ -787,6 +791,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     return AO_ABORT(rc);
 }
 
+int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, const libxl_asyncop_how *ao_how)
+{
+	return __libxl_domain_suspend(ctx, domid, fd, flags, 0,0, ao_how);
+}
+
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
+                         int max_iters, int max_factor,
+                         const libxl_asyncop_how *ao_how)
+{
+	return __libxl_domain_suspend(ctx, domid, fd, flags, max_iters,
+                                      max_factor, ao_how);
+}
+
 int libxl_domain_pause(libxl_ctx *ctx, uint32_t domid)
 {
     int ret;
diff -r 6727070b4129 -r e44917a64f31 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
 void libxl_domain_config_init(libxl_domain_config *d_config);
 void libxl_domain_config_dispose(libxl_domain_config *d_config);
 
-int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+#ifdef LIBXL_API_VERSION
+#if LIBXL_API_VERSION == 0x040200
+#define libxl_domain_suspend libxl_domain_suspend_0x040200
+int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
                          int flags, /* LIBXL_SUSPEND_* */
                          const libxl_asyncop_how *ao_how)
                          LIBXL_EXTERNAL_CALLERS_ONLY;
+#endif
+#else
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, /* LIBXL_SUSPEND_* */
+                         int max_iters, int max_factor,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+#endif
+
 #define LIBXL_SUSPEND_DEBUG 1
 #define LIBXL_SUSPEND_LIVE 2
+#define LIBXL_SUSPEND_NO_FINAL_SUSPEND 4
 
 /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
  *   If this parameter is true, use co-operative resume. The guest
diff -r 6727070b4129 -r e44917a64f31 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
 
     dss->xcflags = (live) ? XCFLAGS_LIVE : 0
           | (debug) ? XCFLAGS_DEBUG : 0
+          | (dss->xlflags & LIBXL_SUSPEND_NO_FINAL_SUSPEND) ?
XCFLAGS_DOMSAVE_NOSUSPEND : 0
           | (dss->hvm) ? XCFLAGS_HVM : 0;
 
     dss->suspend_eventchn = -1;
diff -r 6727070b4129 -r e44917a64f31 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2253,6 +2253,9 @@ struct libxl__domain_suspend_state {
     xc_evtchn *xce; /* event channel handle */
     int suspend_eventchn;
     int hvm;
+    int max_iters;
+    int max_factor;
+    int xlflags;
     int xcflags;
     int guest_responded;
     const char *dm_savefile;
diff -r 6727070b4129 -r e44917a64f31 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
 
     save_domain_core_writeconfig(fd, filename, config_data, config_len);
 
-    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
+    int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0, NULL);
     close(fd);
 
     if (rc < 0)
@@ -3332,6 +3332,8 @@ static void migrate_do_preamble(int send
 }
 
 static void migrate_domain(uint32_t domid, const char *rune,
+                           int max_iters, int max_factor,
+                           int no_suspend,
                            const char *override_config_file)
 {
     pid_t child = -1;
@@ -3341,6 +3343,7 @@ static void migrate_domain(uint32_t domi
     char rc_buf;
     uint8_t *config_data;
     int config_len;
+    int flags = LIBXL_SUSPEND_LIVE;
 
     save_domain_core_begin(domid, override_config_file,
                            &config_data, &config_len);
@@ -3358,7 +3361,10 @@ static void migrate_domain(uint32_t domi
 
     xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS, 0);
 
-    rc = libxl_domain_suspend(ctx, domid, send_fd, LIBXL_SUSPEND_LIVE, NULL);
+    if (no_suspend)
+        flags |= LIBXL_SUSPEND_NO_FINAL_SUSPEND;
+
+    rc = libxl_domain_suspend(ctx, domid, send_fd, flags, max_iters,
max_factor, NULL);
     if (rc) {
         fprintf(stderr, "migration sender: libxl_domain_suspend
failed"
                 " (rc=%d)\n", rc);
@@ -3751,8 +3757,16 @@ int main_migrate(int argc, char **argv)
     char *rune = NULL;
     char *host;
     int opt, daemonize = 1, monitor = 1, debug = 0;
-
-    SWITCH_FOREACH_OPT(opt, "FC:s:ed", NULL, "migrate", 2)
{
+    int max_iters = 0, max_factor = 0, no_suspend = 0;
+    static struct option opts[] = {
+        {"max_iters", 1, 0, ''M''},
+        {"max_factor", 1, 0, ''m''},
+        {"no_suspend", 0, 0, ''N''},
+        COMMON_LONG_OPTS,
+        {0, 0, 0, 0}
+    };
+
+    SWITCH_FOREACH_OPT(opt, "FC:s:edM:m:N", opts,
"migrate", 2) {
     case ''C'':
         config_filename = optarg;
         break;
@@ -3769,6 +3783,15 @@ int main_migrate(int argc, char **argv)
     case ''d'':
         debug = 1;
         break;
+    case ''M'':
+        max_iters = atoi(optarg);
+        break;
+    case ''m'':
+        max_factor = atoi(optarg);
+        break;
+    case ''N'':
+        no_suspend = 1;
+        break;
     }
 
     domid = find_domain(argv[optind]);
@@ -3784,7 +3807,7 @@ int main_migrate(int argc, char **argv)
             return 1;
     }
 
-    migrate_domain(domid, rune, config_filename);
+    migrate_domain(domid, rune, max_iters, max_factor, no_suspend,
config_filename);
     return 0;
 }
 
diff -r 6727070b4129 -r e44917a64f31 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -145,15 +145,20 @@ struct cmd_spec cmd_table[] = {
     },
     { "migrate",
       &main_migrate, 0, 1,
-      "Save a domain state to restore later",
+      "Migrate a domain to <host>",
       "[options] <Domain> <host>",
-      "-h              Print this help.\n"
-      "-C <config>     Send <config> instead of config file
from creation.\n"
-      "-s <sshcommand> Use <sshcommand> instead of ssh. 
String will be passed\n"
-      "                to sh. If empty, run <host> instead of ssh
<host> xl\n"
-      "                migrate-receive [-d -e]\n"
-      "-e              Do not wait in the background (on <host>) for
the death\n"
-      "                of the domain."
+      "-h               Print this help.\n"
+      "-C <config>      Send <config> instead of config file
from creation.\n"
+      "-s <sshcommand>  Use <sshcommand> instead of ssh. 
String will be passed\n"
+      "                 to sh. If empty, run <host> instead of ssh
<host> xl\n"
+      "                 migrate-receive [-d -e]\n"
+      "-e               Do not wait in the background (on <host>)
for the death\n"
+      "                 of the domain.\n"
+      "-M <number>      Number of iterations before final suspend
(default: 30)\n"
+      "--max_iters <number>\n"
+      "-m <factor>      Max amount of memory to transfer before
final suspend (default: 3*RAM).\n"
+      "--max_factor <factor>\n"
+      "-N, --no_suspend Abort migration instead of doing final
suspend."
     },
     { "dump-core",
       &main_dump_core, 0, 1,
diff -r 6727070b4129 -r e44917a64f31 tools/python/xen/xend/XendCheckpoint.py
--- a/tools/python/xen/xend/XendCheckpoint.py
+++ b/tools/python/xen/xend/XendCheckpoint.py
@@ -118,9 +118,19 @@ def save(fd, dominfo, network, live, dst
         # enabled. Passing "0" simply uses the defaults compiled into
         # libxenguest; see the comments and/or code in xc_linux_save() for
         # more information.
+        max_iters = dominfo.info.get(''max_iters'',
"0")
+        max_factor = dominfo.info.get(''max_factor'',
"0")
+        no_suspend = dominfo.info.get(''no_suspend'', None)
+        if max_iters == "None":
+            max_iters = "0"
+        if max_factor == "None":
+            max_factor = "0"
+        if no_suspend == "None":
+            no_suspend = "0"
         cmd = [xen.util.auxbin.pathTo(XC_SAVE), str(fd),
-               str(dominfo.getDomid()), "0", "0", 
-               str(int(live) | (int(hvm) << 2)) ]
+               str(dominfo.getDomid()),
+               max_iters, max_factor,
+               str(int(live) | (int(hvm) << 2) | (int(no_suspend)
<< 5) ]
         log.debug("[xc_save]: %s", string.join(cmd))
 
         def saveInputHandler(line, tochild):
diff -r 6727070b4129 -r e44917a64f31 tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py
+++ b/tools/python/xen/xend/XendDomain.py
@@ -1832,6 +1832,18 @@ class XendDomain:
             log.exception(ex)
             raise XendError(str(ex))
 
+    def domain_migrate_constraints_set((self, domid, max_iters, max_factor,
no_suspend):
+        """Set the Migrate Constraints of this domain.
+        @param domid: Domain ID or Name
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param no_suspend: Abort migration instead of doing final suspend
+        """
+        dominfo = self.domain_lookup_nr(domid)
+        if not dominfo:
+            raise XendInvalidDomain(str(domid))
+        dominfo.setMigrateConstraints(max_iters, max_factor, no_suspend)
+
     def domain_maxmem_set(self, domid, mem):
         """Set the memory limit for a domain.
 
diff -r 6727070b4129 -r e44917a64f31 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -1459,6 +1459,18 @@ class XendDomainInfo:
         pci_conf = self.info[''devices''][dev_uuid][1]
         return map(pci_dict_to_bdf_str, pci_conf[''devs''])
 
+    def setMigrateConstraints(self, max_iters, max_factor, no_suspend):
+        """Set the Migrate Constraints of this domain.
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param no_suspend: Abort migration instead of doing final suspend
+        """
+        log.debug("Setting setMigrateConstraints of domain %s (%s) to
''%s'' ''%s'' ''%s''.",
+                  self.info[''name_label''], str(self.domid),
max_iters, max_factor, no_suspend)
+        self.info[''max_iters''] = str(max_iters)
+        self.info[''max_factor''] = str(max_factor)
+        self.info[''no_suspend''] = str(no_suspend)
+
     def setMemoryTarget(self, target):
         """Set the memory target of this domain.
         @param target: In MiB.
diff -r 6727070b4129 -r e44917a64f31 tools/python/xen/xm/migrate.py
--- a/tools/python/xen/xm/migrate.py
+++ b/tools/python/xen/xm/migrate.py
@@ -55,6 +55,18 @@ gopts.opt(''change_home_server'',
short=''c
           fn=set_true, default=0,
           use="Change home server for managed domains.")
 
+gopts.opt(''max_iters'', short=''M'',
val=''max_iters'',
+          fn=set_int, default=None,
+          use="Number of iterations before final suspend (default:
30).")
+
+gopts.opt(''max_factor'', short=''m'',
val=''max_factor'',
+          fn=set_int, default=None,
+          use="Max amount of memory to transfer before final suspend
(default: 3*RAM).")
+
+gopts.opt(''no_suspend'', short=''S'',
+          fn=set_true, default=None,
+          use="Abort migration instead of doing final suspend.")
+
 def help():
     return str(gopts)
     
@@ -80,6 +92,10 @@ def main(argv):
         server.xenapi.VM.migrate(vm_ref, dst, bool(opts.vals.live),
                                  other_config)
     else:
+        server.xend.domain.migrate_constraints_set(dom,
+                                                   opts.vals.max_iters,
+                                                   opts.vals.max_factor,
+                                                   opts.vals.no_suspend)
         server.xend.domain.migrate(dom, dst, opts.vals.live,
                                    opts.vals.port,
                                    opts.vals.node,

On Wed, Jan 30, Ian Campbell wrote:
> On Mon, 2013-01-28 at 17:32 +0000, Olaf Hering wrote:
> > A variant of this change has been tested with xend, the patch below is
> > only compile tested. The changes to libxl change the API, is that
> > approach acceptable?
> 
> I''m afraid not, the compatibility requirements are covered in the
> comment near the top of libxl.h.
> 
> So you either need a new function or to leverage the LIBXL_API_VERSION
> define (which the user must supply) such that people providing 0x040200
> see the current interface and people providing 0x040300 (or nothing) see
> the new one.
I sent v2 of this patch which includes an (the first) attempt to use
LIBXL_API_VERSION. I think in this patch the SONAME needs to be changed
as well now.

Olaf

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1359649053 -3600
# Node ID bd1412703913e2e77ec54da8600887368a893597
# Parent  256d59b2bc8a413876559dc8daf4c52ba46677de
tools: set migration constraints from cmdline

Add new options to xm/xl migrate to control the process of migration.
The intention is to optionally abort the migration if it takes too long
to migrate a busy guest due to the high number of dirty pages. Currently
the guest is suspended to transfer the remaining dirty pages. This
transfer can take too long, which can confuse the guest if its suspended
for too long.

-M <number>   Number of iterations before final suspend (default: 30)
--max_iters <number>

-m <factor>   Max amount of memory to transfer before final suspend
(default: 3*RAM)
--max_factor <factor>

-N            Abort migration instead of doing final suspend.
--no_suspend



The changes to libxl change the API, is that approach acceptable?

v3:
 - move logic errors in libxl__domain_suspend and fixed help text in
   cmd_table to separate patches
 - fix syntax error in XendCheckpoint.py
 - really pass max_iters and max_factor in libxl__xc_domain_save
 - make libxl_domain_suspend_0x040200 declaration globally visible
 - bump libxenlight.so SONAME from 2.0 to 3.0 due to changed
   libxl_domain_suspend

v2:
 - use LIBXL_API_VERSION and define libxl_domain_suspend_0x040200
 - fix logic error in min_reached check in xc_domain_save
 - add longopts
 - update --help text
 - correct description of migrate --help text

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 256d59b2bc8a -r bd1412703913 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -43,6 +43,8 @@
 */
 #define DEF_MAX_ITERS   29   /* limit us to 30 times round loop   */
 #define DEF_MAX_FACTOR   3   /* never send more than 3x p2m_size  */
+/* Suspend guest for final transit once number of dirty pages is that low */
+#define DEF_MIN_REMAINING 50
 
 struct save_ctx {
     unsigned long hvirt_start; /* virtual starting address of the hypervisor */
@@ -804,6 +806,7 @@ int xc_domain_save(xc_interface *xch, in
     int rc = 1, frc, i, j, last_iter = 0, iter = 0;
     int live  = (flags & XCFLAGS_LIVE);
     int debug = (flags & XCFLAGS_DEBUG);
+    int no_suspend = (flags & XCFLAGS_DOMSAVE_NOSUSPEND);
     int superpages = !!hvm;
     int race = 0, sent_last_iter, skip_this_iter = 0;
     unsigned int sent_this_iter = 0;
@@ -1525,10 +1528,20 @@ int xc_domain_save(xc_interface *xch, in
 
         if ( live )
         {
+            int min_reached = sent_this_iter + skip_this_iter <
DEF_MIN_REMAINING;
             if ( (iter >= max_iters) ||
-                 (sent_this_iter+skip_this_iter < 50) ||
+                 min_reached ||
                  (total_sent > dinfo->p2m_size*max_factor) )
             {
+                if ( !min_reached && no_suspend )
+                {
+                    ERROR("Live migration aborted, as requested. (guest
too busy?)"
+                     " total_sent %lu iter %d, max_iters %u max_factor
%u",
+                      total_sent, iter, max_iters, max_factor);
+                    rc = 1;
+                    goto out;
+                }
+
                 DPRINTF("Start last iteration\n");
                 last_iter = 1;
 
diff -r 256d59b2bc8a -r bd1412703913 tools/libxc/xenguest.h
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -28,6 +28,8 @@
 #define XCFLAGS_HVM       4
 #define XCFLAGS_STDVGA    8
 #define XCFLAGS_CHECKPOINT_COMPRESS    16
+#define XCFLAGS_DOMSAVE_NOSUSPEND      32
+
 #define X86_64_B_SIZE   64 
 #define X86_32_B_SIZE   32
 
diff -r 256d59b2bc8a -r bd1412703913 tools/libxl/Makefile
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -5,7 +5,7 @@
 XEN_ROOT = $(CURDIR)/../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-MAJOR = 2.0
+MAJOR = 2.1
 MINOR = 0
 
 XLUMAJOR = 1.0
diff -r 256d59b2bc8a -r bd1412703913 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -756,7 +756,8 @@ static void domain_suspend_cb(libxl__egc
 
 }
 
-int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
+static int __libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, int max_iters, int max_factor,
                          const libxl_asyncop_how *ao_how)
 {
     AO_CREATE(ctx, domid, ao_how);
@@ -779,6 +780,9 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     dss->type = type;
     dss->live = flags & LIBXL_SUSPEND_LIVE;
     dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+    dss->max_iters = max_iters;
+    dss->max_factor = max_factor;
+    dss->xlflags = flags;
 
     libxl__domain_suspend(egc, dss);
     return AO_INPROGRESS;
@@ -787,6 +791,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     return AO_ABORT(rc);
 }
 
+int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, const libxl_asyncop_how *ao_how)
+{
+	return __libxl_domain_suspend(ctx, domid, fd, flags, 0,0, ao_how);
+}
+
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
+                         int max_iters, int max_factor,
+                         const libxl_asyncop_how *ao_how)
+{
+	return __libxl_domain_suspend(ctx, domid, fd, flags, max_iters,
+                                      max_factor, ao_how);
+}
+
 int libxl_domain_pause(libxl_ctx *ctx, uint32_t domid)
 {
     int ret;
diff -r 256d59b2bc8a -r bd1412703913 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
 void libxl_domain_config_init(libxl_domain_config *d_config);
 void libxl_domain_config_dispose(libxl_domain_config *d_config);
 
-int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
                          int flags, /* LIBXL_SUSPEND_* */
                          const libxl_asyncop_how *ao_how)
                          LIBXL_EXTERNAL_CALLERS_ONLY;
+#ifdef LIBXL_API_VERSION
+#if LIBXL_API_VERSION == 0x040200
+#define libxl_domain_suspend libxl_domain_suspend_0x040200
+#endif
+#else
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, /* LIBXL_SUSPEND_* */
+                         int max_iters, int max_factor,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+#endif
+
 #define LIBXL_SUSPEND_DEBUG 1
 #define LIBXL_SUSPEND_LIVE 2
+#define LIBXL_SUSPEND_NO_FINAL_SUSPEND 4
 
 /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
  *   If this parameter is true, use co-operative resume. The guest
diff -r 256d59b2bc8a -r bd1412703913 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
 
     dss->xcflags = (live ? XCFLAGS_LIVE : 0)
           | (debug ? XCFLAGS_DEBUG : 0)
+          | (dss->xlflags & LIBXL_SUSPEND_NO_FINAL_SUSPEND ?
XCFLAGS_DOMSAVE_NOSUSPEND : 0)
           | (dss->hvm ? XCFLAGS_HVM : 0);
 
     dss->suspend_eventchn = -1;
diff -r 256d59b2bc8a -r bd1412703913 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2253,6 +2253,9 @@ struct libxl__domain_suspend_state {
     xc_evtchn *xce; /* event channel handle */
     int suspend_eventchn;
     int hvm;
+    int max_iters;
+    int max_factor;
+    int xlflags;
     int xcflags;
     int guest_responded;
     const char *dm_savefile;
diff -r 256d59b2bc8a -r bd1412703913 tools/libxl/libxl_save_callout.c
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -108,8 +108,8 @@ void libxl__xc_domain_save(libxl__egc *e
     }
 
     const unsigned long argnums[] = {
-        dss->domid, 0, 0, dss->xcflags, dss->hvm,
vm_generationid_addr,
-        toolstack_data_fd, toolstack_data_len,
+        dss->domid, dss->max_iters, dss->max_factor, dss->xcflags,
dss->hvm,
+        vm_generationid_addr, toolstack_data_fd, toolstack_data_len,
         cbflags,
     };
 
diff -r 256d59b2bc8a -r bd1412703913 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
 
     save_domain_core_writeconfig(fd, filename, config_data, config_len);
 
-    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
+    int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0, NULL);
     close(fd);
 
     if (rc < 0)
@@ -3332,6 +3332,7 @@ static void migrate_do_preamble(int send
 }
 
 static void migrate_domain(uint32_t domid, const char *rune, int debug,
+                           int max_iters, int max_factor, int no_suspend,
                            const char *override_config_file)
 {
     pid_t child = -1;
@@ -3360,7 +3361,10 @@ static void migrate_domain(uint32_t domi
 
     if (debug)
         flags |= LIBXL_SUSPEND_DEBUG;
-    rc = libxl_domain_suspend(ctx, domid, send_fd, flags, NULL);
+    if (no_suspend)
+        flags |= LIBXL_SUSPEND_NO_FINAL_SUSPEND;
+
+    rc = libxl_domain_suspend(ctx, domid, send_fd, flags, max_iters,
max_factor, NULL);
     if (rc) {
         fprintf(stderr, "migration sender: libxl_domain_suspend
failed"
                 " (rc=%d)\n", rc);
@@ -3753,8 +3757,16 @@ int main_migrate(int argc, char **argv)
     char *rune = NULL;
     char *host;
     int opt, daemonize = 1, monitor = 1, debug = 0;
-
-    SWITCH_FOREACH_OPT(opt, "FC:s:ed", NULL, "migrate", 2)
{
+    int max_iters = 0, max_factor = 0, no_suspend = 0;
+    static struct option opts[] = {
+        {"max_iters", 1, 0, ''M''},
+        {"max_factor", 1, 0, ''m''},
+        {"no_suspend", 0, 0, ''N''},
+        COMMON_LONG_OPTS,
+        {0, 0, 0, 0}
+    };
+
+    SWITCH_FOREACH_OPT(opt, "FC:s:edM:m:N", opts,
"migrate", 2) {
     case ''C'':
         config_filename = optarg;
         break;
@@ -3771,6 +3783,15 @@ int main_migrate(int argc, char **argv)
     case ''d'':
         debug = 1;
         break;
+    case ''M'':
+        max_iters = atoi(optarg);
+        break;
+    case ''m'':
+        max_factor = atoi(optarg);
+        break;
+    case ''N'':
+        no_suspend = 1;
+        break;
     }
 
     domid = find_domain(argv[optind]);
@@ -3786,7 +3807,7 @@ int main_migrate(int argc, char **argv)
             return 1;
     }
 
-    migrate_domain(domid, rune, debug, config_filename);
+    migrate_domain(domid, rune, debug, max_iters, max_factor, no_suspend,
config_filename);
     return 0;
 }
 
diff -r 256d59b2bc8a -r bd1412703913 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -147,14 +147,19 @@ struct cmd_spec cmd_table[] = {
       &main_migrate, 0, 1,
       "Migrate a domain to another host",
       "[options] <Domain> <host>",
-      "-h              Print this help.\n"
-      "-C <config>     Send <config> instead of config file
from creation.\n"
-      "-s <sshcommand> Use <sshcommand> instead of ssh. 
String will be passed\n"
-      "                to sh. If empty, run <host> instead of ssh
<host> xl\n"
-      "                migrate-receive [-d -e]\n"
-      "-e              Do not wait in the background (on <host>) for
the death\n"
-      "                of the domain.\n"
-      "-d              Print huge (!) amount of debug during the migration
process."
+      "-h               Print this help.\n"
+      "-C <config>      Send <config> instead of config file
from creation.\n"
+      "-s <sshcommand>  Use <sshcommand> instead of ssh. 
String will be passed\n"
+      "                 to sh. If empty, run <host> instead of ssh
<host> xl\n"
+      "                 migrate-receive [-d -e]\n"
+      "-e               Do not wait in the background (on <host>)
for the death\n"
+      "                 of the domain.\n"
+      "-d               Print huge (!) amount of debug during the
migration process.\n"
+      "-M <number>      Number of iterations before final suspend
(default: 30)\n"
+      "--max_iters <number>\n"
+      "-m <factor>      Max amount of memory to transfer before
final suspend (default: 3*RAM).\n"
+      "--max_factor <factor>\n"
+      "-N, --no_suspend Abort migration instead of doing final
suspend."
     },
     { "dump-core",
       &main_dump_core, 0, 1,
diff -r 256d59b2bc8a -r bd1412703913 tools/python/xen/xend/XendCheckpoint.py
--- a/tools/python/xen/xend/XendCheckpoint.py
+++ b/tools/python/xen/xend/XendCheckpoint.py
@@ -118,9 +118,19 @@ def save(fd, dominfo, network, live, dst
         # enabled. Passing "0" simply uses the defaults compiled into
         # libxenguest; see the comments and/or code in xc_linux_save() for
         # more information.
+        max_iters = dominfo.info.get(''max_iters'',
"0")
+        max_factor = dominfo.info.get(''max_factor'',
"0")
+        no_suspend = dominfo.info.get(''no_suspend'', None)
+        if max_iters == "None":
+            max_iters = "0"
+        if max_factor == "None":
+            max_factor = "0"
+        if no_suspend == "None":
+            no_suspend = "0"
         cmd = [xen.util.auxbin.pathTo(XC_SAVE), str(fd),
-               str(dominfo.getDomid()), "0", "0", 
-               str(int(live) | (int(hvm) << 2)) ]
+               str(dominfo.getDomid()),
+               max_iters, max_factor,
+               str(int(live) | (int(hvm) << 2) | (int(no_suspend)
<< 5)) ]
         log.debug("[xc_save]: %s", string.join(cmd))
 
         def saveInputHandler(line, tochild):
diff -r 256d59b2bc8a -r bd1412703913 tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py
+++ b/tools/python/xen/xend/XendDomain.py
@@ -1832,6 +1832,18 @@ class XendDomain:
             log.exception(ex)
             raise XendError(str(ex))
 
+    def domain_migrate_constraints_set((self, domid, max_iters, max_factor,
no_suspend):
+        """Set the Migrate Constraints of this domain.
+        @param domid: Domain ID or Name
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param no_suspend: Abort migration instead of doing final suspend
+        """
+        dominfo = self.domain_lookup_nr(domid)
+        if not dominfo:
+            raise XendInvalidDomain(str(domid))
+        dominfo.setMigrateConstraints(max_iters, max_factor, no_suspend)
+
     def domain_maxmem_set(self, domid, mem):
         """Set the memory limit for a domain.
 
diff -r 256d59b2bc8a -r bd1412703913 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -1459,6 +1459,18 @@ class XendDomainInfo:
         pci_conf = self.info[''devices''][dev_uuid][1]
         return map(pci_dict_to_bdf_str, pci_conf[''devs''])
 
+    def setMigrateConstraints(self, max_iters, max_factor, no_suspend):
+        """Set the Migrate Constraints of this domain.
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param no_suspend: Abort migration instead of doing final suspend
+        """
+        log.debug("Setting setMigrateConstraints of domain %s (%s) to
''%s'' ''%s'' ''%s''.",
+                  self.info[''name_label''], str(self.domid),
max_iters, max_factor, no_suspend)
+        self.info[''max_iters''] = str(max_iters)
+        self.info[''max_factor''] = str(max_factor)
+        self.info[''no_suspend''] = str(no_suspend)
+
     def setMemoryTarget(self, target):
         """Set the memory target of this domain.
         @param target: In MiB.
diff -r 256d59b2bc8a -r bd1412703913 tools/python/xen/xm/migrate.py
--- a/tools/python/xen/xm/migrate.py
+++ b/tools/python/xen/xm/migrate.py
@@ -55,6 +55,18 @@ gopts.opt(''change_home_server'',
short=''c
           fn=set_true, default=0,
           use="Change home server for managed domains.")
 
+gopts.opt(''max_iters'', short=''M'',
val=''max_iters'',
+          fn=set_int, default=None,
+          use="Number of iterations before final suspend (default:
30).")
+
+gopts.opt(''max_factor'', short=''m'',
val=''max_factor'',
+          fn=set_int, default=None,
+          use="Max amount of memory to transfer before final suspend
(default: 3*RAM).")
+
+gopts.opt(''no_suspend'', short=''S'',
+          fn=set_true, default=None,
+          use="Abort migration instead of doing final suspend.")
+
 def help():
     return str(gopts)
     
@@ -80,6 +92,10 @@ def main(argv):
         server.xenapi.VM.migrate(vm_ref, dst, bool(opts.vals.live),
                                  other_config)
     else:
+        server.xend.domain.migrate_constraints_set(dom,
+                                                   opts.vals.max_iters,
+                                                   opts.vals.max_factor,
+                                                   opts.vals.no_suspend)
         server.xend.domain.migrate(dom, dst, opts.vals.live,
                                    opts.vals.port,
                                    opts.vals.node,

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1359746832 -3600
# Node ID 785c8f34e1f802106e53ca4d2c54dce55c8ee166
# Parent  fd711ebdce9a58556d62c2daaf5d49ab06de4a3c
tools: set migration constraints from cmdline

Add new options to xm/xl migrate to control the process of migration.
The intention is to optionally abort the migration if it takes too long
to migrate a busy guest due to the high number of dirty pages. Currently
the guest is suspended to transfer the remaining dirty pages. This
transfer can take too long, which can confuse the guest if its suspended
for too long.

-M <number>   Number of iterations before final suspend (default: 30)
--max_iters <number>

-m <factor>   Max amount of memory to transfer before final suspend
(default: 3*RAM)
--max_factor <factor>

-N            Abort migration instead of doing final suspend.
--no_suspend



The changes to libxl change the API, is that approach acceptable?

v4:
 - update default for no_suspend from None to 0 in XendCheckpoint.py:save
 - update logoutput in setMigrateConstraints
 - change xm migrate defaults from None to 0
 - add new options to xl.1
 - fix syntax error in XendDomain.py:domain_migrate_constraints_set
 - fix xm migrate -N option name to match xl migrate

v3:
 - move logic errors in libxl__domain_suspend and fixed help text in
   cmd_table to separate patches
 - fix syntax error in XendCheckpoint.py
 - really pass max_iters and max_factor in libxl__xc_domain_save
 - make libxl_domain_suspend_0x040200 declaration globally visible
 - bump libxenlight.so SONAME from 2.0 to 2.1 due to changed
   libxl_domain_suspend

v2:
 - use LIBXL_API_VERSION and define libxl_domain_suspend_0x040200
 - fix logic error in min_reached check in xc_domain_save
 - add longopts
 - update --help text
 - correct description of migrate --help text

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r fd711ebdce9a -r 785c8f34e1f8 docs/man/xl.pod.1
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -391,6 +391,18 @@ Send <config> instead of config file fro
 
 Print huge (!) amount of debug during the migration process.
 
+=item B<-M> I<number>, B<--max_iters> I<number>
+
+Number of iterations before final suspend (default: 30)
+
+=item B<-m> I<factor>, B<--max_factor> I<factor>
+
+Max amount of memory to transfer before final suspend (default: 3*RAM)
+
+=item B<-N>, B<--no_suspend>
+
+Abort migration instead of doing final suspend.
+
 =back
 
 =item B<remus> [I<OPTIONS>] I<domain-id> I<host>
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -43,6 +43,8 @@
 */
 #define DEF_MAX_ITERS   29   /* limit us to 30 times round loop   */
 #define DEF_MAX_FACTOR   3   /* never send more than 3x p2m_size  */
+/* Suspend guest for final transit once number of dirty pages is that low */
+#define DEF_MIN_REMAINING 50
 
 struct save_ctx {
     unsigned long hvirt_start; /* virtual starting address of the hypervisor */
@@ -804,6 +806,7 @@ int xc_domain_save(xc_interface *xch, in
     int rc = 1, frc, i, j, last_iter = 0, iter = 0;
     int live  = (flags & XCFLAGS_LIVE);
     int debug = (flags & XCFLAGS_DEBUG);
+    int no_suspend = (flags & XCFLAGS_DOMSAVE_NOSUSPEND);
     int superpages = !!hvm;
     int race = 0, sent_last_iter, skip_this_iter = 0;
     unsigned int sent_this_iter = 0;
@@ -1525,10 +1528,20 @@ int xc_domain_save(xc_interface *xch, in
 
         if ( live )
         {
+            int min_reached = sent_this_iter + skip_this_iter <
DEF_MIN_REMAINING;
             if ( (iter >= max_iters) ||
-                 (sent_this_iter+skip_this_iter < 50) ||
+                 min_reached ||
                  (total_sent > dinfo->p2m_size*max_factor) )
             {
+                if ( !min_reached && no_suspend )
+                {
+                    ERROR("Live migration aborted, as requested. (guest
too busy?)"
+                     " total_sent %lu iter %d, max_iters %u max_factor
%u",
+                      total_sent, iter, max_iters, max_factor);
+                    rc = 1;
+                    goto out;
+                }
+
                 DPRINTF("Start last iteration\n");
                 last_iter = 1;
 
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxc/xenguest.h
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -28,6 +28,8 @@
 #define XCFLAGS_HVM       4
 #define XCFLAGS_STDVGA    8
 #define XCFLAGS_CHECKPOINT_COMPRESS    16
+#define XCFLAGS_DOMSAVE_NOSUSPEND      32
+
 #define X86_64_B_SIZE   64 
 #define X86_32_B_SIZE   32
 
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/Makefile
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -5,7 +5,7 @@
 XEN_ROOT = $(CURDIR)/../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-MAJOR = 2.0
+MAJOR = 2.1
 MINOR = 0
 
 XLUMAJOR = 1.0
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -756,7 +756,8 @@ static void domain_suspend_cb(libxl__egc
 
 }
 
-int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
+static int __libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, int max_iters, int max_factor,
                          const libxl_asyncop_how *ao_how)
 {
     AO_CREATE(ctx, domid, ao_how);
@@ -779,6 +780,9 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     dss->type = type;
     dss->live = flags & LIBXL_SUSPEND_LIVE;
     dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+    dss->max_iters = max_iters;
+    dss->max_factor = max_factor;
+    dss->xlflags = flags;
 
     libxl__domain_suspend(egc, dss);
     return AO_INPROGRESS;
@@ -787,6 +791,20 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     return AO_ABORT(rc);
 }
 
+int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, const libxl_asyncop_how *ao_how)
+{
+	return __libxl_domain_suspend(ctx, domid, fd, flags, 0,0, ao_how);
+}
+
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
+                         int max_iters, int max_factor,
+                         const libxl_asyncop_how *ao_how)
+{
+	return __libxl_domain_suspend(ctx, domid, fd, flags, max_iters,
+                                      max_factor, ao_how);
+}
+
 int libxl_domain_pause(libxl_ctx *ctx, uint32_t domid)
 {
     int ret;
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
 void libxl_domain_config_init(libxl_domain_config *d_config);
 void libxl_domain_config_dispose(libxl_domain_config *d_config);
 
-int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
                          int flags, /* LIBXL_SUSPEND_* */
                          const libxl_asyncop_how *ao_how)
                          LIBXL_EXTERNAL_CALLERS_ONLY;
+#ifdef LIBXL_API_VERSION
+#if LIBXL_API_VERSION == 0x040200
+#define libxl_domain_suspend libxl_domain_suspend_0x040200
+#endif
+#else
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, /* LIBXL_SUSPEND_* */
+                         int max_iters, int max_factor,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+#endif
+
 #define LIBXL_SUSPEND_DEBUG 1
 #define LIBXL_SUSPEND_LIVE 2
+#define LIBXL_SUSPEND_NO_FINAL_SUSPEND 4
 
 /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
  *   If this parameter is true, use co-operative resume. The guest
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
 
     dss->xcflags = (live ? XCFLAGS_LIVE : 0)
           | (debug ? XCFLAGS_DEBUG : 0)
+          | (dss->xlflags & LIBXL_SUSPEND_NO_FINAL_SUSPEND ?
XCFLAGS_DOMSAVE_NOSUSPEND : 0)
           | (dss->hvm ? XCFLAGS_HVM : 0);
 
     dss->suspend_eventchn = -1;
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2253,6 +2253,9 @@ struct libxl__domain_suspend_state {
     xc_evtchn *xce; /* event channel handle */
     int suspend_eventchn;
     int hvm;
+    int max_iters;
+    int max_factor;
+    int xlflags;
     int xcflags;
     int guest_responded;
     const char *dm_savefile;
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl_save_callout.c
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -108,8 +108,8 @@ void libxl__xc_domain_save(libxl__egc *e
     }
 
     const unsigned long argnums[] = {
-        dss->domid, 0, 0, dss->xcflags, dss->hvm,
vm_generationid_addr,
-        toolstack_data_fd, toolstack_data_len,
+        dss->domid, dss->max_iters, dss->max_factor, dss->xcflags,
dss->hvm,
+        vm_generationid_addr, toolstack_data_fd, toolstack_data_len,
         cbflags,
     };
 
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
 
     save_domain_core_writeconfig(fd, filename, config_data, config_len);
 
-    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
+    int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0, NULL);
     close(fd);
 
     if (rc < 0)
@@ -3332,6 +3332,7 @@ static void migrate_do_preamble(int send
 }
 
 static void migrate_domain(uint32_t domid, const char *rune, int debug,
+                           int max_iters, int max_factor, int no_suspend,
                            const char *override_config_file)
 {
     pid_t child = -1;
@@ -3360,7 +3361,10 @@ static void migrate_domain(uint32_t domi
 
     if (debug)
         flags |= LIBXL_SUSPEND_DEBUG;
-    rc = libxl_domain_suspend(ctx, domid, send_fd, flags, NULL);
+    if (no_suspend)
+        flags |= LIBXL_SUSPEND_NO_FINAL_SUSPEND;
+
+    rc = libxl_domain_suspend(ctx, domid, send_fd, flags, max_iters,
max_factor, NULL);
     if (rc) {
         fprintf(stderr, "migration sender: libxl_domain_suspend
failed"
                 " (rc=%d)\n", rc);
@@ -3753,8 +3757,16 @@ int main_migrate(int argc, char **argv)
     char *rune = NULL;
     char *host;
     int opt, daemonize = 1, monitor = 1, debug = 0;
-
-    SWITCH_FOREACH_OPT(opt, "FC:s:ed", NULL, "migrate", 2)
{
+    int max_iters = 0, max_factor = 0, no_suspend = 0;
+    static struct option opts[] = {
+        {"max_iters", 1, 0, ''M''},
+        {"max_factor", 1, 0, ''m''},
+        {"no_suspend", 0, 0, ''N''},
+        COMMON_LONG_OPTS,
+        {0, 0, 0, 0}
+    };
+
+    SWITCH_FOREACH_OPT(opt, "FC:s:edM:m:N", opts,
"migrate", 2) {
     case ''C'':
         config_filename = optarg;
         break;
@@ -3771,6 +3783,15 @@ int main_migrate(int argc, char **argv)
     case ''d'':
         debug = 1;
         break;
+    case ''M'':
+        max_iters = atoi(optarg);
+        break;
+    case ''m'':
+        max_factor = atoi(optarg);
+        break;
+    case ''N'':
+        no_suspend = 1;
+        break;
     }
 
     domid = find_domain(argv[optind]);
@@ -3786,7 +3807,7 @@ int main_migrate(int argc, char **argv)
             return 1;
     }
 
-    migrate_domain(domid, rune, debug, config_filename);
+    migrate_domain(domid, rune, debug, max_iters, max_factor, no_suspend,
config_filename);
     return 0;
 }
 
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -147,14 +147,19 @@ struct cmd_spec cmd_table[] = {
       &main_migrate, 0, 1,
       "Migrate a domain to another host",
       "[options] <Domain> <host>",
-      "-h              Print this help.\n"
-      "-C <config>     Send <config> instead of config file
from creation.\n"
-      "-s <sshcommand> Use <sshcommand> instead of ssh. 
String will be passed\n"
-      "                to sh. If empty, run <host> instead of ssh
<host> xl\n"
-      "                migrate-receive [-d -e]\n"
-      "-e              Do not wait in the background (on <host>) for
the death\n"
-      "                of the domain.\n"
-      "-d              Print huge (!) amount of debug during the migration
process."
+      "-h               Print this help.\n"
+      "-C <config>      Send <config> instead of config file
from creation.\n"
+      "-s <sshcommand>  Use <sshcommand> instead of ssh. 
String will be passed\n"
+      "                 to sh. If empty, run <host> instead of ssh
<host> xl\n"
+      "                 migrate-receive [-d -e]\n"
+      "-e               Do not wait in the background (on <host>)
for the death\n"
+      "                 of the domain.\n"
+      "-d               Print huge (!) amount of debug during the
migration process.\n"
+      "-M <number>      Number of iterations before final suspend
(default: 30)\n"
+      "--max_iters <number>\n"
+      "-m <factor>      Max amount of memory to transfer before
final suspend (default: 3*RAM).\n"
+      "--max_factor <factor>\n"
+      "-N, --no_suspend Abort migration instead of doing final
suspend."
     },
     { "dump-core",
       &main_dump_core, 0, 1,
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/python/xen/xend/XendCheckpoint.py
--- a/tools/python/xen/xend/XendCheckpoint.py
+++ b/tools/python/xen/xend/XendCheckpoint.py
@@ -118,9 +118,19 @@ def save(fd, dominfo, network, live, dst
         # enabled. Passing "0" simply uses the defaults compiled into
         # libxenguest; see the comments and/or code in xc_linux_save() for
         # more information.
+        max_iters = dominfo.info.get(''max_iters'',
"0")
+        max_factor = dominfo.info.get(''max_factor'',
"0")
+        no_suspend = dominfo.info.get(''no_suspend'',
"0")
+        if max_iters == "None":
+            max_iters = "0"
+        if max_factor == "None":
+            max_factor = "0"
+        if no_suspend == "None":
+            no_suspend = "0"
         cmd = [xen.util.auxbin.pathTo(XC_SAVE), str(fd),
-               str(dominfo.getDomid()), "0", "0", 
-               str(int(live) | (int(hvm) << 2)) ]
+               str(dominfo.getDomid()),
+               max_iters, max_factor,
+               str(int(live) | (int(hvm) << 2) | (int(no_suspend)
<< 5)) ]
         log.debug("[xc_save]: %s", string.join(cmd))
 
         def saveInputHandler(line, tochild):
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py
+++ b/tools/python/xen/xend/XendDomain.py
@@ -1832,6 +1832,18 @@ class XendDomain:
             log.exception(ex)
             raise XendError(str(ex))
 
+    def domain_migrate_constraints_set(self, domid, max_iters, max_factor,
no_suspend):
+        """Set the Migrate Constraints of this domain.
+        @param domid: Domain ID or Name
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param no_suspend: Abort migration instead of doing final suspend
+        """
+        dominfo = self.domain_lookup_nr(domid)
+        if not dominfo:
+            raise XendInvalidDomain(str(domid))
+        dominfo.setMigrateConstraints(max_iters, max_factor, no_suspend)
+
     def domain_maxmem_set(self, domid, mem):
         """Set the memory limit for a domain.
 
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -1459,6 +1459,18 @@ class XendDomainInfo:
         pci_conf = self.info[''devices''][dev_uuid][1]
         return map(pci_dict_to_bdf_str, pci_conf[''devs''])
 
+    def setMigrateConstraints(self, max_iters, max_factor, no_suspend):
+        """Set the Migrate Constraints of this domain.
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param no_suspend: Abort migration instead of doing final suspend
+        """
+        log.debug("Setting migration constraints of domain %s (%s) to
''%s'' ''%s'' ''%s''.",
+                  self.info[''name_label''], str(self.domid),
max_iters, max_factor, no_suspend)
+        self.info[''max_iters''] = str(max_iters)
+        self.info[''max_factor''] = str(max_factor)
+        self.info[''no_suspend''] = str(no_suspend)
+
     def setMemoryTarget(self, target):
         """Set the memory target of this domain.
         @param target: In MiB.
diff -r fd711ebdce9a -r 785c8f34e1f8 tools/python/xen/xm/migrate.py
--- a/tools/python/xen/xm/migrate.py
+++ b/tools/python/xen/xm/migrate.py
@@ -55,6 +55,18 @@ gopts.opt(''change_home_server'',
short=''c
           fn=set_true, default=0,
           use="Change home server for managed domains.")
 
+gopts.opt(''max_iters'', short=''M'',
val=''max_iters'',
+          fn=set_int, default=0,
+          use="Number of iterations before final suspend (default:
30).")
+
+gopts.opt(''max_factor'', short=''m'',
val=''max_factor'',
+          fn=set_int, default=0,
+          use="Max amount of memory to transfer before final suspend
(default: 3*RAM).")
+
+gopts.opt(''no_suspend'', short=''N'',
+          fn=set_true, default=0,
+          use="Abort migration instead of doing final suspend.")
+
 def help():
     return str(gopts)
     
@@ -80,6 +92,10 @@ def main(argv):
         server.xenapi.VM.migrate(vm_ref, dst, bool(opts.vals.live),
                                  other_config)
     else:
+        server.xend.domain.migrate_constraints_set(dom,
+                                                   opts.vals.max_iters,
+                                                   opts.vals.max_factor,
+                                                   opts.vals.no_suspend)
         server.xend.domain.migrate(dom, dst, opts.vals.live,
                                    opts.vals.port,
                                    opts.vals.node,

On Wed, Jan 30, Ian Campbell wrote:
> On Mon, 2013-01-28 at 17:32 +0000, Olaf Hering wrote:
> > A variant of this change has been tested with xend, the patch below is
> > only compile tested. The changes to libxl change the API, is that
> > approach acceptable?
> 
> I''m afraid not, the compatibility requirements are covered in the
> comment near the top of libxl.h.
> 
> So you either need a new function or to leverage the LIBXL_API_VERSION
> define (which the user must supply) such that people providing 0x040200
> see the current interface and people providing 0x040300 (or nothing) see
> the new one.
And to avoid the API change at all, should max_iters and max_factor be
passed via xenstore to xc_domain_save()? So that either the caller of
xc_domain_save reads the values from xenstore, or the function itself
reads it from there. What do you think?


Olaf

On Mon, 2013-02-04 at 09:57 +0000, Olaf Hering wrote:
> On Wed, Jan 30, Ian Campbell wrote:
> 
> > On Mon, 2013-01-28 at 17:32 +0000, Olaf Hering wrote:
> > > A variant of this change has been tested with xend, the patch
below is
> > > only compile tested. The changes to libxl change the API, is that
> > > approach acceptable?
> > 
> > I''m afraid not, the compatibility requirements are covered in
the
> > comment near the top of libxl.h.
> > 
> > So you either need a new function or to leverage the LIBXL_API_VERSION
> > define (which the user must supply) such that people providing
0x040200
> > see the current interface and people providing 0x040300 (or nothing)
see
> > the new one.
> 
> And to avoid the API change at all, should max_iters and max_factor be
> passed via xenstore to xc_domain_save()? So that either the caller of
> xc_domain_save reads the values from xenstore, or the function itself
> reads it from there. What do you think?
Unfortunately libxc cannot access xenstore.

Ian.

On Mon, Feb 04, Ian Campbell wrote:
> On Mon, 2013-02-04 at 09:57 +0000, Olaf Hering wrote:
> > On Wed, Jan 30, Ian Campbell wrote:
> > 
> > > On Mon, 2013-01-28 at 17:32 +0000, Olaf Hering wrote:
> > > > A variant of this change has been tested with xend, the
patch below is
> > > > only compile tested. The changes to libxl change the API, is
that
> > > > approach acceptable?
> > > 
> > > I''m afraid not, the compatibility requirements are
covered in the
> > > comment near the top of libxl.h.
> > > 
> > > So you either need a new function or to leverage the
LIBXL_API_VERSION
> > > define (which the user must supply) such that people providing
0x040200
> > > see the current interface and people providing 0x040300 (or
nothing) see
> > > the new one.
> > 
> > And to avoid the API change at all, should max_iters and max_factor be
> > passed via xenstore to xc_domain_save()? So that either the caller of
> > xc_domain_save reads the values from xenstore, or the function itself
> > reads it from there. What do you think?
> 
> Unfortunately libxc cannot access xenstore.
In case of xm, it would be the xc_save binary, which can receive both
values from argv[].
In case of xl, it would be a xenstore write in main_migrate and a read
in libxl_domain_suspend. Which would cover also other callers of
libxl_domain_suspend.

Olaf

On Fri, 2013-02-01 at 19:34 +0000, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1359746832 -3600
> # Node ID 785c8f34e1f802106e53ca4d2c54dce55c8ee166
> # Parent  fd711ebdce9a58556d62c2daaf5d49ab06de4a3c
> tools: set migration constraints from cmdline
> 
> Add new options to xm/xl migrate to control the process of migration.
> The intention is to optionally abort the migration if it takes too long
> to migrate a busy guest due to the high number of dirty pages. Currently
> the guest is suspended to transfer the remaining dirty pages. This
> transfer can take too long, which can confuse the guest if its suspended
> for too long.
> 
> -M <number>   Number of iterations before final suspend (default: 30)
> --max_iters <number>
> 
> -m <factor>   Max amount of memory to transfer before final suspend
(default: 3*RAM)
> --max_factor <factor>
> 
> -N            Abort migration instead of doing final suspend.
> --no_suspend
Is this last one a debug option? (Having read the patch I think not, but
the description here doesn''t really explain it)
> The changes to libxl change the API, is that approach acceptable?
I think this is now along the right lines (see below though).
> v4:
>  - update default for no_suspend from None to 0 in XendCheckpoint.py:save
>  - update logoutput in setMigrateConstraints
>  - change xm migrate defaults from None to 0
>  - add new options to xl.1
>  - fix syntax error in XendDomain.py:domain_migrate_constraints_set
>  - fix xm migrate -N option name to match xl migrate
> 
> v3:
>  - move logic errors in libxl__domain_suspend and fixed help text in
>    cmd_table to separate patches
>  - fix syntax error in XendCheckpoint.py
>  - really pass max_iters and max_factor in libxl__xc_domain_save
>  - make libxl_domain_suspend_0x040200 declaration globally visible
>  - bump libxenlight.so SONAME from 2.0 to 2.1 due to changed
>    libxl_domain_suspend
> 
> v2:
>  - use LIBXL_API_VERSION and define libxl_domain_suspend_0x040200
>  - fix logic error in min_reached check in xc_domain_save
>  - add longopts
>  - update --help text
>  - correct description of migrate --help text
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r fd711ebdce9a -r 785c8f34e1f8 docs/man/xl.pod.1
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -391,6 +391,18 @@ Send <config> instead of config file fro
> 
>  Print huge (!) amount of debug during the migration process.
> 
> +=item B<-M> I<number>, B<--max_iters> I<number>
> +
> +Number of iterations before final suspend (default: 30)
> +
> +=item B<-m> I<factor>, B<--max_factor> I<factor>
> +
> +Max amount of memory to transfer before final suspend (default: 3*RAM)
> +
> +=item B<-N>, B<--no_suspend>
> +
> +Abort migration instead of doing final suspend.
> +
>  =back
> 
>  =item B<remus> [I<OPTIONS>] I<domain-id> I<host>
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxc/xc_domain_save.c
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
The changes to this file only seem to implement the -N and not the other
two?
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
>  void libxl_domain_config_init(libxl_domain_config *d_config);
>  void libxl_domain_config_dispose(libxl_domain_config *d_config);
> 
> -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> +int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
>                           int flags, /* LIBXL_SUSPEND_* */
>                           const libxl_asyncop_how *ao_how)
>                           LIBXL_EXTERNAL_CALLERS_ONLY;
> +#ifdef LIBXL_API_VERSION
> +#if LIBXL_API_VERSION == 0x040200
> +#define libxl_domain_suspend libxl_domain_suspend_0x040200
int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
                         int flags, /* LIBXL_SUSPEND_* */
                         int max_iters, int max_factor,
                         const libxl_asyncop_how *ao_how)
                         LIBXL_EXTERNAL_CALLERS_ONLY;
#ifdef LIBXL_API_VERSION
#if LIBXL_API_VERSION == 0x040200
#define libxl_domain_suspend(ctx, domid, fd, flags, ao_how) \
            libxl_domain_suspend(ctx, domid, fd, flags, 0, 0, ao_how) 
#endif /* LIBXL_API_VERSION == 0x040200 */
#endif /* defined(LIBXL_API_VERSION) */

Should work?

Not sure if
#if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION == 0x040200
works in CPP.

Maybe we should
#ifndef LIBXL_API_VERSION
#define LIBXL_API_VERSION LIBXL_API_VERSION_LATEST
#endif
?
> +
>  #define LIBXL_SUSPEND_DEBUG 1
>  #define LIBXL_SUSPEND_LIVE 2
> +#define LIBXL_SUSPEND_NO_FINAL_SUSPEND 4
(These should really be in the IDL, but this is a pre-existing
condition)

The name of this new option doesn''t quite describe what it does, since
it doesn''t disable the final suspend always, only under certain
condition.

LIBXL_SUSPEND_ABORT_ON_<xxx>

Where the <xxx> is tricky ;-)

<xxx> == DIRTYING_TOO_FAST
<xxx> == GUEST_TOO_BUSY
<xxx> == YOUR_SUGGESTIONS_HERE
> 
>  /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
>   *   If this parameter is true, use co-operative resume. The guest
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
> 
>      dss->xcflags = (live ? XCFLAGS_LIVE : 0)
>            | (debug ? XCFLAGS_DEBUG : 0)
> +          | (dss->xlflags & LIBXL_SUSPEND_NO_FINAL_SUSPEND ?
XCFLAGS_DOMSAVE_NOSUSPEND : 0)
>            | (dss->hvm ? XCFLAGS_HVM : 0);
> 
>      dss->suspend_eventchn = -1;
> diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
> 
>      save_domain_core_writeconfig(fd, filename, config_data, config_len);
> 
> -    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
> +    int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0, NULL);
Doesn''t this need to pass down the selected values?
> @@ -3753,8 +3757,16 @@ int main_migrate(int argc, char **argv)
>      char *rune = NULL;
>      char *host;
>      int opt, daemonize = 1, monitor = 1, debug = 0;
> -
> -    SWITCH_FOREACH_OPT(opt, "FC:s:ed", NULL,
"migrate", 2) {
> +    int max_iters = 0, max_factor = 0, no_suspend = 0;
> +    static struct option opts[] = {
> +        {"max_iters", 1, 0, ''M''},
> +        {"max_factor", 1, 0, ''m''},
> +        {"no_suspend", 0, 0, ''N''},
I think _ in arguments is pretty ugly... But I see we have others
already.
> diff -r fd711ebdce9a -r 785c8f34e1f8
tools/python/xen/xend/XendCheckpoint.py
> --- a/tools/python/xen/xend/XendCheckpoint.py
> +++ b/tools/python/xen/xend/XendCheckpoint.py
I didn''t look at any of the python stuff.

Ian.

On Mon, 2013-02-04 at 13:09 +0000, Olaf Hering wrote:
> On Mon, Feb 04, Ian Campbell wrote:
> 
> > On Mon, 2013-02-04 at 09:57 +0000, Olaf Hering wrote:
> > > On Wed, Jan 30, Ian Campbell wrote:
> > > 
> > > > On Mon, 2013-01-28 at 17:32 +0000, Olaf Hering wrote:
> > > > > A variant of this change has been tested with xend, the
patch below is
> > > > > only compile tested. The changes to libxl change the
API, is that
> > > > > approach acceptable?
> > > > 
> > > > I''m afraid not, the compatibility requirements are
covered in the
> > > > comment near the top of libxl.h.
> > > > 
> > > > So you either need a new function or to leverage the
LIBXL_API_VERSION
> > > > define (which the user must supply) such that people
providing 0x040200
> > > > see the current interface and people providing 0x040300 (or
nothing) see
> > > > the new one.
> > > 
> > > And to avoid the API change at all, should max_iters and
max_factor be
> > > passed via xenstore to xc_domain_save()? So that either the
caller of
> > > xc_domain_save reads the values from xenstore, or the function
itself
> > > reads it from there. What do you think?
> > 
> > Unfortunately libxc cannot access xenstore.
> 
> In case of xm, it would be the xc_save binary, which can receive both
> values from argv[].
> In case of xl, it would be a xenstore write in main_migrate and a read
> in libxl_domain_suspend. Which would cover also other callers of
> libxl_domain_suspend.
I see, I think this is worse than the LIBXL_API_VERSION approach you''ve
already implemented.

Also, now I think about it, users of libxl are not expected to have to
use xenstore either.

Ian.

On Mon, Feb 04, Ian Campbell wrote:
> On Fri, 2013-02-01 at 19:34 +0000, Olaf Hering wrote:
> > # HG changeset patch
> > # User Olaf Hering <olaf@aepfle.de>
> > # Date 1359746832 -3600
> > # Node ID 785c8f34e1f802106e53ca4d2c54dce55c8ee166
> > # Parent  fd711ebdce9a58556d62c2daaf5d49ab06de4a3c
> > tools: set migration constraints from cmdline
> > 
> > Add new options to xm/xl migrate to control the process of migration.
> > The intention is to optionally abort the migration if it takes too
long
> > to migrate a busy guest due to the high number of dirty pages.
Currently
> > the guest is suspended to transfer the remaining dirty pages. This
> > transfer can take too long, which can confuse the guest if its
suspended
> > for too long.
> > 
> > -M <number>   Number of iterations before final suspend
(default: 30)
> > --max_iters <number>
> > 
> > -m <factor>   Max amount of memory to transfer before final
suspend (default: 3*RAM)
> > --max_factor <factor>
> > 
> > -N            Abort migration instead of doing final suspend.
> > --no_suspend
> 
> Is this last one a debug option? (Having read the patch I think not, but
> the description here doesn''t really explain it)
No, its not debug. And the naming of that -N option is not really good,
I agree.

What it is supposed to do is to avoid the final suspend+migrate+resume
step when either there were too many iterations or too many pages
transfered when the guest continues to dirty many pages. Its not
predictable how long the guest will be suspended to transfer the
remaining pages to the new host. I think even transfering 1GB RAM at
1000/GBs takes maybe 10 seconds and with large guests there may be
several GB dirty pages, so that a guest may be suspended for a minute.
This caused issues in a customer environment.

Instead the migration should be aborted and the guest should continue to
run on the old host.

> >  =item B<remus> [I<OPTIONS>] I<domain-id>
I<host>
> > diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxc/xc_domain_save.c
> > --- a/tools/libxc/xc_domain_save.c
> > +++ b/tools/libxc/xc_domain_save.c
> 
> The changes to this file only seem to implement the -N and not the other
> two?
xc_domain_save already has max_iters and max_factor as arguments.
> > diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl.h
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
> >  void libxl_domain_config_init(libxl_domain_config *d_config);
> >  void libxl_domain_config_dispose(libxl_domain_config *d_config);
> > 
> > -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> > +int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int
fd,
> >                           int flags, /* LIBXL_SUSPEND_* */
> >                           const libxl_asyncop_how *ao_how)
> >                           LIBXL_EXTERNAL_CALLERS_ONLY;
> > +#ifdef LIBXL_API_VERSION
> > +#if LIBXL_API_VERSION == 0x040200
> > +#define libxl_domain_suspend libxl_domain_suspend_0x040200
> 
> int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
>                          int flags, /* LIBXL_SUSPEND_* */
>                          int max_iters, int max_factor,
>                          const libxl_asyncop_how *ao_how)
>                          LIBXL_EXTERNAL_CALLERS_ONLY;
> #ifdef LIBXL_API_VERSION
> #if LIBXL_API_VERSION == 0x040200
> #define libxl_domain_suspend(ctx, domid, fd, flags, ao_how) \
>             libxl_domain_suspend(ctx, domid, fd, flags, 0, 0, ao_how) 
> #endif /* LIBXL_API_VERSION == 0x040200 */
> #endif /* defined(LIBXL_API_VERSION) */
> 
> Should work?
That may work, have to try it. In the end if we make such a change it
would serve as example for other upcoming API changes.
> >  #define LIBXL_SUSPEND_DEBUG 1
> >  #define LIBXL_SUSPEND_LIVE 2
> > +#define LIBXL_SUSPEND_NO_FINAL_SUSPEND 4
> 
> (These should really be in the IDL, but this is a pre-existing
> condition)
> 
> The name of this new option doesn''t quite describe what it does,
since
> it doesn''t disable the final suspend always, only under certain
> condition.
> 
> LIBXL_SUSPEND_ABORT_ON_<xxx>
> 
> Where the <xxx> is tricky ;-)
> 
> <xxx> == DIRTYING_TOO_FAST
> <xxx> == GUEST_TOO_BUSY
> <xxx> == YOUR_SUGGESTIONS_HERE
Thats alot more descriptive, thanks. DIRTYING_TOO_FAST describes it well
I think.

> >  /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param
fast )]
> >   *   If this parameter is true, use co-operative resume. The guest
> > diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl_dom.c
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
> > 
> >      dss->xcflags = (live ? XCFLAGS_LIVE : 0)
> >            | (debug ? XCFLAGS_DEBUG : 0)
> > +          | (dss->xlflags & LIBXL_SUSPEND_NO_FINAL_SUSPEND ?
XCFLAGS_DOMSAVE_NOSUSPEND : 0)
> >            | (dss->hvm ? XCFLAGS_HVM : 0);
> > 
> >      dss->suspend_eventchn = -1;
> 
> > diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/xl_cmdimpl.c
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
> > 
> >      save_domain_core_writeconfig(fd, filename, config_data,
config_len);
> > 
> > -    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
> > +    int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0, NULL);
> 
> Doesn''t this need to pass down the selected values?
save_domain is different from migrate_domain, I think xl save will
suspend the guest anyway? But I notice the -c option for "save", so
perhaps it would be useful to catch busy guests as well here?


Olaf

On Mon, 2013-02-04 at 13:41 +0000, Olaf Hering wrote:
> On Mon, Feb 04, Ian Campbell wrote:
> 
> > On Fri, 2013-02-01 at 19:34 +0000, Olaf Hering wrote:
> > > # HG changeset patch
> > > # User Olaf Hering <olaf@aepfle.de>
> > > # Date 1359746832 -3600
> > > # Node ID 785c8f34e1f802106e53ca4d2c54dce55c8ee166
> > > # Parent  fd711ebdce9a58556d62c2daaf5d49ab06de4a3c
> > > tools: set migration constraints from cmdline
> > > 
> > > Add new options to xm/xl migrate to control the process of
migration.
> > > The intention is to optionally abort the migration if it takes
too long
> > > to migrate a busy guest due to the high number of dirty pages.
Currently
> > > the guest is suspended to transfer the remaining dirty pages.
This
> > > transfer can take too long, which can confuse the guest if its
suspended
> > > for too long.
> > > 
> > > -M <number>   Number of iterations before final suspend
(default: 30)
> > > --max_iters <number>
> > > 
> > > -m <factor>   Max amount of memory to transfer before final
suspend (default: 3*RAM)
> > > --max_factor <factor>
> > > 
> > > -N            Abort migration instead of doing final suspend.
> > > --no_suspend
> > 
> > Is this last one a debug option? (Having read the patch I think not,
but
> > the description here doesn''t really explain it)
> 
> No, its not debug. And the naming of that -N option is not really good,
> I agree.
> 
> What it is supposed to do is to avoid the final suspend+migrate+resume
> step when either there were too many iterations or too many pages
> transfered when the guest continues to dirty many pages. Its not
> predictable how long the guest will be suspended to transfer the
> remaining pages to the new host. I think even transfering 1GB RAM at
> 1000/GBs takes maybe 10 seconds and with large guests there may be
> several GB dirty pages, so that a guest may be suspended for a minute.
> This caused issues in a customer environment.
> 
> Instead the migration should be aborted and the guest should continue to
> run on the old host.
That''s what I understood from reading the code, can we make the docs
say
it too ;-)
> 
> 
> > >  =item B<remus> [I<OPTIONS>] I<domain-id>
I<host>
> > > diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxc/xc_domain_save.c
> > > --- a/tools/libxc/xc_domain_save.c
> > > +++ b/tools/libxc/xc_domain_save.c
> > 
> > The changes to this file only seem to implement the -N and not the
other
> > two?
> 
> xc_domain_save already has max_iters and max_factor as arguments.
Ah, I didn''t appreciate that.

BTW did you want to add a way to override DEF_MIN_REMAINING?
> 
> > > diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl.h
> > > --- a/tools/libxl/libxl.h
> > > +++ b/tools/libxl/libxl.h
> > > @@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
> > >  void libxl_domain_config_init(libxl_domain_config *d_config);
> > >  void libxl_domain_config_dispose(libxl_domain_config *d_config);
> > > 
> > > -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> > > +int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t
domid, int fd,
> > >                           int flags, /* LIBXL_SUSPEND_* */
> > >                           const libxl_asyncop_how *ao_how)
> > >                           LIBXL_EXTERNAL_CALLERS_ONLY;
> > > +#ifdef LIBXL_API_VERSION
> > > +#if LIBXL_API_VERSION == 0x040200
> > > +#define libxl_domain_suspend libxl_domain_suspend_0x040200
> > 
> > int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> >                          int flags, /* LIBXL_SUSPEND_* */
> >                          int max_iters, int max_factor,
> >                          const libxl_asyncop_how *ao_how)
> >                          LIBXL_EXTERNAL_CALLERS_ONLY;
> > #ifdef LIBXL_API_VERSION
> > #if LIBXL_API_VERSION == 0x040200
> > #define libxl_domain_suspend(ctx, domid, fd, flags, ao_how) \
> >             libxl_domain_suspend(ctx, domid, fd, flags, 0, 0, ao_how) 
> > #endif /* LIBXL_API_VERSION == 0x040200 */
> > #endif /* defined(LIBXL_API_VERSION) */
> > 
> > Should work?
> 
> That may work, have to try it. In the end if we make such a change it
> would serve as example for other upcoming API changes.
Agreed.
> > >  /* @param suspend_cancel [from xenctrl.h:xc_domain_resume(
@param fast )]
> > >   *   If this parameter is true, use co-operative resume. The
guest
> > > diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/libxl_dom.c
> > > --- a/tools/libxl/libxl_dom.c
> > > +++ b/tools/libxl/libxl_dom.c
> > > @@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
> > > 
> > >      dss->xcflags = (live ? XCFLAGS_LIVE : 0)
> > >            | (debug ? XCFLAGS_DEBUG : 0)
> > > +          | (dss->xlflags &
LIBXL_SUSPEND_NO_FINAL_SUSPEND ? XCFLAGS_DOMSAVE_NOSUSPEND : 0)
> > >            | (dss->hvm ? XCFLAGS_HVM : 0);
> > > 
> > >      dss->suspend_eventchn = -1;
> > 
> > > diff -r fd711ebdce9a -r 785c8f34e1f8 tools/libxl/xl_cmdimpl.c
> > > --- a/tools/libxl/xl_cmdimpl.c
> > > +++ b/tools/libxl/xl_cmdimpl.c
> > > @@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
> > > 
> > >      save_domain_core_writeconfig(fd, filename, config_data,
config_len);
> > > 
> > > -    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
> > > +    int rc = libxl_domain_suspend(ctx, domid, fd, 0, 0, 0,
NULL);
> > 
> > Doesn''t this need to pass down the selected values?
> 
> save_domain is different from migrate_domain, I think xl save will
> suspend the guest anyway?
Oh yes, I didn''t spot this as the save path (despite it being right
there in the function name).
>  But I notice the -c option for "save", so
> perhaps it would be useful to catch busy guests as well here?
I''m not really sure what the usecase is for save -c. I''d be a
bit
inclined to leave it until someone wants it I think.

On Mon, Feb 04, Ian Campbell wrote:
> > Instead the migration should be aborted and the guest should continue
to
> > run on the old host.
> 
> That''s what I understood from reading the code, can we make the
docs say
> it too ;-)
-A --abort-if-busy maybe?
> > > >  =item B<remus> [I<OPTIONS>] I<domain-id>
I<host>
> > > > diff -r fd711ebdce9a -r 785c8f34e1f8
tools/libxc/xc_domain_save.c
> > > > --- a/tools/libxc/xc_domain_save.c
> > > > +++ b/tools/libxc/xc_domain_save.c
> > > 
> > > The changes to this file only seem to implement the -N and not
the other
> > > two?
> > 
> > xc_domain_save already has max_iters and max_factor as arguments.
> 
> Ah, I didn''t appreciate that.
> 
> BTW did you want to add a way to override DEF_MIN_REMAINING?
I did consider it, but havent looked closer how to do it.
> I''m not really sure what the usecase is for save -c. I''d
be a bit
> inclined to leave it until someone wants it I think.
save -c looks like a snapshot feature, but does not seem to take storage
into accound. So yes, I will leave it alone.

Olaf

On Mon, 2013-02-04 at 13:55 +0000, Olaf Hering wrote:
> On Mon, Feb 04, Ian Campbell wrote:
> 
> > > Instead the migration should be aborted and the guest should
continue to
> > > run on the old host.
> > 
> > That''s what I understood from reading the code, can we make
the docs say
> > it too ;-)
> 
> -A --abort-if-busy maybe?
Could work. Not sure if everything needs to have a short option, but I
suppose there''s no harm.
> > > > >  =item B<remus> [I<OPTIONS>]
I<domain-id> I<host>
> > > > > diff -r fd711ebdce9a -r 785c8f34e1f8
tools/libxc/xc_domain_save.c
> > > > > --- a/tools/libxc/xc_domain_save.c
> > > > > +++ b/tools/libxc/xc_domain_save.c
> > > > 
> > > > The changes to this file only seem to implement the -N and
not the other
> > > > two?
> > > 
> > > xc_domain_save already has max_iters and max_factor as arguments.
> > 
> > Ah, I didn''t appreciate that.
> > 
> > BTW did you want to add a way to override DEF_MIN_REMAINING?
> 
> I did consider it, but havent looked closer how to do it.
Might be best to do while you are changing the API anyway?

It just occurred to me, instead of adding lots of individual arguments
perhaps packing them into a (e.g.) libxl_save_properties and adding a
pointer would be a nicer and more extensible (in the future) interface?
> > I''m not really sure what the usecase is for save -c.
I''d be a bit
> > inclined to leave it until someone wants it I think.
> 
> save -c looks like a snapshot feature, but does not seem to take storage
> into accound. So yes, I will leave it alone.
Ack.

On Mon, Feb 04, Ian Campbell wrote:
> It just occurred to me, instead of adding lots of individual arguments
> perhaps packing them into a (e.g.) libxl_save_properties and adding a
> pointer would be a nicer and more extensible (in the future) interface?
Something like this (copy&paste from hg diff)?
I did not find a way to put also libxl_asyncop_how into
libxl_save_properties, the checker complains about missing
LIBXL_EXTERNAL_CALLERS_ONLY.

Olaf

diff -r 6087ff7a1aea tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -500,18 +500,24 @@ int libxl_domain_create_restore(libxl_ct
 void libxl_domain_config_init(libxl_domain_config *d_config);
 void libxl_domain_config_dispose(libxl_domain_config *d_config);

+typedef struct {
+    uint32_t domid;
+    int fd;
+    int flags; /* LIBXL_SUSPEND_* */
+    int max_iters;
+    int max_factor;
+} libxl_save_properties;
+
 int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
-                         int flags, /* LIBXL_SUSPEND_* */
-                         const libxl_asyncop_how *ao_how)
-                         LIBXL_EXTERNAL_CALLERS_ONLY;
+                                  int flags, /* LIBXL_SUSPEND_* */
+                                  const libxl_asyncop_how *ao_how)
+                                  LIBXL_EXTERNAL_CALLERS_ONLY;
 #ifdef LIBXL_API_VERSION
 #if LIBXL_API_VERSION == 0x040200
 #define libxl_domain_suspend libxl_domain_suspend_0x040200
 #endif
 #else
-int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
-                         int flags, /* LIBXL_SUSPEND_* */
-                         int max_iters, int max_factor,
+int libxl_domain_suspend(libxl_ctx *ctx, const libxl_save_properties *props,
                          const libxl_asyncop_how *ao_how)
                          LIBXL_EXTERNAL_CALLERS_ONLY;
 #endif

On Mon, 2013-02-04 at 18:43 +0000, Olaf Hering wrote:
> On Mon, Feb 04, Ian Campbell wrote:
> 
> > It just occurred to me, instead of adding lots of individual arguments
> > perhaps packing them into a (e.g.) libxl_save_properties and adding a
> > pointer would be a nicer and more extensible (in the future)
interface?
> 
> Something like this (copy&paste from hg diff)?
It''s a tricky balance of what goes in the struct and what goes in the
prototype but I didn''t imagine putting domid or fd into the struct,
domid because that''s how other libxl APIs behave and fd because it
isn''t
really a property of the save/migrate as such.

Moving flags into the struct could be optional is it complicates the
compat code too much.
> I did not find a way to put also libxl_asyncop_how into
> libxl_save_properties, the checker complains about missing
> LIBXL_EXTERNAL_CALLERS_ONLY.
I think this is fine to remain in the prototype alongside domid etc.
> 
> Olaf
> 
> diff -r 6087ff7a1aea tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -500,18 +500,24 @@ int libxl_domain_create_restore(libxl_ct
>  void libxl_domain_config_init(libxl_domain_config *d_config);
>  void libxl_domain_config_dispose(libxl_domain_config *d_config);
> 
> +typedef struct {
> +    uint32_t domid;
> +    int fd;
> +    int flags; /* LIBXL_SUSPEND_* */
> +    int max_iters;
> +    int max_factor;
> +} libxl_save_properties;
> +
>  int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
> -                         int flags, /* LIBXL_SUSPEND_* */
> -                         const libxl_asyncop_how *ao_how)
> -                         LIBXL_EXTERNAL_CALLERS_ONLY;
> +                                  int flags, /* LIBXL_SUSPEND_* */
> +                                  const libxl_asyncop_how *ao_how)
> +                                  LIBXL_EXTERNAL_CALLERS_ONLY;
>  #ifdef LIBXL_API_VERSION
>  #if LIBXL_API_VERSION == 0x040200
>  #define libxl_domain_suspend libxl_domain_suspend_0x040200
>  #endif
>  #else
> -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> -                         int flags, /* LIBXL_SUSPEND_* */
> -                         int max_iters, int max_factor,
> +int libxl_domain_suspend(libxl_ctx *ctx, const libxl_save_properties
*props,
>                           const libxl_asyncop_how *ao_how)
>                           LIBXL_EXTERNAL_CALLERS_ONLY;
>  #endif
>

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1360058505 -3600
# Node ID f474238aa14e1216e297ddb8fa20e8cdbacfa78a
# Parent  fd711ebdce9a58556d62c2daaf5d49ab06de4a3c
tools: set migration constraints from cmdline

Add new options to xm/xl migrate to control the process of migration.
The intention is to optionally abort the migration if it takes too long
to migrate a busy guest due to the high number of dirty pages. Currently
the guest is suspended to transfer the remaining dirty pages. This
transfer can take too long, which can confuse the guest if its suspended
for too long.

-M <number>   Number of iterations before final suspend (default: 30)
--max_iters <number>

-m <factor>   Max amount of memory to transfer before final suspend
(default: 3*RAM)
--max_factor <factor>

-A            Abort migration instead of doing final suspend.
--abort_if_busy



The changes to libxl change the API, is that approach acceptable?

v5:
 - adjust libxl_domain_suspend prototype, move flags, max_iters,
   max_factor into a new, optional struct libxl_save_properties
 - rename XCFLAGS_DOMSAVE_NOSUSPEND to XCFLAGS_DOMSAVE_ABORT_IF_BUSY
 - rename LIBXL_SUSPEND_NO_FINAL_SUSPEND to LIBXL_SUSPEND_ABORT_IF_BUSY
 - rename variables no_suspend to abort_if_busy
 - rename option -N/--no_suspend to -A/--abort_if_busy
 - update xl.1, extend description of -A option

v4:
 - update default for no_suspend from None to 0 in XendCheckpoint.py:save
 - update logoutput in setMigrateConstraints
 - change xm migrate defaults from None to 0
 - add new options to xl.1
 - fix syntax error in XendDomain.py:domain_migrate_constraints_set
 - fix xm migrate -N option name to match xl migrate

v3:
 - move logic errors in libxl__domain_suspend and fixed help text in
   cmd_table to separate patches
 - fix syntax error in XendCheckpoint.py
 - really pass max_iters and max_factor in libxl__xc_domain_save
 - make libxl_domain_suspend_0x040200 declaration globally visible
 - bump libxenlight.so SONAME from 2.0 to 2.1 due to changed
   libxl_domain_suspend

v2:
 - use LIBXL_API_VERSION and define libxl_domain_suspend_0x040200
 - fix logic error in min_reached check in xc_domain_save
 - add longopts
 - update --help text
 - correct description of migrate --help text

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r fd711ebdce9a -r f474238aa14e docs/man/xl.pod.1
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -391,6 +391,21 @@ Send <config> instead of config file fro
 
 Print huge (!) amount of debug during the migration process.
 
+=item B<-M> I<number>, B<--max_iters> I<number>
+
+Number of iterations before final suspend (default: 30)
+
+=item B<-m> I<factor>, B<--max_factor> I<factor>
+
+Max amount of memory to transfer before final suspend (default: 3*RAM)
+
+=item B<-A>, B<--abort_if_busy>
+
+Abort migration instead of doing final suspend/transfer/resume if the
+guest has still dirty pages after the number of iterations and/or the
+amount of RAM transfered. This avoids long periods of time were the
+guest is suspended.
+
 =back
 
 =item B<remus> [I<OPTIONS>] I<domain-id> I<host>
diff -r fd711ebdce9a -r f474238aa14e tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -43,6 +43,8 @@
 */
 #define DEF_MAX_ITERS   29   /* limit us to 30 times round loop   */
 #define DEF_MAX_FACTOR   3   /* never send more than 3x p2m_size  */
+/* Suspend guest for final transit once number of dirty pages is that low */
+#define DEF_MIN_REMAINING 50
 
 struct save_ctx {
     unsigned long hvirt_start; /* virtual starting address of the hypervisor */
@@ -804,6 +806,7 @@ int xc_domain_save(xc_interface *xch, in
     int rc = 1, frc, i, j, last_iter = 0, iter = 0;
     int live  = (flags & XCFLAGS_LIVE);
     int debug = (flags & XCFLAGS_DEBUG);
+    int abort_if_busy = (flags & XCFLAGS_DOMSAVE_ABORT_IF_BUSY);
     int superpages = !!hvm;
     int race = 0, sent_last_iter, skip_this_iter = 0;
     unsigned int sent_this_iter = 0;
@@ -1525,10 +1528,20 @@ int xc_domain_save(xc_interface *xch, in
 
         if ( live )
         {
+            int min_reached = sent_this_iter + skip_this_iter <
DEF_MIN_REMAINING;
             if ( (iter >= max_iters) ||
-                 (sent_this_iter+skip_this_iter < 50) ||
+                 min_reached ||
                  (total_sent > dinfo->p2m_size*max_factor) )
             {
+                if ( !min_reached && abort_if_busy )
+                {
+                    ERROR("Live migration aborted, as requested. (guest
too busy?)"
+                     " total_sent %lu iter %d, max_iters %u max_factor
%u",
+                      total_sent, iter, max_iters, max_factor);
+                    rc = 1;
+                    goto out;
+                }
+
                 DPRINTF("Start last iteration\n");
                 last_iter = 1;
 
diff -r fd711ebdce9a -r f474238aa14e tools/libxc/xenguest.h
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -28,6 +28,8 @@
 #define XCFLAGS_HVM       4
 #define XCFLAGS_STDVGA    8
 #define XCFLAGS_CHECKPOINT_COMPRESS    16
+#define XCFLAGS_DOMSAVE_ABORT_IF_BUSY  32
+
 #define X86_64_B_SIZE   64 
 #define X86_32_B_SIZE   32
 
diff -r fd711ebdce9a -r f474238aa14e tools/libxl/Makefile
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -5,7 +5,7 @@
 XEN_ROOT = $(CURDIR)/../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-MAJOR = 2.0
+MAJOR = 2.1
 MINOR = 0
 
 XLUMAJOR = 1.0
diff -r fd711ebdce9a -r f474238aa14e tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -756,8 +756,9 @@ static void domain_suspend_cb(libxl__egc
 
 }
 
-int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
-                         const libxl_asyncop_how *ao_how)
+static int __libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+                                  const libxl_save_properties *props,
+                                  const libxl_asyncop_how *ao_how)
 {
     AO_CREATE(ctx, domid, ao_how);
     int rc;
@@ -777,8 +778,13 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     dss->domid = domid;
     dss->fd = fd;
     dss->type = type;
-    dss->live = flags & LIBXL_SUSPEND_LIVE;
-    dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+    if (props) {
+        dss->live = props->xlflags & LIBXL_SUSPEND_LIVE;
+        dss->debug = props->xlflags & LIBXL_SUSPEND_DEBUG;
+        dss->max_iters = props->max_iters;
+        dss->max_factor = props->max_factor;
+        dss->xlflags = props->xlflags;
+    }
 
     libxl__domain_suspend(egc, dss);
     return AO_INPROGRESS;
@@ -787,6 +793,22 @@ int libxl_domain_suspend(libxl_ctx *ctx,
     return AO_ABORT(rc);
 }
 
+int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
+                         int flags, const libxl_asyncop_how *ao_how)
+{
+    libxl_save_properties props = {
+        .xlflags = flags,
+    };
+    return __libxl_domain_suspend(ctx, domid, fd, &props, ao_how);
+}
+
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+                         const libxl_save_properties *props,
+                         const libxl_asyncop_how *ao_how)
+{
+    return __libxl_domain_suspend(ctx, domid, fd, props, ao_how);
+}
+
 int libxl_domain_pause(libxl_ctx *ctx, uint32_t domid)
 {
     int ret;
diff -r fd711ebdce9a -r f474238aa14e tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -500,12 +500,30 @@ int libxl_domain_create_restore(libxl_ct
 void libxl_domain_config_init(libxl_domain_config *d_config);
 void libxl_domain_config_dispose(libxl_domain_config *d_config);
 
+typedef struct {
+    int xlflags; /* LIBXL_SUSPEND_* */
+    int max_iters;
+    int max_factor;
+} libxl_save_properties;
+
+int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int fd,
+                                  int flags, /* LIBXL_SUSPEND_* */
+                                  const libxl_asyncop_how *ao_how)
+                                  LIBXL_EXTERNAL_CALLERS_ONLY;
+#ifdef LIBXL_API_VERSION
+#if LIBXL_API_VERSION == 0x040200
+#define libxl_domain_suspend libxl_domain_suspend_0x040200
+#endif
+#else
 int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
-                         int flags, /* LIBXL_SUSPEND_* */
+                         const libxl_save_properties *props,
                          const libxl_asyncop_how *ao_how)
                          LIBXL_EXTERNAL_CALLERS_ONLY;
+#endif
+
 #define LIBXL_SUSPEND_DEBUG 1
 #define LIBXL_SUSPEND_LIVE 2
+#define LIBXL_SUSPEND_ABORT_IF_BUSY 4
 
 /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
  *   If this parameter is true, use co-operative resume. The guest
diff -r fd711ebdce9a -r f474238aa14e tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1240,6 +1240,7 @@ void libxl__domain_suspend(libxl__egc *e
 
     dss->xcflags = (live ? XCFLAGS_LIVE : 0)
           | (debug ? XCFLAGS_DEBUG : 0)
+          | (dss->xlflags & LIBXL_SUSPEND_ABORT_IF_BUSY ?
XCFLAGS_DOMSAVE_ABORT_IF_BUSY : 0)
           | (dss->hvm ? XCFLAGS_HVM : 0);
 
     dss->suspend_eventchn = -1;
diff -r fd711ebdce9a -r f474238aa14e tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2253,6 +2253,9 @@ struct libxl__domain_suspend_state {
     xc_evtchn *xce; /* event channel handle */
     int suspend_eventchn;
     int hvm;
+    int max_iters;
+    int max_factor;
+    int xlflags;
     int xcflags;
     int guest_responded;
     const char *dm_savefile;
diff -r fd711ebdce9a -r f474238aa14e tools/libxl/libxl_save_callout.c
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -108,8 +108,8 @@ void libxl__xc_domain_save(libxl__egc *e
     }
 
     const unsigned long argnums[] = {
-        dss->domid, 0, 0, dss->xcflags, dss->hvm,
vm_generationid_addr,
-        toolstack_data_fd, toolstack_data_len,
+        dss->domid, dss->max_iters, dss->max_factor, dss->xcflags,
dss->hvm,
+        vm_generationid_addr, toolstack_data_fd, toolstack_data_len,
         cbflags,
     };
 
diff -r fd711ebdce9a -r f474238aa14e tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3172,7 +3172,7 @@ static int save_domain(uint32_t domid, c
 
     save_domain_core_writeconfig(fd, filename, config_data, config_len);
 
-    int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
+    int rc = libxl_domain_suspend(ctx, domid, fd, NULL, NULL);
     close(fd);
 
     if (rc < 0)
@@ -3332,6 +3332,7 @@ static void migrate_do_preamble(int send
 }
 
 static void migrate_domain(uint32_t domid, const char *rune, int debug,
+                           int max_iters, int max_factor, int abort_if_busy,
                            const char *override_config_file)
 {
     pid_t child = -1;
@@ -3340,7 +3341,12 @@ static void migrate_domain(uint32_t domi
     char *away_domname;
     char rc_buf;
     uint8_t *config_data;
-    int config_len, flags = LIBXL_SUSPEND_LIVE;
+    int config_len;
+    libxl_save_properties props = {
+        .xlflags = LIBXL_SUSPEND_LIVE,
+        .max_iters = max_iters,
+        .max_factor = max_factor,
+    };
 
     save_domain_core_begin(domid, override_config_file,
                            &config_data, &config_len);
@@ -3359,8 +3365,11 @@ static void migrate_domain(uint32_t domi
     xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS, 0);
 
     if (debug)
-        flags |= LIBXL_SUSPEND_DEBUG;
-    rc = libxl_domain_suspend(ctx, domid, send_fd, flags, NULL);
+        props.xlflags |= LIBXL_SUSPEND_DEBUG;
+    if (abort_if_busy)
+        props.xlflags |= LIBXL_SUSPEND_ABORT_IF_BUSY;
+
+    rc = libxl_domain_suspend(ctx, domid, send_fd, &props, NULL);
     if (rc) {
         fprintf(stderr, "migration sender: libxl_domain_suspend
failed"
                 " (rc=%d)\n", rc);
@@ -3753,8 +3762,16 @@ int main_migrate(int argc, char **argv)
     char *rune = NULL;
     char *host;
     int opt, daemonize = 1, monitor = 1, debug = 0;
-
-    SWITCH_FOREACH_OPT(opt, "FC:s:ed", NULL, "migrate", 2)
{
+    int max_iters = 0, max_factor = 0, abort_if_busy = 0;
+    static struct option opts[] = {
+        {"max_iters", 1, 0, ''M''},
+        {"max_factor", 1, 0, ''m''},
+        {"abort_if_busy", 0, 0, ''A''},
+        COMMON_LONG_OPTS,
+        {0, 0, 0, 0}
+    };
+
+    SWITCH_FOREACH_OPT(opt, "FC:s:edM:m:A", opts,
"migrate", 2) {
     case ''C'':
         config_filename = optarg;
         break;
@@ -3771,6 +3788,15 @@ int main_migrate(int argc, char **argv)
     case ''d'':
         debug = 1;
         break;
+    case ''M'':
+        max_iters = atoi(optarg);
+        break;
+    case ''m'':
+        max_factor = atoi(optarg);
+        break;
+    case ''A'':
+        abort_if_busy = 1;
+        break;
     }
 
     domid = find_domain(argv[optind]);
@@ -3786,7 +3812,7 @@ int main_migrate(int argc, char **argv)
             return 1;
     }
 
-    migrate_domain(domid, rune, debug, config_filename);
+    migrate_domain(domid, rune, debug, max_iters, max_factor, abort_if_busy,
config_filename);
     return 0;
 }
 
diff -r fd711ebdce9a -r f474238aa14e tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -147,14 +147,19 @@ struct cmd_spec cmd_table[] = {
       &main_migrate, 0, 1,
       "Migrate a domain to another host",
       "[options] <Domain> <host>",
-      "-h              Print this help.\n"
-      "-C <config>     Send <config> instead of config file
from creation.\n"
-      "-s <sshcommand> Use <sshcommand> instead of ssh. 
String will be passed\n"
-      "                to sh. If empty, run <host> instead of ssh
<host> xl\n"
-      "                migrate-receive [-d -e]\n"
-      "-e              Do not wait in the background (on <host>) for
the death\n"
-      "                of the domain.\n"
-      "-d              Print huge (!) amount of debug during the migration
process."
+      "-h               Print this help.\n"
+      "-C <config>      Send <config> instead of config file
from creation.\n"
+      "-s <sshcommand>  Use <sshcommand> instead of ssh. 
String will be passed\n"
+      "                 to sh. If empty, run <host> instead of ssh
<host> xl\n"
+      "                 migrate-receive [-d -e]\n"
+      "-e               Do not wait in the background (on <host>)
for the death\n"
+      "                 of the domain.\n"
+      "-d               Print huge (!) amount of debug during the
migration process.\n"
+      "-M <number>      Number of iterations before final suspend
(default: 30)\n"
+      "--max_iters <number>\n"
+      "-m <factor>      Max amount of memory to transfer before
final suspend (default: 3*RAM).\n"
+      "--max_factor <factor>\n"
+      "-A, --abort_if_busy Abort migration instead of doing final
suspend."
     },
     { "dump-core",
       &main_dump_core, 0, 1,
diff -r fd711ebdce9a -r f474238aa14e tools/python/xen/xend/XendCheckpoint.py
--- a/tools/python/xen/xend/XendCheckpoint.py
+++ b/tools/python/xen/xend/XendCheckpoint.py
@@ -118,9 +118,19 @@ def save(fd, dominfo, network, live, dst
         # enabled. Passing "0" simply uses the defaults compiled into
         # libxenguest; see the comments and/or code in xc_linux_save() for
         # more information.
+        max_iters = dominfo.info.get(''max_iters'',
"0")
+        max_factor = dominfo.info.get(''max_factor'',
"0")
+        abort_if_busy = dominfo.info.get(''abort_if_busy'',
"0")
+        if max_iters == "None":
+            max_iters = "0"
+        if max_factor == "None":
+            max_factor = "0"
+        if abort_if_busy == "None":
+            abort_if_busy = "0"
         cmd = [xen.util.auxbin.pathTo(XC_SAVE), str(fd),
-               str(dominfo.getDomid()), "0", "0", 
-               str(int(live) | (int(hvm) << 2)) ]
+               str(dominfo.getDomid()),
+               max_iters, max_factor,
+               str(int(live) | (int(hvm) << 2) | (int(abort_if_busy)
<< 5)) ]
         log.debug("[xc_save]: %s", string.join(cmd))
 
         def saveInputHandler(line, tochild):
diff -r fd711ebdce9a -r f474238aa14e tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py
+++ b/tools/python/xen/xend/XendDomain.py
@@ -1832,6 +1832,18 @@ class XendDomain:
             log.exception(ex)
             raise XendError(str(ex))
 
+    def domain_migrate_constraints_set(self, domid, max_iters, max_factor,
abort_if_busy):
+        """Set the Migrate Constraints of this domain.
+        @param domid: Domain ID or Name
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param abort_if_busy: Abort migration instead of doing final suspend
+        """
+        dominfo = self.domain_lookup_nr(domid)
+        if not dominfo:
+            raise XendInvalidDomain(str(domid))
+        dominfo.setMigrateConstraints(max_iters, max_factor, abort_if_busy)
+
     def domain_maxmem_set(self, domid, mem):
         """Set the memory limit for a domain.
 
diff -r fd711ebdce9a -r f474238aa14e tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -1459,6 +1459,18 @@ class XendDomainInfo:
         pci_conf = self.info[''devices''][dev_uuid][1]
         return map(pci_dict_to_bdf_str, pci_conf[''devs''])
 
+    def setMigrateConstraints(self, max_iters, max_factor, abort_if_busy):
+        """Set the Migrate Constraints of this domain.
+        @param max_iters: Number of iterations before final suspend
+        @param max_factor: Max amount of memory to transfer before final
suspend
+        @param abort_if_busy: Abort migration instead of doing final suspend
+        """
+        log.debug("Setting migration constraints of domain %s (%s) to
''%s'' ''%s'' ''%s''.",
+                  self.info[''name_label''], str(self.domid),
max_iters, max_factor, abort_if_busy)
+        self.info[''max_iters''] = str(max_iters)
+        self.info[''max_factor''] = str(max_factor)
+        self.info[''abort_if_busy''] = str(abort_if_busy)
+
     def setMemoryTarget(self, target):
         """Set the memory target of this domain.
         @param target: In MiB.
diff -r fd711ebdce9a -r f474238aa14e tools/python/xen/xm/migrate.py
--- a/tools/python/xen/xm/migrate.py
+++ b/tools/python/xen/xm/migrate.py
@@ -55,6 +55,18 @@ gopts.opt(''change_home_server'',
short=''c
           fn=set_true, default=0,
           use="Change home server for managed domains.")
 
+gopts.opt(''max_iters'', short=''M'',
val=''max_iters'',
+          fn=set_int, default=0,
+          use="Number of iterations before final suspend (default:
30).")
+
+gopts.opt(''max_factor'', short=''m'',
val=''max_factor'',
+          fn=set_int, default=0,
+          use="Max amount of memory to transfer before final suspend
(default: 3*RAM).")
+
+gopts.opt(''abort_if_busy'', short=''A'',
+          fn=set_true, default=0,
+          use="Abort migration instead of doing final suspend.")
+
 def help():
     return str(gopts)
     
@@ -80,6 +92,10 @@ def main(argv):
         server.xenapi.VM.migrate(vm_ref, dst, bool(opts.vals.live),
                                  other_config)
     else:
+        server.xend.domain.migrate_constraints_set(dom,
+                                                   opts.vals.max_iters,
+                                                   opts.vals.max_factor,
+                                                   opts.vals.abort_if_busy)
         server.xend.domain.migrate(dom, dst, opts.vals.live,
                                    opts.vals.port,
                                    opts.vals.node,

On Tue, Feb 05, Ian Campbell wrote:
> On Mon, 2013-02-04 at 18:43 +0000, Olaf Hering wrote:
> > On Mon, Feb 04, Ian Campbell wrote:
> > 
> > > It just occurred to me, instead of adding lots of individual
arguments
> > > perhaps packing them into a (e.g.) libxl_save_properties and
adding a
> > > pointer would be a nicer and more extensible (in the future)
interface?
> > 
> > Something like this (copy&paste from hg diff)?
> 
> It''s a tricky balance of what goes in the struct and what goes in
the
> prototype but I didn''t imagine putting domid or fd into the
struct,
> domid because that''s how other libxl APIs behave and fd because it
isn''t
> really a property of the save/migrate as such.
> 
> Moving flags into the struct could be optional is it complicates the
> compat code too much.
I sent v5 with a new libxl_domain_suspend prototype. If that direction
is ok I will look how to improve the LIBXL_API_VERSION handling and how
to provide a knob for DEF_MIN_REMAINING.

While I do that I would like to drop the hvm parameter from
xc_domain_save because there is a flag for this.

Olaf

On Mon, Feb 04, Ian Campbell wrote:
> On Fri, 2013-02-01 at 19:34 +0000, Olaf Hering wrote:
> > +++ b/tools/libxl/libxl.h
> > @@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
> >  void libxl_domain_config_init(libxl_domain_config *d_config);
> >  void libxl_domain_config_dispose(libxl_domain_config *d_config);
> > 
> > -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> > +int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t domid, int
fd,
> >                           int flags, /* LIBXL_SUSPEND_* */
> >                           const libxl_asyncop_how *ao_how)
> >                           LIBXL_EXTERNAL_CALLERS_ONLY;
> > +#ifdef LIBXL_API_VERSION
> > +#if LIBXL_API_VERSION == 0x040200
> > +#define libxl_domain_suspend libxl_domain_suspend_0x040200
> 
> int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
>                          int flags, /* LIBXL_SUSPEND_* */
>                          int max_iters, int max_factor,
>                          const libxl_asyncop_how *ao_how)
>                          LIBXL_EXTERNAL_CALLERS_ONLY;
> #ifdef LIBXL_API_VERSION
> #if LIBXL_API_VERSION == 0x040200
> #define libxl_domain_suspend(ctx, domid, fd, flags, ao_how) \
>             libxl_domain_suspend(ctx, domid, fd, flags, 0, 0, ao_how) 
> #endif /* LIBXL_API_VERSION == 0x040200 */
> #endif /* defined(LIBXL_API_VERSION) */
> 
> Should work?
> 
> Not sure if
> #if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION == 0x040200
> works in CPP.
> 
> Maybe we should
> #ifndef LIBXL_API_VERSION
> #define LIBXL_API_VERSION LIBXL_API_VERSION_LATEST
> #endif
> ?
Here is another attempt to make use of LIBXL_API_VERSION. Two versions
just for testing. I would use the static inline variant because we code
in C, not cpp.


...
/* API compatibility. */
#ifdef LIBXL_API_VERSION
#if LIBXL_API_VERSION != 0x040200 && LIBXL_API_VERSION != 0x040300
#error Unknown LIBXL_API_VERSION
#endif
#endif

typedef struct {
    int xlflags; /* LIBXL_SUSPEND_* */
    int max_iters;
    int max_factor;
} libxl_save_properties;

int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
                         const libxl_save_properties *props,
                         const libxl_asyncop_how *ao_how)
                         LIBXL_EXTERNAL_CALLERS_ONLY;
#ifdef LIBXL_API_VERSION
#if LIBXL_API_VERSION == 0x040200
#define libxl_domain_suspend(__ctx, __domid, __fd, __flags, __ao_how) \
({ \
    libxl_save_properties __props = { .xlflags = (__flags) }; \
    int __ret = libxl_domain_suspend((__ctx), (__domid), (__fd), &__props,
(__ao_how)); \
    __ret; \
})
#elif LIBXL_API_VERSION == 0x040300
static inline int libxl_domain_suspend_0x040300(libxl_ctx *ctx, uint32_t domid,
int fd,
                          int flags, const libxl_asyncop_how *ao_how)
{
    libxl_save_properties props = { .xlflags = flags };
    return libxl_domain_suspend(ctx, domid, fd, &props, ao_how);
}
#define libxl_domain_suspend libxl_domain_suspend_0x040300
#endif
#endif


/* cat > t.c
 * gcc -Wall -o t t.c --save-temps -g
 * -I tools/libxc -I tools/libxl
 * -Ltools/libxc -L tools/libxl -lxenlight
 * -Wl,-rpath,$PWD/tools/libxc,-rpath,$PWD/tools/libxl
 */
#define LIBXL_API_VERSION  0x040300

#include "tools/libxl/libxl.h"

int main(void)
{
	int ret;
#if LIBXL_API_VERSION == 0x040200
	ret = libxl_domain_suspend(NULL, 0, 0, 0, NULL);
#elif LIBXL_API_VERSION == 0x040300
	ret = libxl_domain_suspend(NULL, 0, 0, 0, NULL);
#else
	ret = libxl_domain_suspend(NULL, 0, 0, NULL, NULL);
#endif
	return ret;
}


Olaf

On Tue, 2013-02-19 at 11:42 +0000, Olaf Hering wrote:
> On Mon, Feb 04, Ian Campbell wrote:
> 
> > On Fri, 2013-02-01 at 19:34 +0000, Olaf Hering wrote:
> > > +++ b/tools/libxl/libxl.h
> > > @@ -500,12 +500,25 @@ int libxl_domain_create_restore(libxl_ct
> > >  void libxl_domain_config_init(libxl_domain_config *d_config);
> > >  void libxl_domain_config_dispose(libxl_domain_config *d_config);
> > > 
> > > -int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> > > +int libxl_domain_suspend_0x040200(libxl_ctx *ctx, uint32_t
domid, int fd,
> > >                           int flags, /* LIBXL_SUSPEND_* */
> > >                           const libxl_asyncop_how *ao_how)
> > >                           LIBXL_EXTERNAL_CALLERS_ONLY;
> > > +#ifdef LIBXL_API_VERSION
> > > +#if LIBXL_API_VERSION == 0x040200
> > > +#define libxl_domain_suspend libxl_domain_suspend_0x040200
> > 
> > int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> >                          int flags, /* LIBXL_SUSPEND_* */
> >                          int max_iters, int max_factor,
> >                          const libxl_asyncop_how *ao_how)
> >                          LIBXL_EXTERNAL_CALLERS_ONLY;
> > #ifdef LIBXL_API_VERSION
> > #if LIBXL_API_VERSION == 0x040200
> > #define libxl_domain_suspend(ctx, domid, fd, flags, ao_how) \
> >             libxl_domain_suspend(ctx, domid, fd, flags, 0, 0, ao_how) 
> > #endif /* LIBXL_API_VERSION == 0x040200 */
> > #endif /* defined(LIBXL_API_VERSION) */
> > 
> > Should work?
> > 
> > Not sure if
> > #if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION ==
0x040200
> > works in CPP.
> > 
> > Maybe we should
> > #ifndef LIBXL_API_VERSION
> > #define LIBXL_API_VERSION LIBXL_API_VERSION_LATEST
> > #endif
> > ?
> 
> Here is another attempt to make use of LIBXL_API_VERSION. Two versions
> just for testing. I would use the static inline variant because we code
> in C, not cpp.
> 
> 
> ...
> /* API compatibility. */
> #ifdef LIBXL_API_VERSION
> #if LIBXL_API_VERSION != 0x040200 && LIBXL_API_VERSION != 0x040300
> #error Unknown LIBXL_API_VERSION
> #endif
> #endif
> 
> typedef struct {
>     int xlflags; /* LIBXL_SUSPEND_* */
Why is this called "xlflags"? xl isn''t the only user of this
interface.
>     int max_iters;
>     int max_factor;
> } libxl_save_properties;
s/save/suspend/ to match the function it is passed to? Perhaps
libxl_domain_suspend_properties?
> int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
>                          const libxl_save_properties *props,
>                          const libxl_asyncop_how *ao_how)
>                          LIBXL_EXTERNAL_CALLERS_ONLY;
> #ifdef LIBXL_API_VERSION
> #if LIBXL_API_VERSION == 0x040200
> #define libxl_domain_suspend(__ctx, __domid, __fd, __flags, __ao_how) \
> ({ \
>     libxl_save_properties __props = { .xlflags = (__flags) }; \
>     int __ret = libxl_domain_suspend((__ctx), (__domid), (__fd),
&__props, (__ao_how)); \
>     __ret; \
> })
> #elif LIBXL_API_VERSION == 0x040300
> static inline int libxl_domain_suspend_0x040300(libxl_ctx *ctx, uint32_t
domid, int fd,
>                           int flags, const libxl_asyncop_how *ao_how)
> {
>     libxl_save_properties props = { .xlflags = flags };
>     return libxl_domain_suspend(ctx, domid, fd, &props, ao_how);
> }
> #define libxl_domain_suspend libxl_domain_suspend_0x040300
I don''t understand the need for this alternative, you are defining
version 0x040300 in this patch and in the absence of LIBXL_API_VERSION
this should therefore be the expected interface I think. We haven''t
released Xen 0x040300 yet.
> #endif
> #endif
> 
> 
> /* cat > t.c
>  * gcc -Wall -o t t.c --save-temps -g
>  * -I tools/libxc -I tools/libxl
>  * -Ltools/libxc -L tools/libxl -lxenlight
>  * -Wl,-rpath,$PWD/tools/libxc,-rpath,$PWD/tools/libxl
>  */
> #define LIBXL_API_VERSION  0x040300
> 
> #include "tools/libxl/libxl.h"
> 
> int main(void)
> {
> 	int ret;
> #if LIBXL_API_VERSION == 0x040200
> 	ret = libxl_domain_suspend(NULL, 0, 0, 0, NULL);
> #elif LIBXL_API_VERSION == 0x040300
> 	ret = libxl_domain_suspend(NULL, 0, 0, 0, NULL);
> #else
> 	ret = libxl_domain_suspend(NULL, 0, 0, NULL, NULL);
> #endif
> 	return ret;
> }
> 
> 
> Olaf

On Tue, Feb 19, Ian Campbell wrote:
> > typedef struct {
> >     int xlflags; /* LIBXL_SUSPEND_* */
> 
> Why is this called "xlflags"? xl isn''t the only user of
this interface.
To make it clear that libxl is the consumer, not libxc. But I think the
comment above makes it clear that libxl flags are expected. I will
rename it to ''flags''.
> >     int max_iters;
> >     int max_factor;
> > } libxl_save_properties;
> 
> s/save/suspend/ to match the function it is passed to? Perhaps
> libxl_domain_suspend_properties?
Will rename it to libxl_domain_suspend_properties.
> > int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
> >                          const libxl_save_properties *props,
> >                          const libxl_asyncop_how *ao_how)
> >                          LIBXL_EXTERNAL_CALLERS_ONLY;
> > #ifdef LIBXL_API_VERSION
> > #if LIBXL_API_VERSION == 0x040200
> > #define libxl_domain_suspend(__ctx, __domid, __fd, __flags, __ao_how)
\
> > ({ \
> >     libxl_save_properties __props = { .xlflags = (__flags) }; \
> >     int __ret = libxl_domain_suspend((__ctx), (__domid), (__fd),
&__props, (__ao_how)); \
> >     __ret; \
> > })
> > #elif LIBXL_API_VERSION == 0x040300
> > static inline int libxl_domain_suspend_0x040300(libxl_ctx *ctx,
uint32_t domid, int fd,
> >                           int flags, const libxl_asyncop_how *ao_how)
> > {
> >     libxl_save_properties props = { .xlflags = flags };
> >     return libxl_domain_suspend(ctx, domid, fd, &props, ao_how);
> > }
> > #define libxl_domain_suspend libxl_domain_suspend_0x040300
> 
> I don''t understand the need for this alternative, you are defining
> version 0x040300 in this patch and in the absence of LIBXL_API_VERSION
> this should therefore be the expected interface I think. We
haven''t
> released Xen 0x040300 yet.
Sorry, I was in a hurry and did not explain the stuff above properly
before sending the mail: 

Both versions of a backwards compatible libxl_domain_suspend interface
work, they are here just for reference.
I like the static inline variant because its C and it does not depend on
the ''({ ... })'' syntax, which is a gcc feature AFAIK.


Olaf