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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-02-04 at 10:23 +0000, Olaf Hering wrote:
> On Mon, Feb 04, Ian Campbell wrote:
> 
> > On Fri, 2013-02-01 at 18:58 +0000, Olaf Hering wrote:
> > > +    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)?)
> 
> I was thinking about that, to extend the called progress functions. In
> the case of xend.log it would produce many lines of progress.

How come many lines? I imaged the above as a single line.

> Since the default xend logging is limited to 1MB, the xend.log file
> would be rotated quickly and maybe useful logging output will be lost?
> 
> But for other callers of the progress functions it might be useful to
> have a parsable output so that they dont have to jump through the hoops.
> No idea if any caller makes use of the progress, given that it did not
> work at all without the other patch I sent last Friday.

xl migrate uses AFAIK, which isn't to say it wasn't broken ;-)

> 
> > > +    l = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, lvl, 
> > > lflags);
> > 
> > Might this fail and therefore require error handling?
> 
> If it fails then no logging will happen.

Which I suppose is better than failing the migration altogether.

> 
> > The lvl and lflags variables seem a bit useless to me.
> 
> They are there to keep the lines short

I would have done:
    l = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr,
                                        si.flags & XCFLAGS_DEBUG ? XTL_DEBUG: 
XTL_DETAIL,
                                        XTL_STDIOSTREAM_HIDE_PROGRESS);

But OK.

I think that addresses both of my questions, so
        Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Although I did mean to ask why the code had to move?

(I've been avoiding committing anything while we sort out the staging
backlog, and I seem to have a massive email backlog this morning to
boot, but this is in my queue)

Ian.


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