[libvirt] [PATCH v2] util: audit: Fix logging an error when kernel lacks audit support
Erik Skultety
eskultet at redhat.com
Wed Jan 16 16:28:26 UTC 2019
On Wed, Jan 16, 2019 at 03:45:06PM +0000, Daniel P. Berrangé wrote:
> On Wed, Jan 16, 2019 at 03:58:24PM +0100, Erik Skultety wrote:
> > Based on an upstream discussion, reporting the errno is useful for the
> > user to know why audit isn't supported. Even though having an error in
> > the logs might look concerning when 'audit_log=1', it also denotes that
> > audit is only going to be used if it's available, continuing normally
> > if it's unavailable for whatever reason.
> >
> > Partially reverts commit 4199c2f221c.
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1596119
> >
> > Signed-off-by: Erik Skultety <eskultet at redhat.com>
> > ---
> > src/remote/remote_daemon.c | 2 +-
> > src/util/viraudit.c | 16 ++--------------
> > src/util/viraudit.h | 2 +-
> > 3 files changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> > index 3be3ad02fc..ededef97b4 100644
> > --- a/src/remote/remote_daemon.c
> > +++ b/src/remote/remote_daemon.c
> > @@ -1380,7 +1380,7 @@ int main(int argc, char **argv) {
> >
> > if (config->audit_level) {
> > VIR_DEBUG("Attempting to configure auditing subsystem");
> > - if (virAuditOpen(config->audit_level) < 0) {
> > + if (virAuditOpen() < 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 a02e5b36fd..135d0e626a 100644
> > --- a/src/util/viraudit.c
> > +++ b/src/util/viraudit.c
> > @@ -54,23 +54,11 @@ static int auditfd = -1;
> > #endif
> > static bool auditlog;
> >
> > -int virAuditOpen(unsigned int audit_level ATTRIBUTE_UNUSED)
> > +int virAuditOpen(void)
> > {
> > #if WITH_AUDIT
> > if ((auditfd = audit_open()) < 0) {
> > - /* 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) {
> > - if (audit_level < 2)
> > - VIR_INFO("Audit is not supported by the kernel");
> > - else
> > - virReportError(VIR_FROM_THIS, "%s", _("Audit is not supported by the kernel"));
> > - } else {
> > - virReportSystemError(errno, "%s", _("Unable to initialize audit layer"));
> > - }
> > -
> > + virReportSystemError(errno, "%s", _("Unable to initialize audit layer"));
> > return -1;
> > }
>
> I'm not a fan of this - I don't think we should report stuff in the
> logs at level error: if we're treating it as non-fatal.
>
> I think it can be refactored to push all the handling of
> audit_level down into virAuditOpen though, instead of having
> it split between virAuditOpen & the caller.
Okay, honestly, this was my quick way away from this, since I wanted to
populate the log with the errno translation either way and get rid of that
bugzilla, which has literally no priority btw and which looked like a 5min fix,
so I'm not going to put any more effort into this for the time being.
Thanks,
Erik
More information about the libvir-list
mailing list