[PATCH v2 0/6] Fix console synchronisation issues

[PATCH v2 0/6] Fix console synchronisation issues

[Wei Liu]

Version 2 of this series. Now it properly handles read and write errors. I also
dropped two patches to not move the wait bodge from xenconsole to libxl.

Wei Liu (6):
  tools/console: fix help string in client
  tools/console: introduce --start-notify-fd option for console client
  libxl: factor out libxl__console_tty_path
  libxl: libxl_{primary_,}console_exec now take notify_fd argument
  docs: document xenconsole startup protocol
  xl: use xenconsole startup protocol

 docs/misc/console.txt       |  6 ++++
 tools/console/client/main.c | 24 ++++++++++++++-
 tools/libxl/libxl.c         | 72 ++++++++++++++++++++++++++++++---------------
 tools/libxl/libxl.h         | 43 +++++++++++++++++++++++++--
 tools/libxl/xl_cmdimpl.c    | 43 +++++++++++++++++++++++++--
 5 files changed, 157 insertions(+), 31 deletions(-)

-- 
2.1.4


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

[PATCH v2 1/6] tools/console: fix help string in client

[Wei Liu]

There is no short '-t' option.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/console/client/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index f660e10..be39700 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -76,7 +76,7 @@ static void usage(const char *program) {
 	       "\n"
 	       "  -h, --help       display this help and exit\n"
 	       "  -n, --num N      use console number N\n"
-	       "  -t, --type TYPE  console type. must be 'pv' or 'serial'\n"
+	       "  --type TYPE      console type. must be 'pv' or 'serial'\n"
 	       , program);
 }
 
-- 
2.1.4


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

[PATCH v2 2/6] tools/console: introduce --start-notify-fd option for console client

[Wei Liu]

The console client will write 0x00 to that fd before entering console
loop to indicate its readiness.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: properly handle write(2) errors
---
 tools/console/client/main.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index be39700..977779f 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -77,6 +77,7 @@ static void usage(const char *program) {
 	       "  -h, --help       display this help and exit\n"
 	       "  -n, --num N      use console number N\n"
 	       "  --type TYPE      console type. must be 'pv' or 'serial'\n"
+	       "  --start-notify-fd N file descriptor used to notify parent\n"
 	       , program);
 }
 
@@ -327,10 +328,12 @@ int main(int argc, char **argv)
 	int ch;
 	unsigned int num = 0;
 	int opt_ind=0;
+	int start_notify_fd = -1;
 	struct option lopt[] = {
 		{ "type",     1, 0, 't' },
 		{ "num",     1, 0, 'n' },
 		{ "help",    0, 0, 'h' },
+		{ "start-notify-fd", 1, 0, 's' },
 		{ 0 },
 
 	};
@@ -364,6 +367,9 @@ int main(int argc, char **argv)
 				exit(EINVAL);
 			}
 			break;
+		case 's':
+			start_notify_fd = atoi(optarg);
+			break;
 		default:
 			fprintf(stderr, "Invalid argument\n");
 			fprintf(stderr, "Try `%s --help' for more information.\n", 
@@ -462,6 +468,22 @@ int main(int argc, char **argv)
 		init_term(STDIN_FILENO, &stdin_old_attr);
 		atexit(restore_term_stdin); /* if this fails, oh dear */
 	}
+
+	if (start_notify_fd != -1) {
+		/* Write 0x00 to notify parent about client's readiness */
+		static const char msg[] = { 0x00 };
+		int r;
+
+		do {
+			r = write(start_notify_fd, msg, 1);
+		} while ((r == -1 && errno == EINTR) || r == 0);
+
+		if (r == -1)
+			err(errno, "Could not notify parent with fd %d",
+			    start_notify_fd);
+		close(start_notify_fd);
+	}
+
 	console_loop(spty, xs, path, interactive);
 
 	free(path);
-- 
2.1.4


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

Re: [PATCH v2 2/6] tools/console: introduce --start-notify-fd option for console client

[Ian Jackson]

Wei Liu writes ("[PATCH v2 2/6] tools/console: introduce --start-notify-fd option for console client"):
> The console client will write 0x00 to that fd before entering console
> loop to indicate its readiness.

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

> +		do {
> +			r = write(start_notify_fd, msg, 1);
> +		} while ((r == -1 && errno == EINTR) || r == 0);

