[PATCH 0/5] xl: console error handling improvements

[PATCH 0/5] xl: console error handling improvements

[Ian Jackson]

xl has some rather clone-and-hackish console client child handling.

 1/5 xl: remove needless error checking from calls to xl_fork
 2/5 xl: Introduce children[].description and
 3/5 xl: Abolish vncviewer_child_report
 4/5 xl: Remove clone-and-hack in do_daemonize
 5/5 xl: console connection: Print errno value from exec

[PATCH 1/5] xl: remove needless error checking from calls to xl_fork

[Ian Jackson]

xl_fork cannot fail - it exits instead.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 8990020..e19b1cf 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -230,10 +230,7 @@ static void autoconnect_vncviewer(uint32_t domid, int autopass)
     vncviewer_child_report();
 
     pid_t pid = xl_fork(child_vncviewer);
-    if (pid < 0) {
-        perror("unable to fork vncviewer");
-        return;
-    } else if (pid > 0)
+    if (pid)
         return;
 
     postfork();
@@ -2009,10 +2006,7 @@ static void autoconnect_console(libxl_ctx *ctx_ignored,
     console_child_report();
 
     pid_t pid = xl_fork(child_console);
-    if (pid < 0) {
-        perror("unable to fork xenconsole");
-        return;
-    } else if (pid > 0)
+    if (pid)
         return;
 
     postfork();
-- 
1.7.10.4

Re: [PATCH 1/5] xl: remove needless error checking from calls to xl_fork

[Ian Campbell]

On Wed, 2014-03-19 at 14:28 +0000, Ian Jackson wrote:
> xl_fork cannot fail - it exits instead.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

[PATCH 2/5] xl: Introduce children[].description and xl_report_child_exitstatus

[Ian Jackson]

We record the descriptive string for the child in the children[]
array, and use it when reporting the exit status.

The only functional change is that the message reported for the
migration child is changed from "migration target process" to
"migration transport process".

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
---
 tools/libxl/xl.c         |   10 +++++++++-
 tools/libxl/xl.h         |    8 +++++++-
 tools/libxl/xl_cmdimpl.c |   21 ++++++++++-----------
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 1d157fe..527b4c5 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -189,12 +189,13 @@ void postfork(void)
     xl_ctx_alloc();
 }
 
