5:56 p.m.
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'])
+ if error or returncode:
+ 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):
--
1.9.0
8:53 p.m.
Reviewed-by: Royce Lv<lvroyce(a)linux.vnet.ibm.com>
On 2014年04月11日 06:56, 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'])
+ if error or returncode:
+ 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):
10:19 p.m.
The tests are failing with this patch set:
E..........EEEE......E.....EE
======================================================================
ERROR: test_exception (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_exception
Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python2.7/unittest/loader.py", line 230, in
_get_module_from_name
__import__(name)
File "/home/alinefm/kimchi/tests/test_exception.py", line 25, in
<module>
import kimchi.mockmodel
File "/home/alinefm/kimchi/src/kimchi/mockmodel.py", line 49, in
<module>
from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
File "/home/alinefm/kimchi/src/kimchi/model/storagepools.py", line 133
kimchi_log.error('Unable to set virt_use_nfs=1. If you use
^
SyntaxError: EOL while scanning string literal
======================================================================
ERROR: test_mockmodel (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_mockmodel
Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python2.7/unittest/loader.py", line 230, in
_get_module_from_name
__import__(name)
File "/home/alinefm/kimchi/tests/test_mockmodel.py", line 27, in
<module>
import kimchi.mockmodel
File "/home/alinefm/kimchi/src/kimchi/mockmodel.py", line 49, in
<module>
from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
File "/home/alinefm/kimchi/src/kimchi/model/storagepools.py", line 133
kimchi_log.error('Unable to set virt_use_nfs=1. If you use
^
SyntaxError: EOL while scanning string literal
======================================================================
ERROR: test_rest (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_rest
Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python2.7/unittest/loader.py", line 230, in
_get_module_from_name
__import__(name)
File "/home/alinefm/kimchi/tests/test_rest.py", line 35, in <module>
import kimchi.mockmodel
File "/home/alinefm/kimchi/src/kimchi/mockmodel.py", line 49, in
<module>
from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
File "/home/alinefm/kimchi/src/kimchi/model/storagepools.py", line 133
kimchi_log.error('Unable to set virt_use_nfs=1. If you use
^
SyntaxError: EOL while scanning string literal
======================================================================
ERROR: test_model (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_model
Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python2.7/unittest/loader.py", line 230, in
_get_module_from_name
__import__(name)
File "/home/alinefm/kimchi/tests/test_model.py", line 34, in <module>
import utils
File "/home/alinefm/kimchi/tests/utils.py", line 35, in <module>
import kimchi.server
File "/home/alinefm/kimchi/src/kimchi/server.py", line 30, in <module>
from kimchi import mockmodel
File "/home/alinefm/kimchi/src/kimchi/mockmodel.py", line 49, in
<module>
from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
File "/home/alinefm/kimchi/src/kimchi/model/storagepools.py", line 133
kimchi_log.error('Unable to set virt_use_nfs=1. If you use
^
SyntaxError: EOL while scanning string literal
======================================================================
ERROR: test_server (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_server
Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python2.7/unittest/loader.py", line 230, in
_get_module_from_name
__import__(name)
File "/home/alinefm/kimchi/tests/test_server.py", line 24, in <module>
import utils
File "/home/alinefm/kimchi/tests/utils.py", line 35, in <module>
import kimchi.server
File "/home/alinefm/kimchi/src/kimchi/server.py", line 30, in <module>
from kimchi import mockmodel
File "/home/alinefm/kimchi/src/kimchi/mockmodel.py", line 49, in
<module>
from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
File "/home/alinefm/kimchi/src/kimchi/model/storagepools.py", line 133
kimchi_log.error('Unable to set virt_use_nfs=1. If you use
^
SyntaxError: EOL while scanning string literal
======================================================================
ERROR: test_authorization (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_authorization
Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python2.7/unittest/loader.py", line 230, in
_get_module_from_name
__import__(name)
File "/home/alinefm/kimchi/tests/test_authorization.py", line 28, in
<module>
import kimchi.mockmodel
File "/home/alinefm/kimchi/src/kimchi/mockmodel.py", line 49, in
<module>
from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
File "/home/alinefm/kimchi/src/kimchi/model/storagepools.py", line 133
kimchi_log.error('Unable to set virt_use_nfs=1. If you use
^
SyntaxError: EOL while scanning string literal
======================================================================
ERROR: test_networkxml (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_networkxml
Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python2.7/unittest/loader.py", line 230, in
_get_module_from_name
__import__(name)
File "/home/alinefm/kimchi/tests/test_networkxml.py", line 25, in
<module>
import utils
File "/home/alinefm/kimchi/tests/utils.py", line 35, in <module>
import kimchi.server
File "/home/alinefm/kimchi/src/kimchi/server.py", line 30, in <module>
from kimchi import mockmodel
File "/home/alinefm/kimchi/src/kimchi/mockmodel.py", line 49, in
<module>
from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
File "/home/alinefm/kimchi/src/kimchi/model/storagepools.py", line 133
kimchi_log.error('Unable to set virt_use_nfs=1. If you use
^
SyntaxError: EOL while scanning string literal
======================================================================
ERROR: test_plugin (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_plugin
Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python2.7/unittest/loader.py", line 230, in
_get_module_from_name
__import__(name)
File "/home/alinefm/kimchi/tests/test_plugin.py", line 28, in <module>
import kimchi.mockmodel
File "/home/alinefm/kimchi/src/kimchi/mockmodel.py", line 49, in
<module>
from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
File "/home/alinefm/kimchi/src/kimchi/model/storagepools.py", line 133
kimchi_log.error('Unable to set virt_use_nfs=1. If you use
^
SyntaxError: EOL while scanning string literal
----------------------------------------------------------------------
Ran 29 tests in 0.011s
FAILED (errors=8)
make[3]: *** [check-local] Error 1
make[3]: Leaving directory `/home/alinefm/kimchi/tests'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory `/home/alinefm/kimchi/tests'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/alinefm/kimchi/tests'
make: *** [check-recursive] Error 1
On 04/10/2014 07:56 PM, 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'])
+ if error or returncode:
+ 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):
10:54 p.m.
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):
12:06 p.m.
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?
Regards,
- Christy
4:20 a.m.
On 04/12/2014 01:06 AM, 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.
I believe we need fix this issue in run_command, which should
reflect the exception too. Would you like to post a patch to improve it?
Thanks!
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().
Yes, it's fine for me
to leave it as-is now.
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?
Regards,
- Christy
1:59 p.m.
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
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
>
3902
days inactive
3908
days old
8 comments
4 participants
participants (4)
-
Aline Manera -
Christy Perez -
Mark Wu -
Royce Lv