I don't think write(,,1) can return 0.  If I wrote this code I would
either call abort, or crash, in that case.  But spinning is OK too.

Ian.

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

[PATCH v2 3/6] libxl: factor out libxl__console_tty_path

[Wei Liu]

No other user yet.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl.c | 57 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 7bd3e8c..9a7104c 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1742,6 +1742,40 @@ static void domain_destroy_domid_cb(libxl__egc *egc,
     dis->callback(egc, dis, rc);
 }
 
+static int libxl__console_tty_path(libxl__gc *gc, uint32_t domid, int cons_num,
+                                   libxl_console_type type, char **tty_path)
+{
+    int rc;
+    char *dom_path;
+
+    dom_path = libxl__xs_get_dompath(gc, domid);
+    if (!dom_path) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    switch (type) {
+    case LIBXL_CONSOLE_TYPE_SERIAL:
+        *tty_path = GCSPRINTF("%s/serial/%d/tty", dom_path, cons_num);
+        rc = 0;
+        break;
+    case LIBXL_CONSOLE_TYPE_PV:
+        if (cons_num == 0)
+            *tty_path = GCSPRINTF("%s/console/tty", dom_path);
+        else
+            *tty_path = GCSPRINTF("%s/device/console/%d/tty", dom_path,
+                                  cons_num);
+        rc = 0;
+        break;
+    default:
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+out:
+    return rc;
+}
+
 int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
                        libxl_console_type type)
 {
@@ -1773,30 +1807,13 @@ int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num,
                           libxl_console_type type, char **path)
 {
     GC_INIT(ctx);
-    char *dom_path;
     char *tty_path;
     char *tty;
     int rc;
 
-    dom_path = libxl__xs_get_dompath(gc, domid);
-    if (!dom_path) {
-        rc = ERROR_FAIL;
-        goto out;
-    }
-
-    switch (type) {
-    case LIBXL_CONSOLE_TYPE_SERIAL:
-        tty_path = GCSPRINTF("%s/serial/0/tty", dom_path);
-        break;
-    case LIBXL_CONSOLE_TYPE_PV:
-        if (cons_num == 0)
-            tty_path = GCSPRINTF("%s/console/tty", dom_path);
-        else
-            tty_path = GCSPRINTF("%s/device/console/%d/tty", dom_path,
-                                cons_num);
-        break;
-    default:
-        rc = ERROR_INVAL;
+    rc = libxl__console_tty_path(gc, domid, cons_num, type, &tty_path);
+    if (rc) {
+        LOG(ERROR, "Failed to get tty path for domain %d\n", domid);
         goto out;
     }
 
-- 
2.1.4


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

Re: [PATCH v2 3/6] libxl: factor out libxl__console_tty_path

[Ian Jackson]

Wei Liu writes ("[PATCH v2 3/6] libxl: factor out libxl__console_tty_path"):
> No other user yet.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Of course the proximate reason for this has been dropped from this
series.

However, I think this patch should be retained, because it's a step
towards implementing libxl_console_getfd.

Ian.

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

[PATCH v2 4/6] libxl: libxl_{primary_, }console_exec now take notify_fd argument

[Wei Liu]

The new argument will be passed down to xenconsole process, which then
uses it to notify readiness.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 15 +++++++++++----
 tools/libxl/libxl.h | 43 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9a7104c..da79dc1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1777,12 +1777,13 @@ out:
 }
 
 int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
-                       libxl_console_type type)
+                       libxl_console_type type, int notify_fd)
 {
     GC_INIT(ctx);
     char *p = GCSPRINTF("%s/xenconsole", libxl__private_bindir_path());
     char *domid_s = GCSPRINTF("%d", domid);
     char *cons_num_s = GCSPRINTF("%d", cons_num);
+    char *notify_fd_s;
     char *cons_type_s;
 
     switch (type) {
@@ -1796,7 +1797,13 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
         goto out;
     }
 
-    execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s, (void *)NULL);
+    if (notify_fd != -1) {
+        notify_fd_s = GCSPRINTF("%d", notify_fd);
+        execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
+              "--start-notify-fd", notify_fd_s, (void *)NULL);
+    } else
+        execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
+              (void *)NULL);
 
 out:
     GC_FREE;
