On 29/03/20 13:22, Stefan Raspl wrote: > I wrote a respective patch and tried it out, and found this approach not to be > workable for a number of reasons: > - The implicit buffering that takes place when redirecting output of kvm_stat in > logging mode to a file messes up the format: logrotate moves the files on the > disks, but there might be still data buffered that hasn't been written out > yet. The SIGHUP triggers a new header to be written with the patch I came up > with, but that header would sometimes appear only after a couple of lines > written to the file, which messes up the format. Flushing stdout in the signal > handler won't help, either - it's already too late by then. I don't understand this. Unless you were using copytruncate, the sequence should be: - logrotate moves file A to B - your file descriptor should point to B, so kvm_stat keeps writing to file B - you send the signal to kvm_stat - kvm_stat closes file B, so all pending records are written - kvm_stat reopens file A and writes the header. If you have a race of some sort, try having the signal handler do nothing but set a flag that is examined in the main loop. > - When restarting kvm_stat logging (e.g. after a reboot), appending to an > existing file should suppress a new header from being written, or it would end > up somewhere in the middle of the file, messing up the format. kvm_stat > doesn't know that or where its output is redirected to, so no chance of > suppressing it within kvm_stat. We would probably require some kind of wrapper > script (and possibly an extra cmd-line option to suppress the header on > start). You could stat the output file, and suppress the header if it is a regular non-empty file. But it would be a problem anyway if the header has changed since the last boot, which prompts the stupid and lazy question: how does your series deal with this? This one seems the biggest of the three problems to me. > - I was surprised to realize that SIGHUP is actually not part of logrotate - > one has to write that manually into the logrotate config as a postrotate... > and I'll openly admit that writing a respective killall-command that aims at a > python script doesn't seem to be trivial... This one is easy, put "ExecReload=/bin/kill -HUP $MAINPID" in the systemd unit and use "systemctl reload kvm_stat.service" in the postrotate command. > As much as I sympathize with re-use of existing components, I'd like to point > out that my original patch was also re-using existing python code for rotating > logs, and made things just _so_ much easier from a user perspective. I understand that, yes. My request was more about not requiring kvm_stat-specific configuration than about reusing existing components, though. Paolo
