7:21 a.m.
Replies in-line...
On Mon, 2014-03-31 at 17:49 -0300, Paulo Ricardo Paz Vital wrote:
On Fri, 2014-03-28 at 16:20 -0500, Christy Perez wrote:
> selinux has a special boolean to make it easier for disk images
> to be stored on a remote NFS server. 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/i18n.py | 2 ++
> src/kimchi/model/storagepools.py | 5 +++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index d45f607..8ade7d7 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -144,6 +144,8 @@ messages = {
> "KCHPOOL0034E": _("Unable to deactivate pool %(name)s as it is
associated with some templates"),
> "KCHPOOL0035E": _("Unable to delete pool %(name)s as it is
associated with some templates"),
> "KCHPOOL0036E": _("A volume group named '%(name)s'
already exists. Please, choose another name to create the logical pool."),
> + "KCHPOOL0037E": _("Unable to set selinux bool virt_use_nfs for
NFS pool usage. Depending on \
> + your NFS config, this may prevent the pool from being
used."),
IMO, this is too long. Why not only "Unable to set selinux bool
virt_use_nfs for NFS pool usage." ?
It is long. :) I was trying to make it
clear that this isn't always a
problem per se. A patch that Royce sent earlier that updated the README
described a situation which didn't need for this to be set. I am trying
to keep users from panicking if they see this in their error log. If
others think it needs to be re-worded, I am open to suggestions.
>
> "KCHVOL0001E": _("Storage volume %(name)s already
exists"),
> "KCHVOL0002E": _("Storage volume %(name)s does not exist in
storage pool %(pool)s"),
> diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py
> index 92b2496..d279ffa 100644
> --- a/src/kimchi/model/storagepools.py
> +++ b/src/kimchi/model/storagepools.py
> @@ -126,6 +126,11 @@ 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('KCHPOOL0037E')
Why only logging the message? I think this should be exposed to user.
For the same
reason stated above. It might not need addressing, and I
don't want to bubble it up. For all we know, an admin uninstalled
selinux completely and they aren't going to need to work around it. If
someone is having issues, I want them to be able to find this as a
clue.
> return name
>
> def _clean_scan(self, pool_name):
Thanks!
- Christy