@@ -1868,7 +1875,7 @@ out:
     return rc;
 }
 
-int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm)
+int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm, int notify_fd)
 {
     uint32_t domid;
     int cons_num;
@@ -1877,7 +1884,7 @@ int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm)
 
     rc = libxl__primary_console_find(ctx, domid_vm, &domid, &cons_num, &type);
     if ( rc ) return rc;
-    return libxl_console_exec(ctx, domid, cons_num, type);
+    return libxl_console_exec(ctx, domid, cons_num, type, notify_fd);
 }
 
 int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 9b59e6e..030aad5 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -67,6 +67,14 @@
  * the same $(XEN_VERSION) (e.g. throughout a major release).
  */
 
+/* LIBXL_HAVE_CONSOLE_NOTIFY_FD
+ *
+ * If this is defined, libxl_console_exec and
+ * libxl_primary_console_exe take a notify_fd parameter. That
+ * parameter will be used to notify the caller that the console is connected.
+ */
+#define LIBXL_HAVE_CONSOLE_NOTIFY_FD 1
+
 /* LIBXL_HAVE_CONST_COPY_AND_LENGTH_FUNCTIONS
  *
  * If this is defined, the copy functions have constified src parameter and the
@@ -1438,15 +1446,44 @@ int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs);
 #endif
 
 int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
-int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type);
+
+/*
+ * If notify_fd is not -1, xenconsole will write 0x00 to it to nofity
+ * the caller that it has connected to the guest console.
+ */
+int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
+                       libxl_console_type type, int notify_fd);
 /* libxl_primary_console_exec finds the domid and console number
  * corresponding to the primary console of the given vm, then calls
  * libxl_console_exec with the right arguments (domid might be different
  * if the guest is using stubdoms).
  * This function can be called after creating the device model, in
  * case of HVM guests, and before libxl_run_bootloader in case of PV
- * guests using pygrub. */
-int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm);
+ * guests using pygrub.
+ * If notify_fd is not -1, xenconsole will write 0x00 to it to nofity
+ * the caller that it has connected to the guest console.
+ */
+int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm,
+                               int notify_fd);
+
+#if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x040800
+
+static inline int libxl_console_exec_0x040700(libxl_ctx *ctx,
+                                              uint32_t domid, int cons_num,
+                                              libxl_console_type type)
+{
+    return libxl_console_exec(ctx, domid, cons_num, type, -1);
+}
+#define libxl_console_exec libxl_console_exec_0x040700
+
+static inline libxl_primary_console_exec_0x040700(libxl_ctx *ctx,
+                                                  uint32_t domid_vm)
+{
+    return libxl_primary_console_exec(ctx, domid_vm, -1);
+}
+#define libxl_primary_console_exec libxl_primary_console_exec_0x040700
+
+#endif
 
 /* libxl_console_get_tty retrieves the specified domain's console tty path
  * and stores it in path. Caller is responsible for freeing the memory.
-- 
2.1.4


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

Re: [PATCH v2 4/6] libxl: libxl_{primary_, }console_exec now take notify_fd argument

[Ian Jackson]

Wei Liu writes ("[PATCH v2 4/6] libxl: libxl_{primary_,}console_exec now take notify_fd argument"):
> The new argument will be passed down to xenconsole process, which then
> uses it to notify readiness.
...
> -    execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s, (void *)NULL);
> +    if (notify_fd != -1) {
> +        notify_fd_s = GCSPRINTF("%d", notify_fd);
> +        execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
> +              "--start-notify-fd", notify_fd_s, (void *)NULL);
> +    } else
> +        execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
> +              (void *)NULL);

Technically this braces-then-non-braces conforms to the spec in
CODING_STYLE but not to the style elsewhere in libxl.  Could you put
braces around the else clause too ?

Sorry for not noticing this earlier.  We can fix this on checkin if
you like.

Ian.

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

Re: [PATCH v2 4/6] libxl: libxl_{primary_, }console_exec now take notify_fd argument

[Wei Liu]

On Mon, Aug 08, 2016 at 11:15:01AM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v2 4/6] libxl: libxl_{primary_,}console_exec now take notify_fd argument"):
> > The new argument will be passed down to xenconsole process, which then
> > uses it to notify readiness.
> ...
> > -    execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s, (void *)NULL);
> > +    if (notify_fd != -1) {
> > +        notify_fd_s = GCSPRINTF("%d", notify_fd);
> > +        execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
> > +              "--start-notify-fd", notify_fd_s, (void *)NULL);
> > +    } else
> > +        execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
> > +              (void *)NULL);
> 
> Technically this braces-then-non-braces conforms to the spec in
> CODING_STYLE but not to the style elsewhere in libxl.  Could you put
> braces around the else clause too ?
> 
> Sorry for not noticing this earlier.  We can fix this on checkin if
> you like.
> 

Sure.

Wei.

> Ian.

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

Re: [PATCH v2 4/6] libxl: libxl_{primary_, }console_exec now take notify_fd argument

[Wei Liu]

On Fri, Aug 05, 2016 at 06:41:32PM +0100, Wei Liu wrote:
> The new argument will be passed down to xenconsole process, which then
> uses it to notify readiness.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Forgot to carry over the ack from previous version.

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

I think the ack still stands because there is no change in this patch.

Wei.

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

Re: [PATCH v2 4/6] libxl: libxl_{primary_, }console_exec now take notify_fd argument

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v2 4/6] libxl: libxl_{primary_,}console_exec now take notify_fd argument"):
> On Fri, Aug 05, 2016 at 06:41:32PM +0100, Wei Liu wrote:
> > The new argument will be passed down to xenconsole process, which then
> > uses it to notify readiness.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Forgot to carry over the ack from previous version.
> 
>   Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> I think the ack still stands because there is no change in this patch.

Yes.  I think this series is done now ?  The minor comments can be
fixed during checkin.

Ian.

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

[PATCH v2 5/6] docs: document xenconsole startup protocol

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 docs/misc/console.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/docs/misc/console.txt b/docs/misc/console.txt
index ed7b795..16da805 100644
--- a/docs/misc/console.txt
+++ b/docs/misc/console.txt
@@ -124,3 +124,9 @@ can only have one pv console with xenstored as backend (the stubdom
 could provide pv console backends to the hvm guest but then it would
 need another pv console connection for each console backend to export
 the pty to dom0).
+
+The xenconsole program supports a very simple protocol to notify parent about
+its readiness. If xenconsole (the client) is exec'ed and has been given a fd
+(normally the write end of a pipe) via --start-notify-fd option, it will
+write 0x00 to that fd after connecting to the guest but before entering the
+event loop. Parent can read from the read end of the fd to know the status.
-- 
2.1.4


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

[PATCH v2 6/6] xl: use xenconsole startup protocol

[Wei Liu]

If user asks xl to automatically connect to console when creating a
guest, use the new startup protocol before trying to unpause domain so
that we don't lose any console output.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: properly handle read(2) errors
---
 tools/libxl/xl_cmdimpl.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 180fc8d..92002dc 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2728,6 +2728,7 @@ static void autoconnect_console(libxl_ctx *ctx_ignored,
                                 libxl_event *ev, void *priv)
 {
     uint32_t bldomid = ev->domid;
+    int notify_fd = *(int*)priv; /* write end of the notification pipe */
 
     libxl_event_free(ctx, ev);
 
