Re: [libvirt] [PATCH] sanlock: Enhance error message to point to possible problem with selinux

On 03/15/2012 10:35 AM, Daniel P. Berrange wrote: > On Thu, Mar 15, 2012 at 05:23:09PM +0100, Peter Krempa wrote: >> If the connection to the sanlock daemon is forbidden by selinux the >> error message was not clear enough. This patch adds a check if proper >> configuration for selinux is used while trying to connect to sanlock. >> >> *src/locking/lock_driver_sanlock.c: >> - add macro virLockSystemError that checks for selinux and >> reports an improved error message >> - modify calls of virReportSystemError to the new macro in >> apropriate places >> >> Background: >> https://bugzilla.redhat.com/show_bug.cgi?id=770488 > > IMHO this is not something we should do here. You're outputing the > message regardless of whether there is even an NFS volume involved, > and harcoding details of the SELinux policy. Finally I don't think > we should blindly tell people to change SELinux tunables without > explaining the implications, which is not practical in an error > message. We've done this sort of targeted error message before; but there we were careful to _only_ issue the message after checking that we were indeed dealing with NFS; see commit 1888363d. [Hmm - should that patch be revisited, to mention virt_use_samba if it was samba rather than nfs that caused the SELinux denial, since those are different bools?]

> > So, IMHO, this belongs in documentation, not in the error messages > here. I think both are appropriate - we definitely need to mention virt_use_sanlock in locking.html.in, with full implications, but I would also like to see the error message, provided that you can be sure that the error message only mentions virt_use_sanlock in the actual case where SELinux is enforcing and the error is confirmed to be due to NFS causing us to need the bool.