daemon/libvirtd.c | 2 +- src/util/viraudit.c | 17 +++++++++++++++-- src/util/viraudit.h | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-)
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)
{
#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));
+ else
+ virReportError(VIR_FROM_THIS, "%s", _(msg));
+ } else {
+ virReportSystemError(errno, "%s", _("Unable to initialize audit layer"));
+ }
+
return -1;
}
diff --git a/src/util/viraudit.h b/src/util/viraudit.h
index edaddf3c886f..e0471be1a85d 100644
--- a/src/util/viraudit.h
+++ b/src/util/viraudit.h
@@ -32,7 +32,7 @@ typedef enum {
VIR_AUDIT_RECORD_RESOURCE,
} virAuditRecordType;
-int virAuditOpen(void);
+int virAuditOpen(unsigned int audit_level);
void virAuditLog(bool enabled);
--
2.13.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Nov 27, 2017 at 07:02 PM +0100, Marc Hartmayer <mhartmay@linux.vnet.ibm.com> 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). Maybe we should also take get_auditfail_action(...) [1] into consideration to determine what action should be taken when the audit subsystem is unavailable? This function is available starting with audit 1.2.4. [1] https://linux.die.net/man/3/get_auditfail_action […snip] -- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Nov 27, 2017 at 07:02 PM +0100, Marc Hartmayer <mhartmay@linux.vnet.ibm.com> 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)
> {
> #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));
> + else
> + virReportError(VIR_FROM_THIS, "%s", _(msg));
> + } else {
> + virReportSystemError(errno, "%s", _("Unable to initialize audit layer"));
> + }
> +
> return -1;
> }
>
> diff --git a/src/util/viraudit.h b/src/util/viraudit.h
> index edaddf3c886f..e0471be1a85d 100644
> --- a/src/util/viraudit.h
> +++ b/src/util/viraudit.h
> @@ -32,7 +32,7 @@ typedef enum {
> VIR_AUDIT_RECORD_RESOURCE,
> } virAuditRecordType;
>
> -int virAuditOpen(void);
> +int virAuditOpen(unsigned int audit_level);
>
> void virAuditLog(bool enabled);
>
> --
> 2.13.4
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
Polite ping :)
--
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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.
> {
> #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"));
> + 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.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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“.
>> + 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.