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@aa.jp.fujitsu.com> Thanks, Nobuhiro Itou.