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