[PATCH 0/5] tools: Migration cosmetic improvements

[PATCH 0/5] tools: Migration cosmetic improvements

[Ian Jackson]

 1/5 xentoollog: provide XTL_STDIOSTREAM_PROGRESS_USE_CR
 2/5 xl: New -t option ("tty") to force \r-based messages
 3/5 xl: migration: pass -t to xl migrate-receive
 4/5 xl: Pass -v options on to migration receiver
 5/5 libxl: multidev: Clarify internal API doc comment

[PATCH 1/5] xentoollog: provide XTL_STDIOSTREAM_PROGRESS_USE_CR

[Ian Jackson]

Provide flags
  XTL_STDIOSTREAM_PROGRESS_USE_CR
  XTL_STDIOSTREAM_PROGRESS_NO_CR
to allow the caller to force, or disable, the use of \r-based
overwriting of progress messages.

In the implementation, rename the variable "tty" to "progress_use_cr".

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Olaf Hering <olaf@aepfle.de>
CC: Ian Campbell <Ian.Campbell@citrix.com>
---
 tools/libxc/xentoollog.h       |    8 +++++---
 tools/libxc/xtl_logger_stdio.c |   18 +++++++++++++++---
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/xentoollog.h b/tools/libxc/xentoollog.h
index 6d36dd9..85d3da9 100644
--- a/tools/libxc/xentoollog.h
+++ b/tools/libxc/xentoollog.h
@@ -65,9 +65,11 @@ struct xentoollog_logger {
 
 /*---------- facilities for consuming log messages ----------*/
 
-#define XTL_STDIOSTREAM_SHOW_PID      01u
-#define XTL_STDIOSTREAM_SHOW_DATE     02u
-#define XTL_STDIOSTREAM_HIDE_PROGRESS 04u
+#define XTL_STDIOSTREAM_SHOW_PID            001u
+#define XTL_STDIOSTREAM_SHOW_DATE           002u
+#define XTL_STDIOSTREAM_HIDE_PROGRESS       004u
+#define XTL_STDIOSTREAM_PROGRESS_USE_CR     010u /* default is to */
+#define XTL_STDIOSTREAM_PROGRESS_NO_CR      020u /* use \r to ttys */
 
 typedef struct xentoollog_logger_stdiostream  xentoollog_logger_stdiostream;
 
diff --git a/tools/libxc/xtl_logger_stdio.c b/tools/libxc/xtl_logger_stdio.c
index aa5501f..47ee257 100644
--- a/tools/libxc/xtl_logger_stdio.c
+++ b/tools/libxc/xtl_logger_stdio.c
@@ -28,6 +28,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <stdbool.h>
 
 struct xentoollog_logger_stdiostream {
     xentoollog_logger vtable;
@@ -35,7 +36,7 @@ struct xentoollog_logger_stdiostream {
     xentoollog_level min_level;
     unsigned flags;
     int progress_erase_len, progress_last_percent;
-    int tty;
+    bool progress_use_cr;
 };
 
 static void progress_erase(xentoollog_logger_stdiostream *lg) {
@@ -119,7 +120,7 @@ static void stdiostream_progress(struct xentoollog_logger *logger_in,
 
     lg->progress_last_percent = percent;
 
-    if (!lg->tty) {
+    if (!lg->progress_use_cr) {
         stdiostream_message(logger_in, this_level, context,
                             "%s: %lu/%lu  %3d%%",
                             doing_what, done, total, percent);
@@ -167,7 +168,18 @@ xentoollog_logger_stdiostream *xtl_createlogger_stdiostream
     newlogger.f = f;
     newlogger.min_level = min_level;
     newlogger.flags = flags;
-    newlogger.tty = isatty(fileno(newlogger.f)) > 0;
+
+    switch (flags & (XTL_STDIOSTREAM_PROGRESS_USE_CR |
+                     XTL_STDIOSTREAM_PROGRESS_NO_CR)) {
+    case XTL_STDIOSTREAM_PROGRESS_USE_CR: newlogger.progress_use_cr = 1; break;
+    case XTL_STDIOSTREAM_PROGRESS_NO_CR:  newlogger.progress_use_cr = 0; break;
+    case 0:
+        newlogger.progress_use_cr = isatty(fileno(newlogger.f)) > 0;
+        break;
+    default:
+        errno = EINVAL;
+        return 0;
+    }
 
     if (newlogger.flags & XTL_STDIOSTREAM_SHOW_DATE) tzset();
 
-- 
1.7.10.4

[PATCH 2/5] xl: New -t option ("tty") to force \r-based messages

[Ian Jackson]

Provide a new xl global option -t which passes
XTL_STDIOSTREAM_PROGRESS_USE_CR to xentoollog.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Olaf Hering <olaf@aepfle.de>
CC: Ian Campbell <Ian.Campbell@citrix.com>
---
 docs/man/xl.pod.1 |    6 ++++++
 tools/libxl/xl.c  |    9 +++++++--
 tools/libxl/xl.h  |    1 +
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index e7b9de2..f7ceaa8 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -81,6 +81,12 @@ Force execution: xl will refuse to run some commands if it detects that xend is
 also running, this option will force the execution of those commands, even
 though it is unsafe.
 
+=item B<-t>
+
+Always use carriage-return-based overwriting for printing progress
+messages without scrolling the screen.  Without -t, this is done only
+if stderr is a tty.
+
 =back
 
 =head1 DOMAIN SUBCOMMANDS
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 657610b..7fdc155 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -48,6 +48,7 @@ char *default_gatewaydev = NULL;
 char *default_vifbackend = NULL;
 enum output_format default_output_format = OUTPUT_FORMAT_JSON;
 int claim_mode = 1;
+bool progress_use_cr = 0;
 
 static xentoollog_level minmsglevel = XTL_PROGRESS;
 
@@ -292,7 +293,7 @@ int main(int argc, char **argv)
     int config_len = 0;
     const char *locks[] = XEND_LOCK;
 
-    while ((opt = getopt(argc, argv, "+vfN")) >= 0) {
+    while ((opt = getopt(argc, argv, "+vftN")) >= 0) {
         switch (opt) {
         case 'v':
             if (minmsglevel > 0) minmsglevel--;
@@ -303,6 +304,9 @@ int main(int argc, char **argv)
         case 'f':
             force_execution = 1;
             break;
+        case 't':
+            progress_use_cr = 1;
+            break;
         default:
             fprintf(stderr, "unknown global option\n");
             exit(2);
@@ -317,7 +321,8 @@ int main(int argc, char **argv)
     }
     opterr = 0;
 
-    logger = xtl_createlogger_stdiostream(stderr, minmsglevel,  0);
+    logger = xtl_createlogger_stdiostream(stderr, minmsglevel,
+        (progress_use_cr ? XTL_STDIOSTREAM_PROGRESS_USE_CR : 0));
     if (!logger) exit(1);
 
     atexit(xl_ctx_free);
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index f188708..1a71234 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -152,6 +152,7 @@ extern int autoballoon;
 extern int run_hotplug_scripts;
 extern int dryrun_only;
 extern int claim_mode;
+extern bool progress_use_cr;
 extern char *lockfile;
 extern char *default_vifscript;
 extern char *default_bridge;
-- 
1.7.10.4

[PATCH 3/5] xl: migration: pass -t to xl migrate-receive

[Ian Jackson]

If we ourselves are using cr-based overwriting for logging to stderr,
pass -t to the migration receiver so that it knows to do the same
(since its stderr is normally the pipe from sshd).

This requires, of course, that the receiver support that option.  This
is OK from a compatibility point of view because we support migration
to newer, but not necessarily to older, versions.  (If unsupported
backwards migration is still desired the use of -s "" allows the
remote invocation rune to be overridden by a command of one's choice.)

This fixes a regression introduced in 2f80ac9c0e8f, where migration
messages from the receiver would not use of the overwriting protocol.

CC: Olaf Hering <olaf@aepfle.de>
CC: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 tools/libxl/xl_cmdimpl.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 6b1ebfa..d52b933 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4110,11 +4110,14 @@ int main_migrate(int argc, char **argv)
     domid = find_domain(argv[optind]);
     host = argv[optind + 1];
 
+    bool pass_tty_arg = progress_use_cr || (isatty(2) > 0);
+
     if (!ssh_command[0]) {
         rune= host;
     } else {
-        if (asprintf(&rune, "exec %s %s xl migrate-receive%s%s",
+        if (asprintf(&rune, "exec %s %s xl%s migrate-receive%s%s",
                      ssh_command, host,
+                     pass_tty_arg ? " -t" : "",
                      daemonize ? "" : " -e",
                      debug ? " -d" : "") < 0)
             return 1;
-- 
1.7.10.4

[PATCH 4/5] xl: Pass -v options on to migration receiver

[Ian Jackson]

Compute a -v option to pass to the migration receiver.

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

diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 7fdc155..22bbcc6 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -50,7 +50,7 @@ enum output_format default_output_format = OUTPUT_FORMAT_JSON;
 int claim_mode = 1;
 bool progress_use_cr = 0;
 
-static xentoollog_level minmsglevel = XTL_PROGRESS;
+xentoollog_level minmsglevel = XTL_PROGRESS;
 
 /* Get autoballoon option based on presence of dom0_mem Xen command
    line option. */
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 1a71234..280d39c 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -153,6 +153,8 @@ extern int run_hotplug_scripts;
 extern int dryrun_only;
 extern int claim_mode;
 extern bool progress_use_cr;
+extern xentoollog_level minmsglevel;
+#define minmsglevel_default XTL_PROGRESS
 extern char *lockfile;
 extern char *default_vifscript;
 extern char *default_bridge;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d52b933..8990020 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4115,9 +4115,21 @@ int main_migrate(int argc, char **argv)
     if (!ssh_command[0]) {
         rune= host;
     } else {
-        if (asprintf(&rune, "exec %s %s xl%s migrate-receive%s%s",
+        char verbose_buf[minmsglevel_default+3];
+        int verbose_len;
+        verbose_buf[0] = ' ';
+        verbose_buf[1] = '-';
+        memset(verbose_buf+2, 'v', minmsglevel_default);
+        verbose_buf[sizeof(verbose_buf)-1] = 0;
+        if (minmsglevel == minmsglevel_default) {
+            verbose_len = 0;
+        } else {
+            verbose_len = (minmsglevel_default - minmsglevel) + 2;
+        }
+        if (asprintf(&rune, "exec %s %s xl%s%.*s migrate-receive%s%s",
                      ssh_command, host,
                      pass_tty_arg ? " -t" : "",
+                     verbose_len, verbose_buf,
                      daemonize ? "" : " -e",
                      debug ? " -d" : "") < 0)
             return 1;
-- 
1.7.10.4

[PATCH 5/5] libxl: multidev: Clarify internal API doc comment

[Ian Jackson]

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

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 69d2ba8..869c8c6 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2077,7 +2077,8 @@ struct libxl__ao_device {
  *    multidev->callback = ...
  * Then zero or more times
  *    libxl__multidev_prepare
- *    libal__initiate_device_{remove/addition}.
+ *    libxl__initiate_device_{remove/addition}
+ *       (or some other thing which will eventually call aodev->callback)
  * Finally, once
  *    libxl__multidev_prepared
  * which will result (perhaps reentrantly) in one call to callback().
-- 
1.7.10.4

Re: [PATCH 0/5] tools: Migration cosmetic improvements

[Olaf Hering]

On Mon, Mar 17, Ian Jackson wrote:

>  1/5 xentoollog: provide XTL_STDIOSTREAM_PROGRESS_USE_CR
>  2/5 xl: New -t option ("tty") to force \r-based messages
>  3/5 xl: migration: pass -t to xl migrate-receive
>  4/5 xl: Pass -v options on to migration receiver
>  5/5 libxl: multidev: Clarify internal API doc comment

This looks good to me, but I did not do any runtime testing.
Thanks!

Olaf

Re: [PATCH 1/5] xentoollog: provide XTL_STDIOSTREAM_PROGRESS_USE_CR

[Ian Campbell]

On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> Provide flags
>   XTL_STDIOSTREAM_PROGRESS_USE_CR
>   XTL_STDIOSTREAM_PROGRESS_NO_CR
> to allow the caller to force, or disable, the use of \r-based
> overwriting of progress messages.
> 
> In the implementation, rename the variable "tty" to "progress_use_cr".
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Olaf Hering <olaf@aepfle.de>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> ---
>  tools/libxc/xentoollog.h       |    8 +++++---
>  tools/libxc/xtl_logger_stdio.c |   18 +++++++++++++++---
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxc/xentoollog.h b/tools/libxc/xentoollog.h
> index 6d36dd9..85d3da9 100644
> --- a/tools/libxc/xentoollog.h
> +++ b/tools/libxc/xentoollog.h
> @@ -65,9 +65,11 @@ struct xentoollog_logger {
>  
>  /*---------- facilities for consuming log messages ----------*/
>  
> -#define XTL_STDIOSTREAM_SHOW_PID      01u
> -#define XTL_STDIOSTREAM_SHOW_DATE     02u
> -#define XTL_STDIOSTREAM_HIDE_PROGRESS 04u
> +#define XTL_STDIOSTREAM_SHOW_PID            001u
> +#define XTL_STDIOSTREAM_SHOW_DATE           002u
> +#define XTL_STDIOSTREAM_HIDE_PROGRESS       004u
> +#define XTL_STDIOSTREAM_PROGRESS_USE_CR     010u /* default is to */
> +#define XTL_STDIOSTREAM_PROGRESS_NO_CR      020u /* use \r to ttys */

Not often you see an octal constant!

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

Re: [PATCH 2/5] xl: New -t option ("tty") to force \r-based messages

[Ian Campbell]

On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> Provide a new xl global option -t which passes
> XTL_STDIOSTREAM_PROGRESS_USE_CR to xentoollog.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Olaf Hering <olaf@aepfle.de>

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

OIO what is the motivation -- when do you want this behaviour while
stdrrr is not a tty?

Ian.

Re: [PATCH 3/5] xl: migration: pass -t to xl migrate-receive

[Ian Campbell]

On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> If we ourselves are using cr-based overwriting for logging to stderr,
> pass -t to the migration receiver so that it knows to do the same
> (since its stderr is normally the pipe from sshd).
> 
> This requires, of course, that the receiver support that option.  This
> is OK from a compatibility point of view because we support migration
> to newer, but not necessarily to older, versions.  (If unsupported
> backwards migration is still desired the use of -s "" allows the
> remote invocation rune to be overridden by a command of one's choice.)
> 
> This fixes a regression introduced in 2f80ac9c0e8f, where migration
> messages from the receiver would not use of the overwriting protocol.

Ah, here's the motivation for the series!

> 
> CC: Olaf Hering <olaf@aepfle.de>

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

Is there any desire to have an opposite to -t?

> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  tools/libxl/xl_cmdimpl.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 6b1ebfa..d52b933 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -4110,11 +4110,14 @@ int main_migrate(int argc, char **argv)
>      domid = find_domain(argv[optind]);
>      host = argv[optind + 1];
>  
> +    bool pass_tty_arg = progress_use_cr || (isatty(2) > 0);
> +
>      if (!ssh_command[0]) {
>          rune= host;
>      } else {
> -        if (asprintf(&rune, "exec %s %s xl migrate-receive%s%s",
> +        if (asprintf(&rune, "exec %s %s xl%s migrate-receive%s%s",
>                       ssh_command, host,
> +                     pass_tty_arg ? " -t" : "",
>                       daemonize ? "" : " -e",
>                       debug ? " -d" : "") < 0)
>              return 1;

Re: [PATCH 4/5] xl: Pass -v options on to migration receiver

[Ian Campbell]

On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> Compute a -v option to pass to the migration receiver.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> ---
>  tools/libxl/xl.c         |    2 +-
>  tools/libxl/xl.h         |    2 ++
>  tools/libxl/xl_cmdimpl.c |   14 +++++++++++++-
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
> index 7fdc155..22bbcc6 100644
> --- a/tools/libxl/xl.c
> +++ b/tools/libxl/xl.c
> @@ -50,7 +50,7 @@ enum output_format default_output_format = OUTPUT_FORMAT_JSON;
>  int claim_mode = 1;
>  bool progress_use_cr = 0;
>  
> -static xentoollog_level minmsglevel = XTL_PROGRESS;
> +xentoollog_level minmsglevel = XTL_PROGRESS;
>  
>  /* Get autoballoon option based on presence of dom0_mem Xen command
>     line option. */
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index 1a71234..280d39c 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -153,6 +153,8 @@ extern int run_hotplug_scripts;
>  extern int dryrun_only;
>  extern int claim_mode;
>  extern bool progress_use_cr;
> +extern xentoollog_level minmsglevel;
> +#define minmsglevel_default XTL_PROGRESS

Was your intention to use this to initialise minmsglevel?

>  extern char *lockfile;
>  extern char *default_vifscript;
>  extern char *default_bridge;
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index d52b933..8990020 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -4115,9 +4115,21 @@ int main_migrate(int argc, char **argv)
>      if (!ssh_command[0]) {
>          rune= host;
>      } else {
> -        if (asprintf(&rune, "exec %s %s xl%s migrate-receive%s%s",
> +        char verbose_buf[minmsglevel_default+3];
> +        int verbose_len;
> +        verbose_buf[0] = ' ';
> +        verbose_buf[1] = '-';
> +        memset(verbose_buf+2, 'v', minmsglevel_default);
> +        verbose_buf[sizeof(verbose_buf)-1] = 0;
> +        if (minmsglevel == minmsglevel_default) {
> +            verbose_len = 0;
> +        } else {
> +            verbose_len = (minmsglevel_default - minmsglevel) + 2;
> +        }
> +        if (asprintf(&rune, "exec %s %s xl%s%.*s migrate-receive%s%s",
>                       ssh_command, host,
>                       pass_tty_arg ? " -t" : "",
> +                     verbose_len, verbose_buf,

Is verbose_len needed here vs verbose_buf[0] ? vebose_buf : ""

Oh, I see the verbose_len = 0 above. Makes sense. (I initially misread
the preceding code as doing stuff with minmsglevel - minmsglevel_default
to produce exactly the required string)

Ian.

Re: [PATCH 5/5] libxl: multidev: Clarify internal API doc comment

[Ian Campbell]

On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

Re: [PATCH 3/5] xl: migration: pass -t to xl migrate-receive

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH 3/5] xl: migration: pass -t to xl migrate-receive"):
> On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> > This fixes a regression introduced in 2f80ac9c0e8f, where migration
> > messages from the receiver would not use of the overwriting protocol.
> 
> Ah, here's the motivation for the series!

Yes.

> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Is there any desire to have an opposite to -t?

I doubt it.  But there's space for it if we want it later.

Ian.

Re: [PATCH 4/5] xl: Pass -v options on to migration receiver

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH 4/5] xl: Pass -v options on to migration receiver"):
> On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> > Compute a -v option to pass to the migration receiver.

> > +extern xentoollog_level minmsglevel;
> > +#define minmsglevel_default XTL_PROGRESS
> 
> Was your intention to use this to initialise minmsglevel?

Oops, yes.  Fixed version below.

> Is verbose_len needed here vs verbose_buf[0] ? vebose_buf : ""
> 
> Oh, I see the verbose_len = 0 above. Makes sense. (I initially misread
> the preceding code as doing stuff with minmsglevel - minmsglevel_default
> to produce exactly the required string)

Yes.

>From c7eada4addca817bd17038918e54f04b4e38bb8f Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Tue, 7 Jan 2014 18:40:05 +0000
Subject: [PATCH v2 4/9] xl: Pass -v options on to migration receiver

Compute a -v option to pass to the migration receiver.

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

---
v2: Use minmsglevel_default to initialise minmsglevel.
---
 tools/libxl/xl.c         |    2 +-
 tools/libxl/xl.h         |    2 ++
 tools/libxl/xl_cmdimpl.c |   14 +++++++++++++-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 7fdc155..1d157fe 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -50,7 +50,7 @@ enum output_format default_output_format = OUTPUT_FORMAT_JSON;
 int claim_mode = 1;
 bool progress_use_cr = 0;
 
-static xentoollog_level minmsglevel = XTL_PROGRESS;
+xentoollog_level minmsglevel = minmsglevel_default;
 
 /* Get autoballoon option based on presence of dom0_mem Xen command
    line option. */
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 1a71234..280d39c 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -153,6 +153,8 @@ extern int run_hotplug_scripts;
 extern int dryrun_only;
 extern int claim_mode;
 extern bool progress_use_cr;
+extern xentoollog_level minmsglevel;
+#define minmsglevel_default XTL_PROGRESS
 extern char *lockfile;
 extern char *default_vifscript;
 extern char *default_bridge;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d52b933..8990020 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4115,9 +4115,21 @@ int main_migrate(int argc, char **argv)
     if (!ssh_command[0]) {
         rune= host;
     } else {
-        if (asprintf(&rune, "exec %s %s xl%s migrate-receive%s%s",
+        char verbose_buf[minmsglevel_default+3];
+        int verbose_len;
+        verbose_buf[0] = ' ';
+        verbose_buf[1] = '-';
+        memset(verbose_buf+2, 'v', minmsglevel_default);
+        verbose_buf[sizeof(verbose_buf)-1] = 0;
+        if (minmsglevel == minmsglevel_default) {
+            verbose_len = 0;
+        } else {
+            verbose_len = (minmsglevel_default - minmsglevel) + 2;
+        }
+        if (asprintf(&rune, "exec %s %s xl%s%.*s migrate-receive%s%s",
                      ssh_command, host,
                      pass_tty_arg ? " -t" : "",
+                     verbose_len, verbose_buf,
                      daemonize ? "" : " -e",
                      debug ? " -d" : "") < 0)
             return 1;
-- 
1.7.10.4

Re: [PATCH 4/5] xl: Pass -v options on to migration receiver

[Ian Campbell]

On Tue, 2014-03-18 at 18:07 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH 4/5] xl: Pass -v options on to migration receiver"):
> > On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> > > Compute a -v option to pass to the migration receiver.
> 
> > > +extern xentoollog_level minmsglevel;
> > > +#define minmsglevel_default XTL_PROGRESS
> > 
> > Was your intention to use this to initialise minmsglevel?
> 
> Oops, yes.  Fixed version below.
> 
> > Is verbose_len needed here vs verbose_buf[0] ? vebose_buf : ""
> > 
> > Oh, I see the verbose_len = 0 above. Makes sense. (I initially misread
> > the preceding code as doing stuff with minmsglevel - minmsglevel_default
> > to produce exactly the required string)
> 
> Yes.
> 
> From c7eada4addca817bd17038918e54f04b4e38bb8f Mon Sep 17 00:00:00 2001
> From: Ian Jackson <ian.jackson@eu.citrix.com>
> Date: Tue, 7 Jan 2014 18:40:05 +0000
> Subject: [PATCH v2 4/9] xl: Pass -v options on to migration receiver
> 
> Compute a -v option to pass to the migration receiver.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

Re: [PATCH 3/5] xl: migration: pass -t to xl migrate-receive

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH 3/5] xl: migration: pass -t to xl migrate-receive"):
> On Mon, 2014-03-17 at 16:03 +0000, Ian Jackson wrote:
> > This fixes a regression introduced in 2f80ac9c0e8f, where migration
> > messages from the receiver would not use of the overwriting protocol.
...
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks, I have pushed this series.

Ian.