[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]