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@xxxxxxxxxx> > > --- > > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