@@ -2740,7 +2741,7 @@ static void autoconnect_console(libxl_ctx *ctx_ignored,
     postfork();
 
     sleep(1);
-    libxl_primary_console_exec(ctx, bldomid);
+    libxl_primary_console_exec(ctx, bldomid, notify_fd);
     /* Do not return. xl continued in child process */
     perror("xl: unable to exec console client");
     _exit(1);
@@ -2810,6 +2811,7 @@ static int create_domain(struct domain_create *dom_info)
     int restore_fd_to_close = -1;
     int send_back_fd = -1;
     const libxl_asyncprogress_how *autoconnect_console_how;
+    int notify_pipe[2] = { -1, -1 };
     struct save_file_header hdr;
     uint32_t domid_soft_reset = INVALID_DOMID;
 
@@ -2997,7 +2999,15 @@ start:
 
     libxl_asyncprogress_how autoconnect_console_how_buf;
     if ( dom_info->console_autoconnect ) {
+        if (libxl_pipe(ctx, notify_pipe)) {
+            fprintf(stderr,
+                    "Failed to create console notification pipe, errno %d\n",
+                    errno);
+            ret = ERROR_FAIL;
+            goto error_out;
+        }
         autoconnect_console_how_buf.callback = autoconnect_console;
+        autoconnect_console_how_buf.for_callback = &notify_pipe[1];
         autoconnect_console_how = &autoconnect_console_how_buf;
     }else{
         autoconnect_console_how = 0;
@@ -3047,6 +3057,33 @@ start:
         restore_fd_to_close = -1;
     }
 
+    if (autoconnect_console_how) {
+        char buf[1];
+        int r;
+
+        /* Try to get notification from xenconsole. Just move on if
+         * error occurs -- it's only minor annoyance if console
+         * doesn't show up.
+         */
+        do {
+            r = read(notify_pipe[0], buf, 1);
+        } while (r == -1 && errno == EINTR);
+
+        if (r == -1)
+            fprintf(stderr,
+                    "Failed to get notification from xenconsole: %s\n",
+                    strerror(errno));
+        else if (r == 0)
+            fprintf(stderr, "Got EOF from xenconsole notification fd\n");
+        else if (r == 1 && buf[0] != 0x00)
+            fprintf(stderr, "Got unexpected response from xenconsole: %x\n",
+                    buf[0]);
+
+        close(notify_pipe[0]);
+        close(notify_pipe[1]);
+        notify_pipe[0] = notify_pipe[1] = -1;
+    }
+
     if (!paused)
         libxl_domain_unpause(ctx, domid);
 
@@ -3754,9 +3791,9 @@ int main_console(int argc, char **argv)
 
     domid = find_domain(argv[optind]);
     if (!type)
-        libxl_primary_console_exec(ctx, domid);
+        libxl_primary_console_exec(ctx, domid, -1);
     else
-        libxl_console_exec(ctx, domid, num, type);
+        libxl_console_exec(ctx, domid, num, type, -1);
     fprintf(stderr, "Unable to attach console\n");
     return EXIT_FAILURE;
 }
