[pve-devel] [RFC pve-qemu] Add systemd journal logging patch

[Stefan Reiter]

On 6/30/20 6:28 PM, Dietmar Maurer wrote:
> comments inline
> 
>> On 06/30/2020 2:06 PM Stefan Reiter <s.reiter at proxmox.com> wrote:
>>
>>   
>> Prints QEMU errors that occur *after* the "-daemonize" fork to the
>> systemd journal, instead of pushing them into /dev/null like before.
>>
>> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
>> ---
>>
>> Useful for debugging rust panics for example. I'm sure there's other ways to go
>> about this (log files? pass the journal fd from outside? pipe it into the
>> journal somehow?) but this one seems simple enough, though it of course requires
>> linking QEMU against libsystemd.
>>
>> @dietmar: is this similar to what you had in mind?
>>
>>   debian/control                                |  1 +
>>   ...ct-stderr-to-journal-when-daemonized.patch | 50 +++++++++++++++++++
>>   debian/patches/series                         |  1 +
>>   3 files changed, 52 insertions(+)
>>   create mode 100644 debian/patches/pve/0052-PVE-redirect-stderr-to-journal-when-daemonized.patch
>>
>> diff --git a/debian/control b/debian/control
>> index caceabb..e6d935d 100644
>> --- a/debian/control
>> +++ b/debian/control
>> @@ -25,6 +25,7 @@ Build-Depends: autotools-dev,
>>                  libseccomp-dev,
>>                  libspice-protocol-dev (>= 0.12.14~),
>>                  libspice-server-dev (>= 0.14.0~),
>> +               libsystemd-dev,
>>                  libusb-1.0-0-dev (>= 1.0.17-1),
>>                  libusbredirparser-dev (>= 0.6-2),
>>                  python3-minimal,
>> diff --git a/debian/patches/pve/0052-PVE-redirect-stderr-to-journal-when-daemonized.patch b/debian/patches/pve/0052-PVE-redirect-stderr-to-journal-when-daemonized.patch
>> new file mode 100644
>> index 0000000..f73de53
>> --- /dev/null
>> +++ b/debian/patches/pve/0052-PVE-redirect-stderr-to-journal-when-daemonized.patch
>> @@ -0,0 +1,50 @@
>> +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
>> +From: Stefan Reiter <s.reiter at proxmox.com>
>> +Date: Tue, 30 Jun 2020 13:10:10 +0200
>> +Subject: [PATCH] PVE: redirect stderr to journal when daemonized
>> +
>> +QEMU uses the logging for error messages usually, so LOG_ERR is most
>> +fitting.
>> +---
>> + Makefile.objs | 1 +
>> + os-posix.c    | 7 +++++--
>> + 2 files changed, 6 insertions(+), 2 deletions(-)
>> +
>> +diff --git a/Makefile.objs b/Makefile.objs
>> +index b7d58e592e..105f23bff7 100644
>> +--- a/Makefile.objs
>> ++++ b/Makefile.objs
>> +@@ -55,6 +55,7 @@ common-obj-y += net/
>> + common-obj-y += qdev-monitor.o
>> + common-obj-$(CONFIG_WIN32) += os-win32.o
>> + common-obj-$(CONFIG_POSIX) += os-posix.o
>> ++os-posix.o-libs := -lsystemd
>> +
>> + common-obj-$(CONFIG_LINUX) += fsdev/
>> +
>> +diff --git a/os-posix.c b/os-posix.c
>> +index 3cd52e1e70..ab4d052c62 100644
>> +--- a/os-posix.c
>> ++++ b/os-posix.c
>> +@@ -28,6 +28,8 @@
>> + #include <pwd.h>
>> + #include <grp.h>
>> + #include <libgen.h>
>> ++#include <systemd/sd-journal.h>
>> ++#include <syslog.h>
>> +
>> + #include "qemu-common.h"
>> + /* Needed early for CONFIG_BSD etc. */
>> +@@ -309,9 +311,10 @@ void os_setup_post(void)
>> +
>> +         dup2(fd, 0);
>> +         dup2(fd, 1);
> 
> I guess we also want to redirect stdout. Or does that produce too much noise?
> 

I figured since QEMU doesn't redirect it to its log file either that it 
wouldn't produce anything useful on stdout?

>> +-        /* In case -D is given do not redirect stderr to /dev/null */
>> ++        /* In case -D is given do not redirect stderr to journal */
>> +         if (!qemu_logfile) {
>> +-            dup2(fd, 2);
>> ++            int journal_fd = sd_journal_stream_fd("QEMU", LOG_ERR, 0);
>> ++            dup2(journal_fd, 2);
>> +         }
>> +
>> +         close(fd);
>> diff --git a/debian/patches/series b/debian/patches/series
>> index 5d6a5d6..e658c1a 100644
>> --- a/debian/patches/series
>> +++ b/debian/patches/series
>> @@ -50,3 +50,4 @@ pve/0048-savevm-async-add-debug-timing-prints.patch
>>   pve/0049-Add-some-qemu_vfree-statements-to-prevent-memory-lea.patch
>>   pve/0050-Fix-backup-for-not-64k-aligned-storages.patch
>>   pve/0051-PVE-Backup-Add-dirty-bitmap-tracking-for-incremental.patch
>> +pve/0052-PVE-redirect-stderr-to-journal-when-daemonized.patch
>> -- 
>> 2.20.1
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>