On 12/13/2017 10:45 AM, Marc Hartmayer wrote:
On Wed, Dec 13, 2017 at 10:22 AM +0100, Michal Privoznik <mprivozn@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@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@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“.
Oh, it's just me being lazy to write the full error message. I wasn't focusing on exact phrasing rather than the structure of code. Feel free to use whatever message you want. The one you already have is fine.
+ 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?
Yes please. Michal