[libvirt] [PATCH] sanlock: Enhance error message to point to possible problem with selinux
Daniel P. Berrange
berrange at redhat.com
Thu Mar 15 16:58:47 UTC 2012
On Thu, Mar 15, 2012 at 10:55:20AM -0600, Eric Blake wrote:
> 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?]
I don't really agree with that previous commit either for the
same reasons.
>
> >
> > 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.
I still don't think it belongs in the error message - with this
kind of message, they'll just see the instruction & blindly follow it
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list