I guess the reason for that was the automatic interpretation/stringification of
setfilecon_errno, but the code was not nice to read and it was a bit confusing.
Also, the logs and error states get cleaner this way.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
I'm still waiting for the build in travis to finish, so don't stone me if it
fails.  The link is here: https://travis-ci.org/nertpinx/libvirt/builds/578418517

 src/security/security_selinux.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 4d0c7a46ae23..bbb5318aa0ee 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1301,14 +1301,18 @@ virSecuritySELinuxSetFileconImpl(const char *path,
         if (setfilecon_errno != EOPNOTSUPP && setfilecon_errno != ENOTSUP &&
             setfilecon_errno != EROFS) {
         VIR_WARNINGS_RESET
-            virReportSystemError(setfilecon_errno,
-                                 _("unable to set security context '%s' on '%s'"),
-                                 tcon, path);
             /* However, don't claim error if SELinux is in Enforcing mode and
              * we are running as unprivileged user and we really did see EPERM.
              * Otherwise we want to return error if SELinux is Enforcing. */
-            if (security_getenforce() == 1 && (setfilecon_errno != EPERM || privileged))
+            if (security_getenforce() == 1 &&
+                (setfilecon_errno != EPERM || privileged)) {
+                virReportSystemError(setfilecon_errno,
+                                     _("unable to set security context '%s' on '%s'"),
+                                     tcon, path);
                 return -1;
+            }
+            VIR_WARN(_("unable to set security context '%s' on '%s' (errno %d)"),
+                     tcon, path, setfilecon_errno);
         } else {
             const char *msg;
             if (virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 1 &&
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 8/29/19 5:39 PM, Martin Kletzander wrote:
> I guess the reason for that was the automatic interpretation/stringification of
> setfilecon_errno, but the code was not nice to read and it was a bit confusing.
> Also, the logs and error states get cleaner this way.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
> I'm still waiting for the build in travis to finish, so don't stone me if it
> fails.  The link is here: https://travis-ci.org/nertpinx/libvirt/builds/578418517
> 
>   src/security/security_selinux.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

And safe for freeze.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Aug 29, 2019 at 05:39:11PM +0200, Martin Kletzander wrote:
>I guess the reason for that was the automatic interpretation/stringification of
>setfilecon_errno, but the code was not nice to read and it was a bit confusing.
>Also, the logs and error states get cleaner this way.
>
>Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>---
>I'm still waiting for the build in travis to finish, so don't stone me if it
>fails.  The link is here: https://travis-ci.org/nertpinx/libvirt/builds/578418517
>

Of course I sent the wrong link.  Right one is here:  https://travis-ci.org/nertpinx/libvirt/builds/578419238
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list