-- 
2.1.4


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

Re: [PATCH v2 6/6] xl: use xenconsole startup protocol

[Ian Jackson]

Wei Liu writes ("[PATCH v2 6/6] xl: use xenconsole startup protocol"):
> If user asks xl to automatically connect to console when creating a
> guest, use the new startup protocol before trying to unpause domain so
> that we don't lose any console output.
...
>      if ( dom_info->console_autoconnect ) {
> +        if (libxl_pipe(ctx, notify_pipe)) {
> +            fprintf(stderr,
> +                    "Failed to create console notification pipe, errno %d\n",
> +                    errno);

Is it really necessary to print to stderr, when libxl_pipe has already
logged a message ?  That seems the compelling advantage of
libxl_pipe...

> +        else if (r == 1 && buf[0] != 0x00)
> +            fprintf(stderr, "Got unexpected response from xenconsole: %x\n",
> +                    buf[0]);

I would generally prefer %#x, but up to you.

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

Thanks,
Ian.

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

Re: [PATCH v2 6/6] xl: use xenconsole startup protocol

[Wei Liu]

On Mon, Aug 08, 2016 at 11:17:20AM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v2 6/6] xl: use xenconsole startup protocol"):
> > If user asks xl to automatically connect to console when creating a
> > guest, use the new startup protocol before trying to unpause domain so
> > that we don't lose any console output.
> ...
> >      if ( dom_info->console_autoconnect ) {
> > +        if (libxl_pipe(ctx, notify_pipe)) {
> > +            fprintf(stderr,
> > +                    "Failed to create console notification pipe, errno %d\n",
> > +                    errno);
> 
> Is it really necessary to print to stderr, when libxl_pipe has already
> logged a message ?  That seems the compelling advantage of
> libxl_pipe...
> 

OK. I can delete this fprintf.

> > +        else if (r == 1 && buf[0] != 0x00)
> > +            fprintf(stderr, "Got unexpected response from xenconsole: %x\n",
> > +                    buf[0]);
> 
> I would generally prefer %#x, but up to you.
> 

Done.

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

Thanks.

> Thanks,
> Ian.

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

Re: [PATCH v2 0/6] Fix console synchronisation issues

[Wei Liu]

Adjusted some patches as required and pushed.

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