[libvirt] [PATCH v2] util: audit: Fix logging an error when kernel lacks audit support

[Daniel P. Berrangé]

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.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|