[PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Ian Campbell]

ARM does not (currently) support migration, so stop offering tasty looking
treats like "xl migrate".

Apart from the UI improvement my intention is to use this in osstest to detect
whether to attempt the save/restore/migrate tests.

Other than the additions of the #define/#ifdef there is a tiny bit of code
motion ("dump-core" in the command list and core_dump_domain in the
implementations) which serves to put ifdeffable bits next to each other.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: goerge.dunlap@citrix.com
---
Release:
   My main motivation here is to be able to get a complete osstest run on armhf
   prior to Xen 4.4 and the lack of migration support is currently blocking
   that (fine) but is also blocking subsequent useful tests. This change will
   allow me to make osstest skip the unsupported functionality, and in a way where
   it will automatically start trying to test it as soon as it is implemented.
---
 tools/libxl/libxl.h       | 14 ++++++++++++++
 tools/libxl/xl.h          |  4 ++++
 tools/libxl/xl_cmdimpl.c  | 20 ++++++++++++--------
 tools/libxl/xl_cmdtable.c | 15 +++++++++------
 4 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 0b992d1..06bbca6 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -431,6 +431,20 @@
  */
 #define LIBXL_HAVE_SIGCHLD_SHARING 1
 
+/*
+ * LIBXL_HAVE_NO_SUSPEND_RESUME
+ *
+ * Is this is defined then the platform has no support for saving,
+ * restoring or migrating a domain. In this case the related functions
+ * should be expected to return failure. That is:
+ *  - libxl_domain_suspend
+ *  - libxl_domain_resume
+ *  - libxl_domain_remus_start
+ */
+#if defined(__arm__) || defined(__aarch64__)
+#define LIBXL_HAVE_NO_SUSPEND_RESUME 1
+#endif
+
 /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
  * called from within libxl itself. Callers outside libxl, who
  * do not #include libxl_internal.h, are fine. */
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index c876a33..f188708 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -43,10 +43,12 @@ int main_pciattach(int argc, char **argv);
 int main_pciassignable_add(int argc, char **argv);
 int main_pciassignable_remove(int argc, char **argv);
 int main_pciassignable_list(int argc, char **argv);
+#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
 int main_restore(int argc, char **argv);
 int main_migrate_receive(int argc, char **argv);
 int main_save(int argc, char **argv);
 int main_migrate(int argc, char **argv);
+#endif
 int main_dump_core(int argc, char **argv);
 int main_pause(int argc, char **argv);
 int main_unpause(int argc, char **argv);
@@ -104,7 +106,9 @@ int main_cpupoolnumasplit(int argc, char **argv);
 int main_getenforce(int argc, char **argv);
 int main_setenforce(int argc, char **argv);
 int main_loadpolicy(int argc, char **argv);
+#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
 int main_remus(int argc, char **argv);
+#endif
 int main_devd(int argc, char **argv);
 
 void help(const char *command);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index aff6f90..4fc46eb 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3384,6 +3384,15 @@ static void list_vm(void)
     libxl_vminfo_list_free(info, nb_vm);
 }
 
+static void core_dump_domain(uint32_t domid, const char *filename)
+{
+    int rc;
+
+    rc=libxl_domain_core_dump(ctx, domid, filename, NULL);
+    if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); }
+}
+
+#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
 static void save_domain_core_begin(uint32_t domid,
                                    const char *override_config_file,
                                    uint8_t **config_data_r,
@@ -3775,14 +3784,6 @@ static void migrate_domain(uint32_t domid, const char *rune, int debug,
     exit(-ERROR_BADFAIL);
 }
 
-static void core_dump_domain(uint32_t domid, const char *filename)
-{
-    int rc;
-
-    rc=libxl_domain_core_dump(ctx, domid, filename, NULL);
-    if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); }
-}
-
 static void migrate_receive(int debug, int daemonize, int monitor,
                             int send_fd, int recv_fd, int remus)
 {
@@ -4102,6 +4103,7 @@ int main_migrate(int argc, char **argv)
     migrate_domain(domid, rune, debug, config_filename);
     return 0;
 }
