[PATCH 0/6] xl/libxl: some cleanup / debugging aid patches

[PATCH 0/6] xl/libxl: some cleanup / debugging aid patches

[Wei Liu]

A handful of patches I accumulated during freeze when I was debugging various
issues in xl / libxl.

Wei.

Wei Liu (6):
  xl: remus/colo: only initialise ha variable when necessary
  libxl: add emacs block to libxl_linux.c
  libxl: linux hotplug: clean up get_hotplug_env
  libxl: debug output for args and env when invoking hotplug script
  libxl: rename a field in libxl__domain_create_state
  libxl: log file name in failure in libxl__create_qemu_logfile

 tools/libxl/libxl_create.c   | 32 ++++++++++++++++----------------
 tools/libxl/libxl_device.c   | 25 +++++++++++++++++++++++++
 tools/libxl/libxl_dm.c       |  6 ++++--
 tools/libxl/libxl_internal.h |  2 +-
 tools/libxl/libxl_linux.c    | 23 ++++++++++++++++-------
 tools/libxl/xl_cmdimpl.c     |  6 ++++--
 6 files changed, 66 insertions(+), 28 deletions(-)

-- 
2.1.4


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

[PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary

[Wei Liu]

The original code is bogus because the common case is no HA enabled.
Setting ha variable at the beginning is not very useful.

Move ha to the scope where it is used.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/xl_cmdimpl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d8530f0..c33691c 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4752,8 +4752,6 @@ static void migrate_receive(int debug, int daemonize, int monitor,
     char rc_buf;
     char *migration_domname;
     struct domain_create dom_info;
-    const char *ha = checkpointed == LIBXL_CHECKPOINTED_STREAM_COLO ?
-                     "COLO" : "Remus";
 
     signal(SIGPIPE, SIG_IGN);
     /* if we get SIGPIPE we'd rather just have it as an error */
@@ -4788,6 +4786,9 @@ static void migrate_receive(int debug, int daemonize, int monitor,
     switch (checkpointed) {
     case LIBXL_CHECKPOINTED_STREAM_REMUS:
     case LIBXL_CHECKPOINTED_STREAM_COLO:
+    {
+        const char *ha = checkpointed == LIBXL_CHECKPOINTED_STREAM_COLO ?
+                         "COLO" : "Remus";
         /* If we are here, it means that the sender (primary) has crashed.
          * TODO: Split-Brain Check.
          */
@@ -4824,6 +4825,7 @@ static void migrate_receive(int debug, int daemonize, int monitor,
                     ha, common_domname, domid, rc);
 
         exit(rc ? EXIT_FAILURE : EXIT_SUCCESS);
+    }
     default:
         /* do nothing */
         break;
-- 
2.1.4


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

Re: [PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary

[Ian Jackson]

Wei Liu writes ("[PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary"):
> The original code is bogus because the common case is no HA enabled.
> Setting ha variable at the beginning is not very useful.

This seems like a plausible cleanup but I don't understand why you say
the original code is `bogus'.  Is it wrong ?  If so you don't say how.

Ian.

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

Re: [PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary

[Wei Liu]

On Tue, Jun 14, 2016 at 11:18:16AM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary"):
> > The original code is bogus because the common case is no HA enabled.
> > Setting ha variable at the beginning is not very useful.
> 
> This seems like a plausible cleanup but I don't understand why you say
> the original code is `bogus'.  Is it wrong ?  If so you don't say how.
> 

What I meant is "the default state should be no HA enabled", hence if we
don't move the ha variable into a narrower scope it should have been
initialised to NULL first.  It is not a bug though.

Wei.

> Ian.

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

Re: [PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary

[Ian Jackson]

Wei Liu writes ("Re: [PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary"):
> On Tue, Jun 14, 2016 at 11:18:16AM +0100, Ian Jackson wrote:
> > Wei Liu writes ("[PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary"):
> > > The original code is bogus because the common case is no HA enabled.
> > > Setting ha variable at the beginning is not very useful.
> > 
> > This seems like a plausible cleanup but I don't understand why you say
> > the original code is `bogus'.  Is it wrong ?  If so you don't say how.
> 
> What I meant is "the default state should be no HA enabled", hence if we
> don't move the ha variable into a narrower scope it should have been
> initialised to NULL first.  It is not a bug though.

IC.  Perhaps you'd like to clarify the commit message ?
Whether or not you do,

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

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

Re: [PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary

[Wei Liu]

On Tue, Jun 14, 2016 at 03:15:19PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary"):
> > On Tue, Jun 14, 2016 at 11:18:16AM +0100, Ian Jackson wrote:
> > > Wei Liu writes ("[PATCH 1/6] xl: remus/colo: only initialise ha variable when necessary"):
> > > > The original code is bogus because the common case is no HA enabled.
> > > > Setting ha variable at the beginning is not very useful.
> > > 
> > > This seems like a plausible cleanup but I don't understand why you say
> > > the original code is `bogus'.  Is it wrong ?  If so you don't say how.
> > 
> > What I meant is "the default state should be no HA enabled", hence if we
> > don't move the ha variable into a narrower scope it should have been
> > initialised to NULL first.  It is not a bug though.
> 
> IC.  Perhaps you'd like to clarify the commit message ?

Sure, I will incorporate my reply above to the commit message.

> Whether or not you do,
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks.

Wei.

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

[PATCH 2/6] libxl: add emacs block to libxl_linux.c

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_linux.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 624f028..aed8009 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -300,3 +300,11 @@ int libxl__pci_topology_init(libxl__gc *gc,
 
     return err;
 }
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.1.4


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

Re: [PATCH 2/6] libxl: add emacs block to libxl_linux.c

[Ian Jackson]

Wei Liu writes ("[PATCH 2/6] libxl: add emacs block to libxl_linux.c"):
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

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

[PATCH 3/6] libxl: linux hotplug: clean up get_hotplug_env

[Wei Liu]

That get_hotplug_env function is called for both block and nic. Move
some nic specific code out of common code to appropriate place.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_linux.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index aed8009..0033a0e 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -39,12 +39,7 @@ static char **get_hotplug_env(libxl__gc *gc,
     const char *type = libxl__device_kind_to_string(dev->backend_kind);
     char *be_path = libxl__device_backend_path(gc, dev);
     char **env;
-    char *gatewaydev;
     int nr = 0;
-    libxl_nic_type nictype;
-
-    gatewaydev = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/%s", be_path,
-                                                        "gatewaydev"));
 
     const int arraysize = 15;
     GCNEW_ARRAY(env, arraysize);
@@ -56,9 +51,15 @@ static char **get_hotplug_env(libxl__gc *gc,
     env[nr++] = GCSPRINTF("backend/%s/%u/%d", type, dev->domid, dev->devid);
     env[nr++] = "XENBUS_BASE_PATH";
     env[nr++] = "backend";
-    env[nr++] = "netdev";
-    env[nr++] = gatewaydev ? : "";
     if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
+        libxl_nic_type nictype;
+        char *gatewaydev;
+
+        gatewaydev = libxl__xs_read(gc, XBT_NULL,
+                                    GCSPRINTF("%s/%s", be_path, "gatewaydev"));
+        env[nr++] = "netdev";
+        env[nr++] = gatewaydev ? : "";
+
         if (libxl__nic_type(gc, dev, &nictype)) {
             LOG(ERROR, "unable to get nictype");
             return NULL;
-- 
2.1.4


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

Re: [PATCH 3/6] libxl: linux hotplug: clean up get_hotplug_env

[Ian Jackson]

Wei Liu writes ("[PATCH 3/6] libxl: linux hotplug: clean up get_hotplug_env"):
> That get_hotplug_env function is called for both block and nic. Move
> some nic specific code out of common code to appropriate place.

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

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

[PATCH 4/6] libxl: debug output for args and env when invoking hotplug script

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_device.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 4717027..b922a94 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1167,6 +1167,31 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
     }
 
     LOG(DEBUG, "calling hotplug script: %s %s", args[0], args[1]);
+    LOG(DEBUG, "extra args:");
+    {
+        const char *arg;
+        unsigned int x = 2;
+
+        arg = args[x];
+        while (arg) {
+            LOG(DEBUG, "\t%s", arg);
+            x++;
+            arg = args[x];
+        }
+    }
+    LOG(DEBUG, "env:");
+    {
+        const char *k, *v;
+        unsigned int x = 0;
+
+        k = env[x];
+        while (k) {
+            v = env[x+1];
+            LOG(DEBUG, "\t%s: %s", k, v);
+            x += 2;
+            k = env[x];
+        }
+    }
 
     nullfd = open("/dev/null", O_RDONLY);
     if (nullfd < 0) {
-- 
2.1.4


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

Re: [PATCH 4/6] libxl: debug output for args and env when invoking hotplug script

[Ian Jackson]

Wei Liu writes ("[PATCH 4/6] libxl: debug output for args and env when invoking hotplug script"):
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
...
> +        const char *arg;
> +        unsigned int x = 2;
> +
> +        arg = args[x];
> +        while (arg) {
> +            LOG(DEBUG, "\t%s", arg);
> +            x++;
> +            arg = args[x];
> +        }

What a strange way to write

   for (x=2; (arg = args[x]); x++) {

or

   for (x=2; (arg = args[x++]); ) {

or

   x = 2;
   while ((arg = args[x++])) {

If you really insist on not doing assignment in the conditional (which
IMO is a very usual C idiom) then you should avoid the repeated code
with

   x = 2;
   for (;;) {
      arg = args[x++];
      if (!arg) break;

or some such.

> +        const char *k, *v;
> +        unsigned int x = 0;
> +
> +        k = env[x];
> +        while (k) {
> +            v = env[x+1];
> +            LOG(DEBUG, "\t%s: %s", k, v);
> +            x += 2;
> +            k = env[x];
> +        }

How about one of

   for (x=0; (k = env[x]); x += 2) {
       v = env[x+1];

   for (x=0; (k = env[x]) && (v = env[x+1]); x += 2) {

   for (x=0; (k = env[x++]) && (v = env[x++]); ) {

   x = 0;
   while ((k = env[x++])) {
       v = env[x++];
       assert(v);

?

Ian.

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

Re: [PATCH 4/6] libxl: debug output for args and env when invoking hotplug script

[Wei Liu]

On Tue, Jun 14, 2016 at 11:26:56AM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH 4/6] libxl: debug output for args and env when invoking hotplug script"):
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ...
> > +        const char *arg;
> > +        unsigned int x = 2;
> > +
> > +        arg = args[x];
> > +        while (arg) {
> > +            LOG(DEBUG, "\t%s", arg);
> > +            x++;
> > +            arg = args[x];
> > +        }

I will use

> 
> What a strange way to write
> 
>    for (x=2; (arg = args[x]); x++) {
> 

This style.

> or
> 
>    for (x=2; (arg = args[x++]); ) {
> 
> or
> 
>    x = 2;
>    while ((arg = args[x++])) {
> 
> If you really insist on not doing assignment in the conditional (which
> IMO is a very usual C idiom) then you should avoid the repeated code
> with
> 
>    x = 2;
>    for (;;) {
>       arg = args[x++];
>       if (!arg) break;
> 
> or some such.
> 
> > +        const char *k, *v;
> > +        unsigned int x = 0;
> > +
> > +        k = env[x];
> > +        while (k) {
> > +            v = env[x+1];
> > +            LOG(DEBUG, "\t%s: %s", k, v);
> > +            x += 2;
> > +            k = env[x];
> > +        }
> 
> How about one of
> 
>    for (x=0; (k = env[x]); x += 2) {
>        v = env[x+1];
> 

And this style.

---8<---
From 49714976c5fde3d08baa6f34295b3f7db6a81444 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Fri, 15 Apr 2016 12:56:03 +0100
Subject: [PATCH] libxl: debug output for args and env when invoking hotplug
 script

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: write the loops differently.
---
 tools/libxl/libxl_device.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 4717027..b3213be 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1167,6 +1167,24 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
     }
 
     LOG(DEBUG, "calling hotplug script: %s %s", args[0], args[1]);
+    LOG(DEBUG, "extra args:");
+    {
+        const char *arg;
+        unsigned int x;
+
+        for (x = 2; (arg = args[x]); x++)
+            LOG(DEBUG, "\t%s", arg);
+    }
+    LOG(DEBUG, "env:");
+    {
+        const char *k, *v;
+        unsigned int x;
+
+        for (x = 0; (k = env[x]); x += 2) {
+            v = env[x+1];
+            LOG(DEBUG, "\t%s: %s", k, v);
+        }
+    }
 
     nullfd = open("/dev/null", O_RDONLY);
     if (nullfd < 0) {
-- 
2.1.4


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

Re: [PATCH 4/6] libxl: debug output for args and env when invoking hotplug script

[Wei Liu]

> From 49714976c5fde3d08baa6f34295b3f7db6a81444 Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Fri, 15 Apr 2016 12:56:03 +0100
> Subject: [PATCH] libxl: debug output for args and env when invoking hotplug
>  script
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Ian, Ping?

> ---
> v2: write the loops differently.
> ---
>  tools/libxl/libxl_device.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 4717027..b3213be 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -1167,6 +1167,24 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
>      }
>  
>      LOG(DEBUG, "calling hotplug script: %s %s", args[0], args[1]);
> +    LOG(DEBUG, "extra args:");
> +    {
> +        const char *arg;
> +        unsigned int x;
> +
> +        for (x = 2; (arg = args[x]); x++)
> +            LOG(DEBUG, "\t%s", arg);
> +    }
> +    LOG(DEBUG, "env:");
> +    {
> +        const char *k, *v;
> +        unsigned int x;
> +
> +        for (x = 0; (k = env[x]); x += 2) {
> +            v = env[x+1];
> +            LOG(DEBUG, "\t%s: %s", k, v);
> +        }
> +    }
>  
>      nullfd = open("/dev/null", O_RDONLY);
>      if (nullfd < 0) {
> -- 
> 2.1.4
> 

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

Re: [PATCH 4/6] libxl: debug output for args and env when invoking hotplug script

[Ian Jackson]

Wei Liu writes ("Re: [PATCH 4/6] libxl: debug output for args and env when invoking hotplug script"):
> > From 49714976c5fde3d08baa6f34295b3f7db6a81444 Mon Sep 17 00:00:00 2001
> > From: Wei Liu <wei.liu2@citrix.com>
> > Date: Fri, 15 Apr 2016 12:56:03 +0100
> > Subject: [PATCH] libxl: debug output for args and env when invoking hotplug
> >  script
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Ian, Ping?

Sorry, yes.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 4/6] libxl: debug output for args and env when invoking hotplug script

[Wei Liu]

On Mon, Jul 04, 2016 at 06:06:27PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH 4/6] libxl: debug output for args and env when invoking hotplug script"):
> > > From 49714976c5fde3d08baa6f34295b3f7db6a81444 Mon Sep 17 00:00:00 2001
> > > From: Wei Liu <wei.liu2@citrix.com>
> > > Date: Fri, 15 Apr 2016 12:56:03 +0100
> > > Subject: [PATCH] libxl: debug output for args and env when invoking hotplug
> > >  script
> > > 
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > Ian, Ping?
> 
> Sorry, yes.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Queued. Thanks.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 5/6] libxl: rename a field in libxl__domain_create_state

[Wei Liu]

The libxl__stub_dm_spawn_state field in libxl__domain_create_state was
named dmss. That was inconsistent with how things were named (usually
acronym) and there was already libxl__dm_spawn_state named dmss in other
places.

Change dmss to sdss and fix up all sites that reference this field.  No
functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_create.c   | 32 ++++++++++++++++----------------
 tools/libxl/libxl_internal.h |  2 +-
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index ca5b167..1b99472 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -903,7 +903,7 @@ static void initiate_domain_create(libxl__egc *egc,
     }
 
     dcs->guest_domid = domid;
-    dcs->dmss.dm.guest_domid = 0; /* means we haven't spawned */
+    dcs->sdss.dm.guest_domid = 0; /* means we haven't spawned */
 
     ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
     if (ret) {
@@ -1050,11 +1050,11 @@ static void domcreate_bootloader_done(libxl__egc *egc,
     /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm.
      * Fill in any field required by either, including both relevant
      * callbacks (_spawn_stub_dm will overwrite our trespass if needed). */
-    dcs->dmss.dm.spawn.ao = ao;
-    dcs->dmss.dm.guest_config = dcs->guest_config;
-    dcs->dmss.dm.build_state = &dcs->build_state;
-    dcs->dmss.dm.callback = domcreate_devmodel_started;
-    dcs->dmss.callback = domcreate_devmodel_started;
+    dcs->sdss.dm.spawn.ao = ao;
+    dcs->sdss.dm.guest_config = dcs->guest_config;
+    dcs->sdss.dm.build_state = &dcs->build_state;
+    dcs->sdss.dm.callback = domcreate_devmodel_started;
+    dcs->sdss.callback = domcreate_devmodel_started;
 
     if (restore_fd < 0 && dcs->domid_soft_reset == INVALID_DOMID) {
         rc = libxl__domain_build(gc, d_config, domid, state);
@@ -1344,7 +1344,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
 
         if (d_config->b_info.device_model_version ==
             LIBXL_DEVICE_MODEL_VERSION_NONE) {
-            domcreate_devmodel_started(egc, &dcs->dmss.dm, 0);
+            domcreate_devmodel_started(egc, &dcs->sdss.dm, 0);
             return;
         }
 
@@ -1352,11 +1352,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         libxl__device_vkb_add(gc, domid, &vkb);
         libxl_device_vkb_dispose(&vkb);
 
-        dcs->dmss.dm.guest_domid = domid;
+        dcs->sdss.dm.guest_domid = domid;
         if (libxl_defbool_val(d_config->b_info.device_model_stubdomain))
-            libxl__spawn_stub_dm(egc, &dcs->dmss);
+            libxl__spawn_stub_dm(egc, &dcs->sdss);
         else
-            libxl__spawn_local_dm(egc, &dcs->dmss.dm);
+            libxl__spawn_local_dm(egc, &dcs->sdss.dm);
 
         /*
          * Handle the domain's (and the related stubdomain's) access to
@@ -1387,12 +1387,12 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         if (ret < 0)
             goto error_out;
         if (ret) {
-            dcs->dmss.dm.guest_domid = domid;
-            libxl__spawn_local_dm(egc, &dcs->dmss.dm);
+            dcs->sdss.dm.guest_domid = domid;
+            libxl__spawn_local_dm(egc, &dcs->sdss.dm);
             return;
         } else {
-            assert(!dcs->dmss.dm.guest_domid);
-            domcreate_devmodel_started(egc, &dcs->dmss.dm, 0);
+            assert(!dcs->sdss.dm.guest_domid);
+            domcreate_devmodel_started(egc, &dcs->sdss.dm, 0);
             return;
         }
     }
@@ -1411,7 +1411,7 @@ static void domcreate_devmodel_started(libxl__egc *egc,
                                        libxl__dm_spawn_state *dmss,
                                        int ret)
 {
-    libxl__domain_create_state *dcs = CONTAINER_OF(dmss, *dcs, dmss.dm);
+    libxl__domain_create_state *dcs = CONTAINER_OF(dmss, *dcs, sdss.dm);
     STATE_AO_GC(dmss->spawn.ao);
     int domid = dcs->guest_domid;
 
@@ -1423,7 +1423,7 @@ static void domcreate_devmodel_started(libxl__egc *egc,
         goto error_out;
     }
 
-    if (dcs->dmss.dm.guest_domid) {
+    if (dcs->sdss.dm.guest_domid) {
         if (d_config->b_info.device_model_version
             == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
             libxl__qmp_initializations(gc, domid, d_config);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index ae16c25..6fd7366 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3570,7 +3570,7 @@ struct libxl__domain_create_state {
     libxl__colo_restore_state crs;
     libxl__checkpoint_devices_state cds;
     libxl__bootloader_state bl;
-    libxl__stub_dm_spawn_state dmss;
+    libxl__stub_dm_spawn_state sdss;
         /* If we're not doing stubdom, we use only dmss.dm,
          * for the non-stubdom device model. */
     libxl__stream_read_state srs;
-- 
2.1.4


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

Re: [PATCH 5/6] libxl: rename a field in libxl__domain_create_state

[Ian Jackson]

Wei Liu writes ("[PATCH 5/6] libxl: rename a field in libxl__domain_create_state"):
> The libxl__stub_dm_spawn_state field in libxl__domain_create_state was
> named dmss. That was inconsistent with how things were named (usually
> acronym) and there was already libxl__dm_spawn_state named dmss in other
> places.
> 
> Change dmss to sdss and fix up all sites that reference this field.  No
> functional change.

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

(without really reading the patch....)

Ian.

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

[PATCH 6/6] libxl: log file name in failure in libxl__create_qemu_logfile

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_dm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 155a653..69d2242 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -52,13 +52,15 @@ static int libxl__create_qemu_logfile(libxl__gc *gc, char *name)
     if (rc) return rc;
 
     logfile_w = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
-    free(logfile);
 
     if (logfile_w < 0) {
-        LOGE(ERROR, "unable to open Qemu logfile");
+        LOGE(ERROR, "unable to open Qemu logfile: %s", logfile);
+        free(logfile);
         return ERROR_FAIL;
     }
 
+    free(logfile);
+
     return logfile_w;
 }
 
-- 
2.1.4


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

Re: [PATCH 6/6] libxl: log file name in failure in libxl__create_qemu_logfile

[Ian Jackson]

Wei Liu writes ("[PATCH 6/6] libxl: log file name in failure in libxl__create_qemu_logfile"):
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

It would be nice to fix the error handling pattern to `goto out'
style, but this patch is an improvement.

Ian.

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

Re: [PATCH 0/6] xl/libxl: some cleanup / debugging aid patches

[Wei Liu]

On Mon, Jun 06, 2016 at 11:52:06AM +0100, Wei Liu wrote:
> A handful of patches I accumulated during freeze when I was debugging various
> issues in xl / libxl.
> 
> Wei.
> 
> Wei Liu (6):
>   xl: remus/colo: only initialise ha variable when necessary
>   libxl: add emacs block to libxl_linux.c
>   libxl: linux hotplug: clean up get_hotplug_env
>   libxl: debug output for args and env when invoking hotplug script
>   libxl: rename a field in libxl__domain_create_state
>   libxl: log file name in failure in libxl__create_qemu_logfile
> 

I've queued up all the acked patches (2, 3, 5 and 6) for committing.

Wei.

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