Re: [PATCH] tools/xc: restore logging in xc_save

[Olaf Hering <olaf@aepfle.de>]

On Mon, Feb 04, Ian Campbell wrote:

> On Fri, 2013-02-01 at 18:58 +0000, Olaf Hering wrote:
> > # HG changeset patch
> > # User Olaf Hering <olaf@aepfle.de>
> > # Date 1359745022 -3600
> > # Node ID d76b38b799293ff17fed8eaaac8fbbebced1b72f
> > # Parent  6d1d516ecaade56f796e3216e9931fdcc12282cd
> > tools/xc: restore logging in xc_save
> > 
> > Prior to xen-4.1 the helper xc_save would print some progress during
> > migration. With the new xc_interface_open API no more messages were
> > printed because no logger was configured.
> > 
> > Restore previous behaviour by providing a logger. The progress in
> > xc_domain_save will be disabled because its output lacks a linefeed
> > which makes xend.log look ugly.
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > 
> > diff -r 6d1d516ecaad -r d76b38b79929 tools/xcutils/xc_save.c
> > --- a/tools/xcutils/xc_save.c
> > +++ b/tools/xcutils/xc_save.c
> > @@ -166,17 +166,15 @@ static int switch_qemu_logdirty(int domi
> >  int
> >  main(int argc, char **argv)
> >  {
> > -    unsigned int maxit, max_f;
> > +    unsigned int maxit, max_f, lflags;
> >      int io_fd, ret, port;
> >      struct save_callbacks callbacks;
> > +    xentoollog_level lvl;
> > +    xentoollog_logger *l;
> >  
> >      if (argc != 6)
> >          errx(1, "usage: %s iofd domid maxit maxf flags", argv[0]);
> >  
> > -    si.xch = xc_interface_open(0,0,0);
> > -    if (!si.xch)
> > -        errx(1, "failed to open control interface");
> > -
> >      io_fd = atoi(argv[1]);
> >      si.domid = atoi(argv[2]);
> >      maxit = atoi(argv[3]);
> > @@ -185,6 +183,13 @@ main(int argc, char **argv)
> >  
> >      si.suspend_evtchn = -1;
> >  
> > +    lvl = si.flags & XCFLAGS_DEBUG ? XTL_DEBUG: XTL_DETAIL;
> > +    lflags = XTL_STDIOSTREAM_HIDE_PROGRESS;
> 
> Would it be useful (as an extension) to implement an XTL_STDIOSTREAM
> flag which makes it output something more suitable for logging,
> e.g. ...10%...20%...30%... 
> (or perhaps automatic based on isatty(outputfd)?)

It could be as simple as this tested patch, if its a logfile always
write a new line.

What do you think about this approach?

Olaf

tools/xc: handle tty output differently in stdiostream_progress

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r d9b27c9c40a2 -r 5d7a62db6b3b tools/libxc/xtl_logger_stdio.c
--- a/tools/libxc/xtl_logger_stdio.c
+++ b/tools/libxc/xtl_logger_stdio.c
@@ -86,7 +86,7 @@ static void stdiostream_progress(struct 
                                  const char *doing_what, int percent,
                                  unsigned long done, unsigned long total) {
     xentoollog_logger_stdiostream *lg = (void*)logger_in;
-    int newpel, extra_erase;
+    int newpel, extra_erase, istty;
     xentoollog_level this_level;
 
     if (lg->flags & XTL_STDIOSTREAM_HIDE_PROGRESS)
@@ -105,7 +105,9 @@ static void stdiostream_progress(struct 
     if (this_level < lg->min_level)
         return;
 
-    if (lg->progress_erase_len)
+    istty = isatty(fileno(lg->f)) > 0;
+
+    if (istty && lg->progress_erase_len)
         putc('\r', lg->f);
 
     lg->progress_last_percent = percent;
@@ -113,10 +115,10 @@ static void stdiostream_progress(struct 
     newpel = fprintf(lg->f, "%s%s" "%s: %lu/%lu  %3d%%%s",
                      context?context:"", context?": ":"",
                      doing_what, done, total, percent,
-		     done == total ? "\n" : "");
+		     (done == total) || !istty ? "\n" : "");
 
     extra_erase = lg->progress_erase_len - newpel;
-    if (extra_erase > 0)
+    if (istty && extra_erase > 0)
         fprintf(lg->f, "%*s\r", extra_erase, "");
 
     lg->progress_erase_len = newpel;