Re: [PATCH 0/7] tools/kvm_stat: add logfile support

Stefan Raspl
2020-03-23T09:58:19Z

On 2020-03-19 12:54, Paolo Bonzini wrote:
> On 06/03/20 12:42, Stefan Raspl wrote:
>> This patch series provides a couple of new options to make logging to
>> files feasible.
>> Specifically, we add command line switches to specify an arbitrary time
>> interval for logging, and to toggle between a .csv and the previous
>> file format. Furthermore, we allow logging to files, where we utilize a
>> rotating set of 6 logfiles, each with its own header for easy post-
>> processing, especially when using .csv format.
>> Since specifying logfile size limits might be a non-trivial exercise,
>> we're throwing in yet another command line option that allows to
>> specify the minimum timeframe that should be covered by logs.
>> Finally, there's a minimal systemd unit file to deploy kvm_stat-based
>> logging in Linux distributions.
>> Note that the decision to write our own logfiles rather than to log to
>> e.g. systemd journal is a conscious one: It is effectively impossible to
>> write csv records into the systemd journal, the header will either
>> disappear after a while or has to be repeated from time to time, which
>> defeats the purpose of having a .csv format that can be easily post-
>> processed, etc.
>> See individual patch description for further details.
>>
>>
>> Stefan Raspl (7):
>> tools/kvm_stat: rework command line sequence and message texts
>> tools/kvm_stat: switch to argparse
>> tools/kvm_stat: add command line switch '-s' to set update interval
>> tools/kvm_stat: add command line switch '-c' to log in csv format
>> tools/kvm_stat: add rotating log support
>> tools/kvm_stat: add command line switch '-T'
>> tools/kvm_stat: add sample systemd unit file
>>
>> tools/kvm/kvm_stat/kvm_stat | 434 +++++++++++++++++++++-------
>> tools/kvm/kvm_stat/kvm_stat.service | 15 +
>> tools/kvm/kvm_stat/kvm_stat.txt | 59 ++--
>> 3 files changed, 384 insertions(+), 124 deletions(-)
>> create mode 100644 tools/kvm/kvm_stat/kvm_stat.service
>>
>
> I queued patches 1-4. For the others, however, I would prefer to add
> support for SIGHUP instead (to reopen the logfile), so that one can use
> the usual logrotate services.

Thx!
As for SIGHUP: The problem that I see with logrotate and likewise approaches is
how the heading is being handled: If it is reprinted every x lines (like the
original logging format in kvmstat does), then it messes up any chance of
loading the output in external tools for further processing.
If the heading is printed once only, then it will get pushed out of the log
files at some time - which is fatal, since '-f <fields>' allows to specify
custom fields, so one cannot reconstruct what the fields were.
That's why I did things as I did - which works great for .csv output.
I'd really like to preserve the use-case where a user has a chance to
post-process the output, especially .csv, in other tools. So how how about we do
both, add support for SIGHUP for users who want to use logrotate (I imagine this
would be used with the original logging format only), and keep the suggested
support for 'native' log-rotation for .csv users?

Ciao,
Stefan