[Kimchi-devel] [PATCH v2] Set virt_use_nfs when NFS pool is added.

[Mark Wu]

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 at 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 at localhost libvirt]# time setsebool -P virt_use_nfs=1

real    0m10.686s
user    0m9.680s
sys    0m0.337s
[root at 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):