4:02 p.m.
Thanks Aline!
On Mon, 2014-04-14 at 15:59 -0300, Aline Manera wrote:
On 04/11/2014 02:06 PM, Christy Perez wrote:
>
>
> On Fri, 2014-04-11 at 11:54 +0800, Mark Wu wrote:
>> On 04/11/2014 06:56 AM, Christy Perez wrote:
>>> selinux has a special boolean to make it easier for disk images
>>> to be managedi by libvirt. Set this to true when a user
>>> adds an NFS storage pool.
>>>
>>> Most virtualzation documentation recommends that this be set
>>> to true. For example:
>>> http://www.ovirt.org/Troubleshooting_NFS_Storage_Issues
>>> http://fedoraproject.org/wiki/How_to_debug_Virtualization_problems
>>>
>>> This will leave it set to true, even if
>>> the user removes NFS storage pools. It is not a security risk, and
>>> we should not set it to False in case it had already been set by the
>>> user for another non-kimchi use.
>>>
>>> Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com>
>>> ---
>>> src/kimchi/model/storagepools.py | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/src/kimchi/model/storagepools.py
b/src/kimchi/model/storagepools.py
>>> index 5af33b7..1ec6e99 100644
>>> --- a/src/kimchi/model/storagepools.py
>>> +++ b/src/kimchi/model/storagepools.py
>>> @@ -126,6 +126,13 @@ class StoragePoolsModel(object):
>>> kimchi_log.error("Problem creating Storage Pool:
%s", e)
>>> raise OperationFailed("KCHPOOL0007E",
>>> {'name': name, 'err':
e.get_error_message()})
>>> + if params['type'] == 'netfs':
>>> + output, error, returncode = run_command(['setsebool',
'-P',
>>> +
'virt_use_nfs=1'])
>> The persistent change of sebool is a very time-consuming operation
>> compared with the runtime change.
>> Please see:
>> [root@localhost libvirt]# time setsebool -P virt_use_nfs=1
>>
>> real 0m10.686s
>> user 0m9.680s
>> sys 0m0.337s
>> [root@localhost libvirt]# time setsebool virt_use_nfs=1
>>
>> real 0m0.035s
>> user 0m0.001s
>> sys 0m0.005s
>>
>> time getsebool virt_use_nfs
>> virt_use_nfs --> off
>>
>> real 0m0.002s
>> user 0m0.000s
>> sys 0m0.001s
>>
>>
>> So I think we could have a little optimization here: check the bool
>> value by getsebool before setting it.
>> I am fine to leave it in a following up patch.
>>> + if error or returncode:
>> Just checking returncode is enough, isn't it?
>>> + kimchi_log.error('Unable to set virt_use_nfs=1. If you
use
>>> + SELinux, this may prevent NFS pools from
>>> + being used.')
>>> return name
>>>
>>> def _clean_scan(self, pool_name):
> Thanks for the comments, Mark. This has uncovered some interesting
> behavior in trying to implement your suggestion.
>
> The error situation I was trying to handle here is if (as Royce pointed
> out), there is no selinux on a system. I assumed that if the command
> wasn't found, there wouldn't be an rc, but that an error message would
> come back. I did some experiments, and it turns out that the way
> kimchi's run_command() works, if a command doesn't exist, there's an
> error printed (e.g. Failed to run command: fakesebool -P virt_use_nfs=1.
> [Errno 2] No such file or directory), but then run_command returns None,
> None, None because of the exception. So, really, I'll get back no error
> or returncode in this situation! I'm not sure this is the correct
> behavior in case of an exception -- but that can be taken up in a
> separate thread.
>
> This also makes it more difficult to know if getsebool virt_use_nfs is
> set or not. If getsebool causes that exception, it won't get back an rc,
> or output, or an error message.
>
> Since this won't be called very often, I think it's okay to leave it
> as-is (except fix the errors that Aline pointed out in my string), and
> revisit after we decide how to deal with run_command().
>
> If that sounds okay, Aline -- are you ok with throwing in some new-line
> chars at the end of those strings, or do you want me to submit a new
> patch with that?
I will fix it before applying so you don't need to resend the patch.
> Regards,
>
>
> - Christy
>