Re: [Qemu-devel] [PATCH V3] Support logging xen-guest console

[Stefano Stabellini]

On Fri, 1 Jul 2011, Chun Yan Liu wrote:
> On Thursday, June 30, 2011 07:18:54 PM you wrote:
> > On 06/30/2011 11:39 AM, Chun Yan Liu wrote:
> > > On Thursday, June 30, 2011 03:58:57 PM Alexander Graf wrote:
> > >> On 30.06.2011, at 09:08, Chunyan Liu wrote:
> > >>> Add code to support logging xen-domU console, as what xenconsoled does.
> > >>> To enable logging, set environment variable XENCONSOLED_TRACE=guest and
> > >>> XENCONSOLED_LOGDIR=<specified directory>, log file will be saved in
> > >>> <specified directory>.
> > >> 
> > >> In fact, this whole thing looks as if you're merely trying to
> > >> reimplement "tee" on the xenconsole output. Wouldn't it make more sense
> > >> to do this in
> > >> 
> > >> the char layer? So we could do:
> > >>    -xenconsole tee:stdio,file:/tmp/xen.log
> > >> 
> > >> or similar? That's probably a lot more useful than a random Xen specific
> > >> hack.
> > > 
> > > Thanks, Alex.  It IS something like "tee". But IMO, change in
> > > xen_console.c and change in char layer are just different time points,
> > > do not have essential difference. Change in xen_console.c is trying to
> > > backup output data into log file before sending to char device, change
> > > in char device is trying to dupicate data from char device to log file.
> > > Correct me if I'm wrong.
> > 
> > Sure, the outcome is the same though, no? We get the output data in both
> > a file and the char backend.
> >  
> Char device in qemu is not only used by console. Compared with the benefits 
> that brings, I still doubt if it is proper to adding this functionality to 
> char layer.  
> Stefano, how do you think?
 
I agree with Alexander that Qemu should take a command line argument to
enable xen_console logging, rather than reading an environmental
variable.
However considering that xen_console allocates char drivers dynamically,
the command line syntax proposed by Alexander is not a good fit.
We need a global command line parameter that enables logging for all
xen_console ptys, for example:

-xencon_logs /path/to/logdir

Regarding the particular way to implement logging, I think this patch is
OK. Of course if the char layer had a "tee" option we could use it to
log the pty output, for example prepending "tee:/log/to/file," to the
filename parameter that we are going to pass to qemu_chr_open.