[libvirt] [PATCH] is_selinux_enabled returns -1 on error, account for this.

[Scott Sullivan]

On 03/18/2014 01:02 PM, Scott Sullivan wrote:
> Per the documentation, is_selinux_enabled() returns -1 on error. 
> Account for this. Previously when -1 was being returned the condition 
> would still be true. I was noticing this because on my system that has 
> selinux disabled I was getting this in the libvirt.log every 5 seconds:
>
> error : virIdentityGetSystem:173 : Unable to lookup SELinux process 
> context: Invalid argument
>
> With this patch applied, I no longer get these messages every 5 
> seconds. I am submitting this in case its deemed useful for inclusion. 
> Anyone have any comments on this change? This is a patch off current 
> master.
>
>
> From 23e0780db43ebd3ea90710750639df901c261674 Mon Sep 17 00:00:00 2001
> From: Scott Sullivan <ssullivan at liquidweb.com>
> Date: Tue, 18 Mar 2014 12:55:50 -0400
> Subject: [PATCH] is_selinux_enabled returns -1 on error, account for 
> this.
>
> ---
>  src/security/security_selinux.c |    2 +-
>  src/util/viridentity.c          |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/security/security_selinux.c 
> b/src/security/security_selinux.c
> index 02c7496..5f46bef 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -784,7 +784,7 @@ error:
>  static int
>  virSecuritySELinuxSecurityDriverProbe(const char *virtDriver)
>  {
> -    if (!is_selinux_enabled())
> +    if (is_selinux_enabled() <= 0)
>          return SECURITY_DRIVER_DISABLE;
>
>      if (virtDriver && STREQ(virtDriver, "LXC")) {
> diff --git a/src/util/viridentity.c b/src/util/viridentity.c
> index 351fdd7..05e7568 100644
> --- a/src/util/viridentity.c
> +++ b/src/util/viridentity.c
> @@ -169,7 +169,7 @@ virIdentityPtr virIdentityGetSystem(void)
>          goto cleanup;
>
>  #if WITH_SELINUX
> -    if (is_selinux_enabled()) {
> +    if (is_selinux_enabled() > 0) {
>          if (getcon(&con) < 0) {
>              virReportSystemError(errno, "%s",
>                                   _("Unable to lookup SELinux process 
> context"));

ping?

Looking for a ACK/NACK on this from a committer. In the case of an error 
condition when calling is_selinux_enabled() it seems safer to assume 
seLinux isn't enabled than to assume it is. If you assume its enabled 
like it is in master, at least one result is "Unable to lookup SELinux 
process context" spewed into libvirt.log many times a minute on my 
systems causing the file to grow large, and needless IO.

On my systems that do exhibit this behavior (CentOS 6), I show seLinux 
as disabled:

[root at host ~]# sestatus
SELinux status:                 disabled
[root at host ~]#