-pid_t xl_fork(xlchildnum child) {
+pid_t xl_fork(xlchildnum child, const char *description) {
     xlchild *ch = &children[child];
     int i;
 
     assert(!ch->pid);
     ch->reaped = 0;
+    ch->description = description;
 
     ch->pid = fork();
     if (ch->pid == -1) {
@@ -238,6 +239,13 @@ int xl_child_pid(xlchildnum child)
     return ch->pid;
 }
 
+void xl_report_child_exitstatus(xentoollog_level level,
+                                xlchildnum child, pid_t pid, int status)
+{
+    libxl_report_child_exitstatus(ctx, level, children[child].description,
+                                  pid, status);
+}
+
 static int xl_reaped_callback(pid_t got, int status, void *user)
 {
     int i;
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 280d39c..40a42b3 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -130,6 +130,7 @@ typedef struct {
     pid_t pid; /* 0: not in use */
     int reaped; /* valid iff pid!=0 */
     int status; /* valid iff reaped */
+    const char *description; /* valid iff pid!=0 */
 } xlchild;
 
 typedef enum {
@@ -139,7 +140,8 @@ typedef enum {
 
 extern xlchild children[child_max];
 
-pid_t xl_fork(xlchildnum); /* like fork, but prints and dies if it fails */
+pid_t xl_fork(xlchildnum, const char *description);
+    /* like fork, but prints and dies if it fails */
 void postfork(void); /* needed only if we aren't going to exec right away */
 
 /* Handles EINTR.  Clears out the xlchild so it can be reused. */
@@ -147,6 +149,10 @@ pid_t xl_waitpid(xlchildnum, int *status, int flags);
 
 int xl_child_pid(xlchildnum); /* returns 0 if child struct is not in use */
 
+void xl_report_child_exitstatus(xentoollog_level level,
+                                xlchildnum child, pid_t pid, int status);
+    /* like libxl_report_child_exitstatus, but uses children[].description */
+
 /* global options */
 extern int autoballoon;
 extern int run_hotplug_scripts;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e19b1cf..0c2eca3 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -220,8 +220,8 @@ static void vncviewer_child_report(void)
         if (got < 0)
             perror("xl: warning, failed to waitpid for vncviewer child");
         else if (status)
-            libxl_report_child_exitstatus(ctx, XTL_ERROR, "vncviewer child",
-                                          xl_child_pid(child_vncviewer), status);
+            xl_report_child_exitstatus(XTL_ERROR, child_vncviewer,
+                                       got, status);
     }
 }
 
@@ -229,7 +229,7 @@ static void autoconnect_vncviewer(uint32_t domid, int autopass)
 {
     vncviewer_child_report();
 
-    pid_t pid = xl_fork(child_vncviewer);
+    pid_t pid = xl_fork(child_vncviewer, "vncviewer child");
     if (pid)
         return;
 
@@ -435,7 +435,7 @@ static int do_daemonize(char *name)
     int nullfd, ret = 0;
     int status = 0;
 
-    child1 = xl_fork(child_waitdaemon);
+    child1 = xl_fork(child_waitdaemon, "domain monitoring daemon");
     if (child1) {
         got_child = xl_waitpid(child_waitdaemon, &status, 0);
         if (got_child != child1) {
@@ -1991,8 +1991,8 @@ static void console_child_report(void)
         if (got < 0)
             perror("xl: warning, failed to waitpid for console child");
         else if (status)
-            libxl_report_child_exitstatus(ctx, XTL_ERROR, "console child",
-                                          xl_child_pid(child_console), status);
+            xl_report_child_exitstatus(XTL_ERROR, child_console,
+                                       got, status);
     }
 }
 
@@ -2005,7 +2005,7 @@ static void autoconnect_console(libxl_ctx *ctx_ignored,
 
     console_child_report();
 
-    pid_t pid = xl_fork(child_console);
+    pid_t pid = xl_fork(child_console, "console child");
     if (pid)
         return;
 
@@ -3525,7 +3525,7 @@ static pid_t create_migration_child(const char *rune, int *send_fd,
     MUST( libxl_pipe(ctx, sendpipe) );
     MUST( libxl_pipe(ctx, recvpipe) );
 
-    child = xl_fork(child_migration);
+    child = xl_fork(child_migration, "migration transport process");
 
     if (!child) {
         dup2(sendpipe[0], 0);
@@ -3586,9 +3586,8 @@ static void migration_child_report(int recv_fd) {
 
         if (child == migration_child) {
             if (status)
-                libxl_report_child_exitstatus(ctx, XTL_INFO,
-                                              "migration target process",
-                                              migration_child, status);
+                xl_report_child_exitstatus(XTL_INFO, child_migration,
+                                           migration_child, status);
             break;
         }
         if (child == -1) {
-- 
1.7.10.4

Re: [PATCH 2/5] xl: Introduce children[].description and xl_report_child_exitstatus

[Ian Campbell]

On Wed, 2014-03-19 at 14:28 +0000, Ian Jackson wrote:
> We record the descriptive string for the child in the children[]
> array, and use it when reporting the exit status.
> 
> The only functional change is that the message reported for the
> migration child is changed from "migration target process" to
> "migration transport process".
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

> ---
>  tools/libxl/xl.c         |   10 +++++++++-
>  tools/libxl/xl.h         |    8 +++++++-
>  tools/libxl/xl_cmdimpl.c |   21 ++++++++++-----------
>  3 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
> index 1d157fe..527b4c5 100644
> --- a/tools/libxl/xl.c
> +++ b/tools/libxl/xl.c
> @@ -189,12 +189,13 @@ void postfork(void)
>      xl_ctx_alloc();
>  }
>  
> -pid_t xl_fork(xlchildnum child) {
> +pid_t xl_fork(xlchildnum child, const char *description) {
>      xlchild *ch = &children[child];
>      int i;
>  
>      assert(!ch->pid);
>      ch->reaped = 0;
> +    ch->description = description;
>  
>      ch->pid = fork();
>      if (ch->pid == -1) {
> @@ -238,6 +239,13 @@ int xl_child_pid(xlchildnum child)
>      return ch->pid;
>  }
>  
> +void xl_report_child_exitstatus(xentoollog_level level,
> +                                xlchildnum child, pid_t pid, int status)
> +{
> +    libxl_report_child_exitstatus(ctx, level, children[child].description,
> +                                  pid, status);
> +}
> +
>  static int xl_reaped_callback(pid_t got, int status, void *user)
>  {
>      int i;
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index 280d39c..40a42b3 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -130,6 +130,7 @@ typedef struct {
>      pid_t pid; /* 0: not in use */
>      int reaped; /* valid iff pid!=0 */
>      int status; /* valid iff reaped */
> +    const char *description; /* valid iff pid!=0 */
>  } xlchild;
>  
>  typedef enum {
> @@ -139,7 +140,8 @@ typedef enum {
>  
>  extern xlchild children[child_max];
>  
> -pid_t xl_fork(xlchildnum); /* like fork, but prints and dies if it fails */
> +pid_t xl_fork(xlchildnum, const char *description);
> +    /* like fork, but prints and dies if it fails */
>  void postfork(void); /* needed only if we aren't going to exec right away */
>  
>  /* Handles EINTR.  Clears out the xlchild so it can be reused. */
> @@ -147,6 +149,10 @@ pid_t xl_waitpid(xlchildnum, int *status, int flags);
>  
>  int xl_child_pid(xlchildnum); /* returns 0 if child struct is not in use */
>  
> +void xl_report_child_exitstatus(xentoollog_level level,
> +                                xlchildnum child, pid_t pid, int status);
> +    /* like libxl_report_child_exitstatus, but uses children[].description */
> +
>  /* global options */
>  extern int autoballoon;
>  extern int run_hotplug_scripts;
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index e19b1cf..0c2eca3 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -220,8 +220,8 @@ static void vncviewer_child_report(void)
>          if (got < 0)
>              perror("xl: warning, failed to waitpid for vncviewer child");
>          else if (status)
> -            libxl_report_child_exitstatus(ctx, XTL_ERROR, "vncviewer child",
> -                                          xl_child_pid(child_vncviewer), status);
> +            xl_report_child_exitstatus(XTL_ERROR, child_vncviewer,
> +                                       got, status);
>      }
>  }
>  
> @@ -229,7 +229,7 @@ static void autoconnect_vncviewer(uint32_t domid, int autopass)
>  {
>      vncviewer_child_report();
>  
> -    pid_t pid = xl_fork(child_vncviewer);
> +    pid_t pid = xl_fork(child_vncviewer, "vncviewer child");
>      if (pid)
>          return;
>  
> @@ -435,7 +435,7 @@ static int do_daemonize(char *name)
>      int nullfd, ret = 0;
>      int status = 0;
>  
> -    child1 = xl_fork(child_waitdaemon);
> +    child1 = xl_fork(child_waitdaemon, "domain monitoring daemon");
>      if (child1) {
>          got_child = xl_waitpid(child_waitdaemon, &status, 0);
>          if (got_child != child1) {
> @@ -1991,8 +1991,8 @@ static void console_child_report(void)
>          if (got < 0)
>              perror("xl: warning, failed to waitpid for console child");
>          else if (status)
> -            libxl_report_child_exitstatus(ctx, XTL_ERROR, "console child",
> -                                          xl_child_pid(child_console), status);
> +            xl_report_child_exitstatus(XTL_ERROR, child_console,
> +                                       got, status);
>      }
>  }
>  
> @@ -2005,7 +2005,7 @@ static void autoconnect_console(libxl_ctx *ctx_ignored,
>  
>      console_child_report();
>  
> -    pid_t pid = xl_fork(child_console);
> +    pid_t pid = xl_fork(child_console, "console child");
>      if (pid)
>          return;
>  
> @@ -3525,7 +3525,7 @@ static pid_t create_migration_child(const char *rune, int *send_fd,
>      MUST( libxl_pipe(ctx, sendpipe) );
>      MUST( libxl_pipe(ctx, recvpipe) );
>  
> -    child = xl_fork(child_migration);
> +    child = xl_fork(child_migration, "migration transport process");
>  
>      if (!child) {
>          dup2(sendpipe[0], 0);
> @@ -3586,9 +3586,8 @@ static void migration_child_report(int recv_fd) {
>  
>          if (child == migration_child) {
>              if (status)
> -                libxl_report_child_exitstatus(ctx, XTL_INFO,
> -                                              "migration target process",
> -                                              migration_child, status);
> +                xl_report_child_exitstatus(XTL_INFO, child_migration,
> +                                           migration_child, status);
>              break;
>          }
>          if (child == -1) {

[PATCH 3/5] xl: Abolish vncviewer_child_report

[Ian Jackson]

vncviewer_child_report was very similar to console_child_report.

We can abolish vncviewer_child_report by adding an xlchildnum
parameter to console_child_report.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |   45 ++++++++++++++++-----------------------------
 1 file changed, 16 insertions(+), 29 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 0c2eca3..d61d301 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -205,29 +205,29 @@ static uint32_t find_domain(const char *p)
     return domid;
 }
 
-static int vncviewer(uint32_t domid, int autopass)
+static void console_child_report(xlchildnum child)
 {
-    libxl_vncviewer_exec(ctx, domid, autopass);
-    fprintf(stderr, "Unable to execute vncviewer\n");
-    return 1;
-}
-
-static void vncviewer_child_report(void)
-{
-    if (xl_child_pid(child_vncviewer)) {
+    if (xl_child_pid(child)) {
         int status;
-        pid_t got = xl_waitpid(child_vncviewer, &status, 0);
+        pid_t got = xl_waitpid(child, &status, 0);
         if (got < 0)
-            perror("xl: warning, failed to waitpid for vncviewer child");
+            fprintf(stderr, "xl: warning, failed to waitpid for %s: %s\n",
+                    children[child].description, strerror(errno));
         else if (status)
-            xl_report_child_exitstatus(XTL_ERROR, child_vncviewer,
-                                       got, status);
+            xl_report_child_exitstatus(XTL_ERROR, child, got, status);
     }
 }
 
+static int vncviewer(uint32_t domid, int autopass)
+{
+    libxl_vncviewer_exec(ctx, domid, autopass);
+    fprintf(stderr, "Unable to execute vncviewer\n");
+    return 1;
+}
+
 static void autoconnect_vncviewer(uint32_t domid, int autopass)
 {
-    vncviewer_child_report();
+   console_child_report(child_vncviewer);
 
     pid_t pid = xl_fork(child_vncviewer, "vncviewer child");
     if (pid)
@@ -1983,19 +1983,6 @@ static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
     return ERROR_NOMEM;
 }
 
-static void console_child_report(void)
-{
-    if (xl_child_pid(child_console)) {
-        int status;
-        pid_t got = xl_waitpid(child_console, &status, 0);
-        if (got < 0)
-            perror("xl: warning, failed to waitpid for console child");
-        else if (status)
-            xl_report_child_exitstatus(XTL_ERROR, child_console,
-                                       got, status);
-    }
-}
-
 static void autoconnect_console(libxl_ctx *ctx_ignored,
                                 libxl_event *ev, void *priv)
 {
@@ -2003,7 +1990,7 @@ static void autoconnect_console(libxl_ctx *ctx_ignored,
 
     libxl_event_free(ctx, ev);
 
-    console_child_report();
+    console_child_report(child_console);
 
     pid_t pid = xl_fork(child_console, "console child");
     if (pid)
@@ -2461,7 +2448,7 @@ out:
 
     free(config_data);
 
-    console_child_report();
+    console_child_report(child_console);
 
     if (deathw)
         libxl_evdisable_domain_death(ctx, deathw);
-- 
1.7.10.4

Re: [PATCH 3/5] xl: Abolish vncviewer_child_report

[Ian Campbell]

On Wed, 2014-03-19 at 14:28 +0000, Ian Jackson wrote:
> vncviewer_child_report was very similar to console_child_report.
> 
> We can abolish vncviewer_child_report by adding an xlchildnum
> parameter to console_child_report.

I suppose VNC is a sort of console, so it's not horribly misnamed now.

> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

Re: [PATCH 3/5] xl: Abolish vncviewer_child_report [and 1 more messages]

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH 3/5] xl: Abolish vncviewer_child_report"):
> On Wed, 2014-03-19 at 14:28 +0000, Ian Jackson wrote:
> > We can abolish vncviewer_child_report by adding an xlchildnum
> > parameter to console_child_report.
> 
> I suppose VNC is a sort of console, so it's not horribly misnamed now.

Indeed, that was my thought.

Ian Campbell writes ("Re: [PATCH 5/5] xl: console connection: Print errno value from exec"):
> libxl_primary_console_exec can also fail if it can't find the primary
> console, which I'm supposing won't set a (useful) errno.
> 
> I think that is probably somewhat tolerable (or at least it is better to
> sometimes have a useful errno than never):

Yes. :-/.

> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks.  I'll add your acks and push the series.

Ian.

[PATCH 4/5] xl: Remove clone-and-hack in do_daemonize

[Ian Jackson]

do_daemonize had open-coded handling of the results from xl_waitpid.

Instead, break out the meat of console_child_report into a new
function child_report (which returns an error code), and use it.

No functional change other than a change to the message printed if
forking the daemonic child fails.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
---
 tools/libxl/xl.h         |    4 ++++
 tools/libxl/xl_cmdimpl.c |   48 +++++++++++++++++++++-------------------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 40a42b3..10a2e66 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -153,6 +153,10 @@ void xl_report_child_exitstatus(xentoollog_level level,
                                 xlchildnum child, pid_t pid, int status);
     /* like libxl_report_child_exitstatus, but uses children[].description */
 
+int child_report(xlchildnum child);
+    /* waits and expects child to exit status 0.
+     * otherwise, logs and returns ERROR_FAIL */
+
 /* global options */
 extern int autoballoon;
 extern int run_hotplug_scripts;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d61d301..27b6f40 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -205,19 +205,28 @@ static uint32_t find_domain(const char *p)
     return domid;
 }
 
-static void console_child_report(xlchildnum child)
+int child_report(xlchildnum child)
 {
-    if (xl_child_pid(child)) {
-        int status;
-        pid_t got = xl_waitpid(child, &status, 0);
-        if (got < 0)
-            fprintf(stderr, "xl: warning, failed to waitpid for %s: %s\n",
-                    children[child].description, strerror(errno));
-        else if (status)
-            xl_report_child_exitstatus(XTL_ERROR, child, got, status);
+    int status;
+    pid_t got = xl_waitpid(child, &status, 0);
+    if (got < 0) {
+        fprintf(stderr, "xl: warning, failed to waitpid for %s: %s\n",
+                children[child].description, strerror(errno));
+        return ERROR_FAIL;
+    } else if (status) {
+        xl_report_child_exitstatus(XTL_ERROR, child, got, status);
+        return ERROR_FAIL;
+    } else {
+        return 0;
     }
 }
 
+static void console_child_report(xlchildnum child)
+{
+    if (xl_child_pid(child))
+        child_report(child);
+}
+
 static int vncviewer(uint32_t domid, int autopass)
 {
     libxl_vncviewer_exec(ctx, domid, autopass);
@@ -431,26 +440,13 @@ out:
 static int do_daemonize(char *name)
 {
     char *fullname;
-    pid_t child1, got_child;
+    pid_t child1;
     int nullfd, ret = 0;
-    int status = 0;
 
-    child1 = xl_fork(child_waitdaemon, "domain monitoring daemon");
+    child1 = xl_fork(child_waitdaemon, "domain monitoring daemonizing child");
     if (child1) {
-        got_child = xl_waitpid(child_waitdaemon, &status, 0);
-        if (got_child != child1) {
-            assert(got_child == -1);
-            LOG("failed to wait for daemonizing child: %s", strerror(errno));
-            ret = ERROR_FAIL;
-            goto out;
-        }
-
-        if (status) {
-            libxl_report_child_exitstatus(ctx, XTL_ERROR,
-                       "daemonizing child", child1, status);
-            ret = ERROR_FAIL;
-            goto out;
-        }
+        ret = child_report(child_waitdaemon);
+        if (ret) goto out;
         ret = 1;
         goto out;
     }
-- 
1.7.10.4

Re: [PATCH 4/5] xl: Remove clone-and-hack in do_daemonize

[Ian Campbell]

On Wed, 2014-03-19 at 14:28 +0000, Ian Jackson wrote:
> do_daemonize had open-coded handling of the results from xl_waitpid.
> 
> Instead, break out the meat of console_child_report into a new
> function child_report (which returns an error code), and use it.
> 
> No functional change other than a change to the message printed if
> forking the daemonic child fails.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

[PATCH 5/5] xl: console connection: Print errno value from exec

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 27b6f40..8389468 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1997,7 +1997,7 @@ static void autoconnect_console(libxl_ctx *ctx_ignored,
     sleep(1);
     libxl_primary_console_exec(ctx, bldomid);
     /* Do not return. xl continued in child process */
-    fprintf(stderr, "Unable to attach console\n");
+    perror("xl: unable to exec console client");
     _exit(1);
 }
 
-- 
1.7.10.4

Re: [PATCH 5/5] xl: console connection: Print errno value from exec

[Ian Campbell]

On Wed, 2014-03-19 at 14:28 +0000, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

libxl_primary_console_exec can also fail if it can't find the primary
console, which I'm supposing won't set a (useful) errno.

I think that is probably somewhat tolerable (or at least it is better to
sometimes have a useful errno than never):

Acked-by: Ian Campbell <ian.campbell@citrix.com>