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

[Marc Hartmayer]

On Wed, Dec 13, 2017 at 10:22 AM +0100, Michal Privoznik <mprivozn at redhat.com> wrote:
> 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 at linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>> ---
>>  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.

Hmm, right.

>
>>  {
>>  #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"));

I think this is fine - but I’m not sure we should omit „by the kernel“.

>> +            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.

Thanks for the review. Shall I send a v2?

>
> Michal
>

--
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294