[libvirt] [PATCH] selinux: Correctly report warning if virt_use_nfs not set

Laine Stump laine at laine.org
Thu Sep 22 16:56:02 UTC 2011


On 09/22/2011 05:48 AM, Michal Privoznik wrote:
> Previous patch c9b37fee tried to deal with virt_use_nfs. But
> setfilecon() returns EOPNOTSUPP on NFS so we need to move the
> warning to else branch.

I have a vague memory of the error code of something like this changing 
from some other error on an older version of RHEL to EOPNOTSUPP on newer 
version. It may have been for something else, but may be worth checking 
out to make sure this patch gives the desired results with, e.g. RHEL5 
and RHEL6.0 as well as 6.2 and Fedora.

> ---
>   src/security/security_selinux.c |   27 +++++++++++++++------------
>   1 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 028f5b2..9a9a305 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -420,23 +420,26 @@ SELinuxSetFilecon(const char *path, char *tcon)
>            * virt_use_{nfs,usb,pci}  boolean tunables to allow it...
>            */
>           if (setfilecon_errno != EOPNOTSUPP) {
> -            const char *errmsg;
> -            if ((virStorageFileIsSharedFSType(path,
> -                                             VIR_STORAGE_FILE_SHFS_NFS) == 1)&&
> -                security_get_boolean_active("virt_use_nfs") != 1) {
> -                errmsg = _("unable to set security context '%s' on '%s'. "
> -                           "Consider setting virt_use_nfs");
> -            } else {
> -                errmsg = _("unable to set security context '%s' on '%s'");
> -            }
>               virReportSystemError(setfilecon_errno,
> -                                 errmsg,
> +                                 _("unable to set security context '%s' on '%s'"),
>                                    tcon, path);
>               if (security_getenforce() == 1)
>                   return -1;
>           } else {
> -            VIR_INFO("Setting security context '%s' on '%s' not supported",
> -                     tcon, path);
> +            const char *msg;
> +            if ((virStorageFileIsSharedFSType(path,
> +                                              VIR_STORAGE_FILE_SHFS_NFS) == 1)&&
> +                security_get_boolean_active("virt_use_nfs") != 1) {
> +                msg = _("Setting security context '%s' on '%s' not supported. "
> +                        "Consider setting virt_use_nfs");
> +               if (security_getenforce() == 1)
> +                   VIR_WARN(msg, tcon, path);
> +               else
> +                   VIR_INFO(msg, tcon, path);
> +            } else {
> +                VIR_INFO(_("Setting security context '%s' "
> +                           "on '%s' not supported"), tcon, path);
> +            }

You could turn this into an if () { ... } else if () { ... } else { ... 
} (ie, remove the extra nesting level of the new addition). I can live 
with it either way, though, so ACK.

>           }
>       }
>       return 0;




More information about the libvir-list mailing list