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>.
So far, qemu doesn't rely on environment variables and I don't see a compelling reason to introduce us relying on them here. Keep in mind that with Xenner, this code is also used without Xen as the host, so it's just like any other device driver in qemu.
Please change it to command line options. Also, please run checkpatch.pl on your patch before sending it :).
Alex

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.
Alex

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.
Thanks,
Chunyan

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.
Alex

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?
Thanks,
Chunyan

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.