Re: [libvirt] [PATCH] audit: Log only an info message if audit_level < 2 and audit is not supported

On 11/27/2017 07:02 PM, Marc Hartmayer wrote: > Replace the error message during startup of libvirtd with an info > message if audit_level < 2 and audit is not supported by the > kernel. Audit is not supported by the current kernel if the kernel > does not have audit compiled in or if audit is disabled (e.g. by the > kernel cmdline). > > Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt; > Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; > --- > daemon/libvirtd.c | 2 +- > src/util/viraudit.c | 17 +++++++++++++++-- > src/util/viraudit.h | 2 +- > 3 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c > index 589b32192e3d..6bbff0d45684 100644 > --- a/daemon/libvirtd.c > +++ b/daemon/libvirtd.c > @@ -1418,7 +1418,7 @@ int main(int argc, char **argv) { > > if (config->audit_level) { > VIR_DEBUG("Attempting to configure auditing subsystem"); > - if (virAuditOpen() < 0) { > + if (virAuditOpen(config->audit_level) < 0) { > if (config->audit_level > 1) { > ret = VIR_DAEMON_ERR_AUDIT; > goto cleanup; > diff --git a/src/util/viraudit.c b/src/util/viraudit.c > index 17e58b3a9574..9b755e384f24 100644 > --- a/src/util/viraudit.c > +++ b/src/util/viraudit.c > @@ -55,11 +55,24 @@ static int auditfd = -1; > #endif > static bool auditlog; > > -int virAuditOpen(void) > +int virAuditOpen(unsigned int audit_level) @audit_level might be unused if building without AUDIT enabled.

> { > #if WITH_AUDIT > if ((auditfd = audit_open()) < 0) { > - virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); > + /* You get these error codes only when the kernel does not > + * have audit compiled in or it's disabled (e.g. by the kernel > + * cmdline) */ > + if (errno == EINVAL || errno == EPROTONOSUPPORT || > + errno == EAFNOSUPPORT) { > + const char msg[] = "Audit is not supported by the kernel"; > + if (audit_level < 2) > + VIR_INFO("%s", _(msg)); This is going to be terrible for translators. If anything, this needs to be: const char *msg = _("error message"); if () VIR_INFO("%s", msg); else virReportError(msg); However, I don't think that we need VIR_INFO to have translated messages at all, therefore we can go with just: if () VIR_INFO("Audit is not supported"); else virReportError(_("Audit is not supported"));

> + else > + virReportError(VIR_FROM_THIS, "%s", _(msg)); > + } else { > + virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); > + } > + Otherwise looking good. In fact, we document the behaviour you're implementing. Wonder how we ended up there.

Michal