+#endif
 
 int main_dump_core(int argc, char **argv)
 {
@@ -7248,6 +7250,7 @@ done:
     return ret;
 }
 
+#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
 int main_remus(int argc, char **argv)
 {
     uint32_t domid;
@@ -7341,6 +7344,7 @@ int main_remus(int argc, char **argv)
     close(send_fd);
     return -ERROR_FAIL;
 }
+#endif
 
 int main_devd(int argc, char **argv)
 {
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index ebe0220..e8ab93a 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -137,6 +137,7 @@ struct cmd_spec cmd_table[] = {
       "                         -autopass\n"
       "--vncviewer-autopass     (consistency alias for --autopass)"
     },
+#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
     { "save",
       &main_save, 0, 1,
       "Save a domain state to restore later",
@@ -158,11 +159,6 @@ struct cmd_spec cmd_table[] = {
       "                of the domain.\n"
       "--debug         Print huge (!) amount of debug during the migration process."
     },
-    { "dump-core",
-      &main_dump_core, 0, 1,
-      "Core dump a domain",
-      "<Domain> <filename>"
-    },
     { "restore",
       &main_restore, 0, 1,
       "Restore a domain from a saved state",
@@ -179,6 +175,12 @@ struct cmd_spec cmd_table[] = {
       "Restore a domain from a saved state",
       "- for internal use only",
     },
+#endif
+    { "dump-core",
+      &main_dump_core, 0, 1,
+      "Core dump a domain",
+      "<Domain> <filename>"
+    },
     { "cd-insert",
       &main_cd_insert, 1, 1,
       "Insert a cdrom into a guest's cd drive",
@@ -474,6 +476,7 @@ struct cmd_spec cmd_table[] = {
       "Loads a new policy int the Flask Xen security module",
       "<policy file>",
     },
+#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
     { "remus",
       &main_remus, 0, 1,
       "Enable Remus HA for domain",
@@ -486,8 +489,8 @@ struct cmd_spec cmd_table[] = {
       "                        ssh <host> xl migrate-receive -r [-e]\n"
       "-e                      Do not wait in the background (on <host>) for the death\n"
       "                        of the domain."
-
     },
+#endif
     { "devd",
       &main_devd, 0, 1,
       "Daemon that listens for devices and launches backends",
-- 
1.8.5.2

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Ian Campbell]

Typoed George's address...

On Wed, 2014-02-12 at 14:27 +0000, Ian Campbell wrote:
> ARM does not (currently) support migration, so stop offering tasty looking
> treats like "xl migrate".
> 
> Apart from the UI improvement my intention is to use this in osstest to detect
> whether to attempt the save/restore/migrate tests.
> 
> Other than the additions of the #define/#ifdef there is a tiny bit of code
> motion ("dump-core" in the command list and core_dump_domain in the
> implementations) which serves to put ifdeffable bits next to each other.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: goerge.dunlap@citrix.com
> ---
> Release:
>    My main motivation here is to be able to get a complete osstest run on armhf
>    prior to Xen 4.4 and the lack of migration support is currently blocking
>    that (fine) but is also blocking subsequent useful tests. This change will
>    allow me to make osstest skip the unsupported functionality, and in a way where
>    it will automatically start trying to test it as soon as it is implemented.
> ---
>  tools/libxl/libxl.h       | 14 ++++++++++++++
>  tools/libxl/xl.h          |  4 ++++
>  tools/libxl/xl_cmdimpl.c  | 20 ++++++++++++--------
>  tools/libxl/xl_cmdtable.c | 15 +++++++++------
>  4 files changed, 39 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 0b992d1..06bbca6 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -431,6 +431,20 @@
>   */
>  #define LIBXL_HAVE_SIGCHLD_SHARING 1
>  
> +/*
> + * LIBXL_HAVE_NO_SUSPEND_RESUME
> + *
> + * Is this is defined then the platform has no support for saving,
> + * restoring or migrating a domain. In this case the related functions
> + * should be expected to return failure. That is:
> + *  - libxl_domain_suspend
> + *  - libxl_domain_resume
> + *  - libxl_domain_remus_start
> + */
> +#if defined(__arm__) || defined(__aarch64__)
> +#define LIBXL_HAVE_NO_SUSPEND_RESUME 1
> +#endif
> +
>  /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
>   * called from within libxl itself. Callers outside libxl, who
>   * do not #include libxl_internal.h, are fine. */
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index c876a33..f188708 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -43,10 +43,12 @@ int main_pciattach(int argc, char **argv);
>  int main_pciassignable_add(int argc, char **argv);
>  int main_pciassignable_remove(int argc, char **argv);
>  int main_pciassignable_list(int argc, char **argv);
> +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
>  int main_restore(int argc, char **argv);
>  int main_migrate_receive(int argc, char **argv);
>  int main_save(int argc, char **argv);
>  int main_migrate(int argc, char **argv);
> +#endif
>  int main_dump_core(int argc, char **argv);
>  int main_pause(int argc, char **argv);
>  int main_unpause(int argc, char **argv);
> @@ -104,7 +106,9 @@ int main_cpupoolnumasplit(int argc, char **argv);
>  int main_getenforce(int argc, char **argv);
>  int main_setenforce(int argc, char **argv);
>  int main_loadpolicy(int argc, char **argv);
> +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
>  int main_remus(int argc, char **argv);
> +#endif
>  int main_devd(int argc, char **argv);
>  
>  void help(const char *command);
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index aff6f90..4fc46eb 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3384,6 +3384,15 @@ static void list_vm(void)
>      libxl_vminfo_list_free(info, nb_vm);
>  }
>  
> +static void core_dump_domain(uint32_t domid, const char *filename)
> +{
> +    int rc;
> +
> +    rc=libxl_domain_core_dump(ctx, domid, filename, NULL);
> +    if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); }
> +}
> +
> +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
>  static void save_domain_core_begin(uint32_t domid,
>                                     const char *override_config_file,
>                                     uint8_t **config_data_r,
> @@ -3775,14 +3784,6 @@ static void migrate_domain(uint32_t domid, const char *rune, int debug,
>      exit(-ERROR_BADFAIL);
>  }
>  
> -static void core_dump_domain(uint32_t domid, const char *filename)
> -{
> -    int rc;
> -
> -    rc=libxl_domain_core_dump(ctx, domid, filename, NULL);
> -    if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); }
> -}
> -
>  static void migrate_receive(int debug, int daemonize, int monitor,
>                              int send_fd, int recv_fd, int remus)
>  {
> @@ -4102,6 +4103,7 @@ int main_migrate(int argc, char **argv)
>      migrate_domain(domid, rune, debug, config_filename);
>      return 0;
>  }
> +#endif
>  
>  int main_dump_core(int argc, char **argv)
>  {
> @@ -7248,6 +7250,7 @@ done:
>      return ret;
>  }
>  
> +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
>  int main_remus(int argc, char **argv)
>  {
>      uint32_t domid;
> @@ -7341,6 +7344,7 @@ int main_remus(int argc, char **argv)
>      close(send_fd);
>      return -ERROR_FAIL;
>  }
> +#endif
>  
>  int main_devd(int argc, char **argv)
>  {
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index ebe0220..e8ab93a 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -137,6 +137,7 @@ struct cmd_spec cmd_table[] = {
>        "                         -autopass\n"
>        "--vncviewer-autopass     (consistency alias for --autopass)"
>      },
> +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
>      { "save",
>        &main_save, 0, 1,
>        "Save a domain state to restore later",
> @@ -158,11 +159,6 @@ struct cmd_spec cmd_table[] = {
>        "                of the domain.\n"
>        "--debug         Print huge (!) amount of debug during the migration process."
>      },
> -    { "dump-core",
> -      &main_dump_core, 0, 1,
> -      "Core dump a domain",
> -      "<Domain> <filename>"
> -    },
>      { "restore",
>        &main_restore, 0, 1,
>        "Restore a domain from a saved state",
> @@ -179,6 +175,12 @@ struct cmd_spec cmd_table[] = {
>        "Restore a domain from a saved state",
>        "- for internal use only",
>      },
> +#endif
> +    { "dump-core",
> +      &main_dump_core, 0, 1,
> +      "Core dump a domain",
> +      "<Domain> <filename>"
> +    },
>      { "cd-insert",
>        &main_cd_insert, 1, 1,
>        "Insert a cdrom into a guest's cd drive",
> @@ -474,6 +476,7 @@ struct cmd_spec cmd_table[] = {
>        "Loads a new policy int the Flask Xen security module",
>        "<policy file>",
>      },
> +#ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
>      { "remus",
>        &main_remus, 0, 1,
>        "Enable Remus HA for domain",
> @@ -486,8 +489,8 @@ struct cmd_spec cmd_table[] = {
>        "                        ssh <host> xl migrate-receive -r [-e]\n"
>        "-e                      Do not wait in the background (on <host>) for the death\n"
>        "                        of the domain."
> -
>      },
> +#endif
>      { "devd",
>        &main_devd, 0, 1,
>        "Daemon that listens for devices and launches backends",

[PATCH OSSTEST] Do not attempt migration tests if the platform doesn't support it

[Ian Campbell]

Doing so blocks the rest of the tests in the job, which may be able to
indepentently complete. So arrange for a ts-migrate-support-check test to run
and gate the remaining migration tests on that.

This relies on the xen patch "xl: suppress suspend/resume functions on
platforms which do not support it" to actually suppress migration support on
arm.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
This needs to go in after the Xen patch. Otherwise this new step will appear to
pass and then start to fail when the Xen patch is applied.
---
 sg-run-job               |  8 +++++++-
 ts-migrate-support-check | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100755 ts-migrate-support-check

diff --git a/sg-run-job b/sg-run-job
index db62365..d894711 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -281,12 +281,18 @@ proc run-job/test-pair {} {
 #    run-ts . remus-failover ts-remus-check         src_host dst_host + debian
 }
 
-proc test-guest {g} {
+proc test-guest-migr {g} {
+    if {[catch { run-ts . = ts-migrate-support-check + host $g }]} return
+
     foreach iteration {{} .2} {
         run-ts . =$iteration ts-guest-saverestore + host $g
         run-ts . =$iteration ts-guest-localmigrate + host $g
     }
     run-ts . = ts-guest-localmigrate x10 + host $g
+}
+
+proc test-guest {g} {
+    test-guest-migr $g
     test-guest-nomigr $g
 }
 
diff --git a/ts-migrate-support-check b/ts-migrate-support-check
new file mode 100755
index 0000000..ffae1b3
--- /dev/null
+++ b/ts-migrate-support-check
@@ -0,0 +1,35 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2014 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+use strict qw(vars);
+use DBI;
+use Osstest;
+use Osstest::TestSupport;
+
+tsreadconfig();
+
+our $ho = selecthost($ARGV[0]);
+
+# all xend/xm platforms support migration
+exit(0) if toolstack()->{Command} eq "xm";
+
+my $help = target_cmd_output_root($ho, toolstack()->{Command}." help");
+
+my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1;
+
+logm("rc=$rc");
+exit($rc);
-- 
1.8.5.2

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it."):
> On Wed, 2014-02-12 at 14:27 +0000, Ian Campbell wrote:
> > ARM does not (currently) support migration, so stop offering tasty looking
> > treats like "xl migrate".

> > Other than the additions of the #define/#ifdef there is a tiny bit of code
> > motion ("dump-core" in the command list and core_dump_domain in the
> > implementations) which serves to put ifdeffable bits next to each other.

I'm not a huge fan of #ifdef but this is tolerable, I think.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

I think this should go into 4.4.  It is essential that we start
advertising lack-of-resume in 4.4 as otherwise in 4.5 we'll have to
invent a new HAVE_HAVE_NO_SUSPEND_RESUME which tells you whether the
lack of HAVE_NO_SUSPEND_RESUME means that you can definitely
suspend/resume.

Ian.

Re: [PATCH OSSTEST] Do not attempt migration tests if the platform doesn't support it

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST] Do not attempt migration tests if the platform doesn't support it"):
> Doing so blocks the rest of the tests in the job, which may be able to
> indepentently complete. So arrange for a ts-migrate-support-check test to run
> and gate the remaining migration tests on that.
> 
> This relies on the xen patch "xl: suppress suspend/resume functions on
> platforms which do not support it" to actually suppress migration support on
> arm.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Olaf Hering]

On Wed, Feb 12, Ian Campbell wrote:

>  #define LIBXL_HAVE_SIGCHLD_SHARING 1
>  
> +/*
> + * LIBXL_HAVE_NO_SUSPEND_RESUME

Think positive?
Make that HAVE_FEATURE and define it on x86.

Olaf

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Ian Campbell]

On Wed, 2014-02-12 at 16:21 +0100, Olaf Hering wrote:
> On Wed, Feb 12, Ian Campbell wrote:
> 
> >  #define LIBXL_HAVE_SIGCHLD_SHARING 1
> >  
> > +/*
> > + * LIBXL_HAVE_NO_SUSPEND_RESUME
> 
> Think positive?
> Make that HAVE_FEATURE and define it on x86.

Could do -- anyone got any strong feeling one way or the other?

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[George Dunlap]

On 02/12/2014 02:32 PM, Ian Campbell wrote:
> Typoed George's address...
>
> On Wed, 2014-02-12 at 14:27 +0000, Ian Campbell wrote:
>> ARM does not (currently) support migration, so stop offering tasty looking
>> treats like "xl migrate".
>>
>> Apart from the UI improvement my intention is to use this in osstest to detect
>> whether to attempt the save/restore/migrate tests.
>>
>> Other than the additions of the #define/#ifdef there is a tiny bit of code
>> motion ("dump-core" in the command list and core_dump_domain in the
>> implementations) which serves to put ifdeffable bits next to each other.
>>
>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> Cc: goerge.dunlap@citrix.com

Looks reasonable.  The risk should be pretty minimal; a compile error at 
most.

Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[George Dunlap]

On 02/12/2014 03:57 PM, Ian Campbell wrote:
> On Wed, 2014-02-12 at 16:21 +0100, Olaf Hering wrote:
>> On Wed, Feb 12, Ian Campbell wrote:
>>
>>>   #define LIBXL_HAVE_SIGCHLD_SHARING 1
>>>   
>>> +/*
>>> + * LIBXL_HAVE_NO_SUSPEND_RESUME
>> Think positive?
>> Make that HAVE_FEATURE and define it on x86.
> Could do -- anyone got any strong feeling one way or the other?

I'm a fan of consistency, so "HAVE_SUSPEND" sounds a little bit better 
to me.  But either way is fine.

  -George

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Ian Campbell]

On Wed, 2014-02-12 at 16:49 +0000, George Dunlap wrote:
> On 02/12/2014 03:57 PM, Ian Campbell wrote:
> > On Wed, 2014-02-12 at 16:21 +0100, Olaf Hering wrote:
> >> On Wed, Feb 12, Ian Campbell wrote:
> >>
> >>>   #define LIBXL_HAVE_SIGCHLD_SHARING 1
> >>>   
> >>> +/*
> >>> + * LIBXL_HAVE_NO_SUSPEND_RESUME
> >> Think positive?
> >> Make that HAVE_FEATURE and define it on x86.
> > Could do -- anyone got any strong feeling one way or the other?
> 
> I'm a fan of consistency, so "HAVE_SUSPEND" sounds a little bit better 
> to me.  But either way is fine.

Right. I think I'll flip it then, it does make more logical sense that
way.

Ian.

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Ian Campbell]

On Wed, 2014-02-12 at 16:54 +0000, Ian Campbell wrote:
> On Wed, 2014-02-12 at 16:49 +0000, George Dunlap wrote:
> > On 02/12/2014 03:57 PM, Ian Campbell wrote:
> > > On Wed, 2014-02-12 at 16:21 +0100, Olaf Hering wrote:
> > >> On Wed, Feb 12, Ian Campbell wrote:
> > >>
> > >>>   #define LIBXL_HAVE_SIGCHLD_SHARING 1
> > >>>   
> > >>> +/*
> > >>> + * LIBXL_HAVE_NO_SUSPEND_RESUME
> > >> Think positive?
> > >> Make that HAVE_FEATURE and define it on x86.
> > > Could do -- anyone got any strong feeling one way or the other?
> > 
> > I'm a fan of consistency, so "HAVE_SUSPEND" sounds a little bit better 
> > to me.  But either way is fine.
> 
> Right. I think I'll flip it then, it does make more logical sense that
> way.

Actually, no I wont.

Anyone who uses the positive version of this will find that migration
support suddenly disappears on x86 when they build against Xen 4.3 or
earlier, unless they are careful to use
        #if defined(LIBL_HAVE_NO_SUSPEND_RESUME) || defined(__i386__) || defined(__x86_64__)
which is worse than a -ve feature flag IMHO.

I will stick with LIBXL_HAVE_NO_SUSPEND_RESUME then.

Ian.

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Ian Campbell]

On Wed, 2014-02-12 at 14:54 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it."):
> > On Wed, 2014-02-12 at 14:27 +0000, Ian Campbell wrote:
> > > ARM does not (currently) support migration, so stop offering tasty looking
> > > treats like "xl migrate".
> 
> > > Other than the additions of the #define/#ifdef there is a tiny bit of code
> > > motion ("dump-core" in the command list and core_dump_domain in the
> > > implementations) which serves to put ifdeffable bits next to each other.
> 
> I'm not a huge fan of #ifdef but this is tolerable, I think.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> I think this should go into 4.4.  It is essential that we start
> advertising lack-of-resume in 4.4 as otherwise in 4.5 we'll have to
> invent a new HAVE_HAVE_NO_SUSPEND_RESUME which tells you whether the
> lack of HAVE_NO_SUSPEND_RESUME means that you can definitely
> suspend/resume.

George release acked it and so I have pushed.

I'll coordinate with you re the osstest push once this passes the xen
gate.

Ian.

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Lai Jiangshan]

On 02/12/2014 10:54 PM, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it."):
>> On Wed, 2014-02-12 at 14:27 +0000, Ian Campbell wrote:
>>> ARM does not (currently) support migration, so stop offering tasty looking
>>> treats like "xl migrate".
> 
>>> Other than the additions of the #define/#ifdef there is a tiny bit of code
>>> motion ("dump-core" in the command list and core_dump_domain in the
>>> implementations) which serves to put ifdeffable bits next to each other.
> 
> I'm not a huge fan of #ifdef but this is tolerable, I think.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Also

Acked-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Thanks,
Lai

> 
> I think this should go into 4.4.  It is essential that we start
> advertising lack-of-resume in 4.4 as otherwise in 4.5 we'll have to
> invent a new HAVE_HAVE_NO_SUSPEND_RESUME which tells you whether the
> lack of HAVE_NO_SUSPEND_RESUME means that you can definitely
> suspend/resume.
> 
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it.

[Ian Campbell]

On Fri, 2014-02-14 at 13:42 +0800, Lai Jiangshan wrote:
> On 02/12/2014 10:54 PM, Ian Jackson wrote:
> > Ian Campbell writes ("Re: [PATCH] xl: suppress suspend/resume functions on platforms which do not support it."):
> >> On Wed, 2014-02-12 at 14:27 +0000, Ian Campbell wrote:
> >>> ARM does not (currently) support migration, so stop offering tasty looking
> >>> treats like "xl migrate".
> > 
> >>> Other than the additions of the #define/#ifdef there is a tiny bit of code
> >>> motion ("dump-core" in the command list and core_dump_domain in the
> >>> implementations) which serves to put ifdeffable bits next to each other.
> > 
> > I'm not a huge fan of #ifdef but this is tolerable, I think.
> > 
> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> Also
> 
> Acked-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Thanks. This patch was already committed.

Ian.