Re: [PATCH] output virsh log to file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Daniel

Thank you for your comment and code review.

> > >   But since the patch is relatively simple based on existing virsh logging
> > > code, I think this could go as a command line option for virsh, for example
> > >    --log filename
> > > where the detailed logs can then be saved if needed when a problem occurs.
> > > I think this would avoid the main drawbacks of your proposed patch.
> > 
> > I agree about a command line option.
> > So, I remaked the patch which --log option is added for virsh.
> > How about this one?
> 
>  I guess you still think of a single log file shared by multiple virsh run,
> and honnestly I think this adds way too much complexity and is not really
> useful. You have one log file per virsh run. It's the responsability of the
> user to pick a log file name. Trying to reinvent syslog at the level of virsh
> does not sounds right to me, or rather it makes what should be a small patch
> something really complex instead, I am not sure it is worth it.

Okey, I corrected all review point.

[...]

>   I expect the use to be the following:
>      - users uses virsh for virtualization operation
>      - something suddenly does not work
>      - then he re-runs the command with logging 
>      - then he can analyze the log or transmit it to a sysadmin who
>        can have a look
> but I don't believe in reimplementing something like syslog within virsh to 
> log all operations all the time, especially with a fixed size buffer. logs 
> will be intermixed, hard to process, add a burden on the server, and makes
> the code way more complex than it needs to be.
> 
>   Maybe I didn't understood how you expected logging to work, but apparently
> we had different viewpoints, I would rather go for the simplest,

I agree.

>   does that still work for your use case ?

Yes.
How about this attached patch?

Signed-off-by: Nobuhiro Itou <fj0873gn@xxxxxxxxxxxxxxxxx>


Thanks,
Nobuhiro Itou.

Attachment: virsh_log.patch
Description: Binary data


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]