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;