xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] xl: console error handling improvements
@ 2014-03-19 14:28 Ian Jackson
  2014-03-19 14:28 ` [PATCH 1/5] xl: remove needless error checking from calls to xl_fork Ian Jackson
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Ian Jackson @ 2014-03-19 14:28 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] xl: remove needless error checking from calls to xl_fork
  2014-03-19 14:28 [PATCH 0/5] xl: console error handling improvements Ian Jackson
@ 2014-03-19 14:28 ` Ian Jackson
  2014-03-21 13:47   ` Ian Campbell
  2014-03-19 14:28 ` [PATCH 2/5] xl: Introduce children[].description and xl_report_child_exitstatus Ian Jackson
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2014-03-19 14:28 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/5] xl: Introduce children[].description and xl_report_child_exitstatus
  2014-03-19 14:28 [PATCH 0/5] xl: console error handling improvements Ian Jackson
  2014-03-19 14:28 ` [PATCH 1/5] xl: remove needless error checking from calls to xl_fork Ian Jackson
@ 2014-03-19 14:28 ` Ian Jackson
  2014-03-21 13:47   ` Ian Campbell
  2014-03-19 14:28 ` [PATCH 3/5] xl: Abolish vncviewer_child_report Ian Jackson
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2014-03-19 14:28 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/5] xl: Abolish vncviewer_child_report
  2014-03-19 14:28 [PATCH 0/5] xl: console error handling improvements Ian Jackson
  2014-03-19 14:28 ` [PATCH 1/5] xl: remove needless error checking from calls to xl_fork Ian Jackson
  2014-03-19 14:28 ` [PATCH 2/5] xl: Introduce children[].description and xl_report_child_exitstatus Ian Jackson
@ 2014-03-19 14:28 ` Ian Jackson
  2014-03-21 13:48   ` Ian Campbell
  2014-03-19 14:28 ` [PATCH 4/5] xl: Remove clone-and-hack in do_daemonize Ian Jackson
  2014-03-19 14:28 ` [PATCH 5/5] xl: console connection: Print errno value from exec Ian Jackson
  4 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2014-03-19 14:28 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/5] xl: Remove clone-and-hack in do_daemonize
  2014-03-19 14:28 [PATCH 0/5] xl: console error handling improvements Ian Jackson
                   ` (2 preceding siblings ...)
  2014-03-19 14:28 ` [PATCH 3/5] xl: Abolish vncviewer_child_report Ian Jackson
@ 2014-03-19 14:28 ` Ian Jackson
  2014-03-21 13:49   ` Ian Campbell
  2014-03-19 14:28 ` [PATCH 5/5] xl: console connection: Print errno value from exec Ian Jackson
  4 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2014-03-19 14:28 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/5] xl: console connection: Print errno value from exec
  2014-03-19 14:28 [PATCH 0/5] xl: console error handling improvements Ian Jackson
                   ` (3 preceding siblings ...)
  2014-03-19 14:28 ` [PATCH 4/5] xl: Remove clone-and-hack in do_daemonize Ian Jackson
@ 2014-03-19 14:28 ` Ian Jackson
  2014-03-21 13:52   ` Ian Campbell
  4 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2014-03-19 14:28 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] xl: remove needless error checking from calls to xl_fork
  2014-03-19 14:28 ` [PATCH 1/5] xl: remove needless error checking from calls to xl_fork Ian Jackson
@ 2014-03-21 13:47   ` Ian Campbell
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Campbell @ 2014-03-21 13:47 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/5] xl: Introduce children[].description and xl_report_child_exitstatus
  2014-03-19 14:28 ` [PATCH 2/5] xl: Introduce children[].description and xl_report_child_exitstatus Ian Jackson
@ 2014-03-21 13:47   ` Ian Campbell
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Campbell @ 2014-03-21 13:47 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

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) {

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] xl: Abolish vncviewer_child_report
  2014-03-19 14:28 ` [PATCH 3/5] xl: Abolish vncviewer_child_report Ian Jackson
@ 2014-03-21 13:48   ` Ian Campbell
  2014-03-21 14:33     ` [PATCH 3/5] xl: Abolish vncviewer_child_report [and 1 more messages] Ian Jackson
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Campbell @ 2014-03-21 13:48 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] xl: Remove clone-and-hack in do_daemonize
  2014-03-19 14:28 ` [PATCH 4/5] xl: Remove clone-and-hack in do_daemonize Ian Jackson
@ 2014-03-21 13:49   ` Ian Campbell
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Campbell @ 2014-03-21 13:49 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] xl: console connection: Print errno value from exec
  2014-03-19 14:28 ` [PATCH 5/5] xl: console connection: Print errno value from exec Ian Jackson
@ 2014-03-21 13:52   ` Ian Campbell
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Campbell @ 2014-03-21 13:52 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] xl: Abolish vncviewer_child_report [and 1 more messages]
  2014-03-21 13:48   ` Ian Campbell
@ 2014-03-21 14:33     ` Ian Jackson
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2014-03-21 14:33 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-03-21 14:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-19 14:28 [PATCH 0/5] xl: console error handling improvements Ian Jackson
2014-03-19 14:28 ` [PATCH 1/5] xl: remove needless error checking from calls to xl_fork Ian Jackson
2014-03-21 13:47   ` Ian Campbell
2014-03-19 14:28 ` [PATCH 2/5] xl: Introduce children[].description and xl_report_child_exitstatus Ian Jackson
2014-03-21 13:47   ` Ian Campbell
2014-03-19 14:28 ` [PATCH 3/5] xl: Abolish vncviewer_child_report Ian Jackson
2014-03-21 13:48   ` Ian Campbell
2014-03-21 14:33     ` [PATCH 3/5] xl: Abolish vncviewer_child_report [and 1 more messages] Ian Jackson
2014-03-19 14:28 ` [PATCH 4/5] xl: Remove clone-and-hack in do_daemonize Ian Jackson
2014-03-21 13:49   ` Ian Campbell
2014-03-19 14:28 ` [PATCH 5/5] xl: console connection: Print errno value from exec Ian Jackson
2014-03-21 13:52   ` Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).