[PATCH] Set virt_use_nfs when NFS pool is added.
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@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."), "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') return name def _clean_scan(self, pool_name): -- 1.8.5.3
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@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." ?
"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.
return name
def _clean_scan(self, pool_name):
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@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
On 2014年03月29日 05:20, 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@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."),
>
> "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'])
1. what about turn this on when start kimchi? Cause we just need to
enable this for the first time.
2. For Debian using apparmor, it does not have setsebool, I think this
need to be handled too.
> + if error or returncode:
> + kimchi_log.error('KCHPOOL0037E')
> return name
>
> def _clean_scan(self, pool_name):
On Tue, 2014-04-01 at 14:24 +0800, Royce Lv wrote:
> On 2014年03月29日 05:20, 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@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."),
> >
> > "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'])
> 1. what about turn this on when start kimchi? Cause we just need to
> enable this for the first time.
I'm okay with that too, but I figured setting it only if it'll be used
made more sense. Is there a reason to set it at startup vs this?
> 2. For Debian using apparmor, it does not have setsebool, I think this
> need to be handled too.
I was using the package repository logic of "just try to set it." I
figured there were too many "what ifs" to check and went with a simple
approach. Is that going to cause issues? Is there an equivalent to
virt_use_nfs for Debian? Or will this problem not occur there?
> > + if error or returncode:
> > + kimchi_log.error('KCHPOOL0037E')
> > return name
> >
> > def _clean_scan(self, pool_name):
>
On 04/01/2014 11:02 AM, Christy Perez wrote:
On Tue, 2014-04-01 at 14:24 +0800, Royce Lv wrote:
On 2014年03月29日 05:20, 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@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."),
I think that log messages does not need to be translated, so you do not need to add it to i18n. I may be wrong, but, for instance, if someone is using kimchi in chinese, then the log entry will be in chinese. The Kimchi server might be in another place, where the admin does not necessarily understand Chinese. Can someone confirm this ? Please
"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'])
1. what about turn this on when start kimchi? Cause we just need to enable this for the first time. I'm okay with that too, but I figured setting it only if it'll be used made more sense. Is there a reason to set it at startup vs this?
2. For Debian using apparmor, it does not have setsebool, I think this need to be handled too. I was using the package repository logic of "just try to set it." I figured there were too many "what ifs" to check and went with a simple approach. Is that going to cause issues? Is there an equivalent to virt_use_nfs for Debian? Or will this problem not occur there?
+ if error or returncode: + kimchi_log.error('KCHPOOL0037E') return name
def _clean_scan(self, pool_name):
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 04/01/2014 04:45 PM, Rodrigo Trujillo wrote:
On 04/01/2014 11:02 AM, Christy Perez wrote:
On Tue, 2014-04-01 at 14:24 +0800, Royce Lv wrote:
On 2014年03月29日 05:20, 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@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."),
I think that log messages does not need to be translated, so you do not need to add it to i18n. I may be wrong, but, for instance, if someone is using kimchi in chinese, then the log entry will be in chinese. The Kimchi server might be in another place, where the admin does not necessarily understand Chinese.
Can someone confirm this ? Please
I confirmed with Aline and, actually, the log functions do not translate "error code" in messages.... so you would see "KCHPOOL0037E" in the log. So, you must hard code the log message.
"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'])
1. what about turn this on when start kimchi? Cause we just need to enable this for the first time. I'm okay with that too, but I figured setting it only if it'll be used made more sense. Is there a reason to set it at startup vs this?
2. For Debian using apparmor, it does not have setsebool, I think this need to be handled too. I was using the package repository logic of "just try to set it." I figured there were too many "what ifs" to check and went with a simple approach. Is that going to cause issues? Is there an equivalent to virt_use_nfs for Debian? Or will this problem not occur there?
+ if error or returncode: + kimchi_log.error('KCHPOOL0037E') return name
def _clean_scan(self, pool_name):
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
I do need to hard-code this message. If there's nothing else, I'll go ahead and submit a v2 with just that change. On Tue, 2014-04-01 at 16:57 -0300, Rodrigo Trujillo wrote:
On 04/01/2014 04:45 PM, Rodrigo Trujillo wrote:
On 04/01/2014 11:02 AM, Christy Perez wrote:
On Tue, 2014-04-01 at 14:24 +0800, Royce Lv wrote:
On 2014年03月29日 05:20, 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@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."),
I think that log messages does not need to be translated, so you do not need to add it to i18n. I may be wrong, but, for instance, if someone is using kimchi in chinese, then the log entry will be in chinese. The Kimchi server might be in another place, where the admin does not necessarily understand Chinese.
Can someone confirm this ? Please
I confirmed with Aline and, actually, the log functions do not translate "error code" in messages.... so you would see "KCHPOOL0037E" in the log. So, you must hard code the log message.
"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'])
1. what about turn this on when start kimchi? Cause we just need to enable this for the first time. I'm okay with that too, but I figured setting it only if it'll be used made more sense. Is there a reason to set it at startup vs this?
2. For Debian using apparmor, it does not have setsebool, I think this need to be handled too. I was using the package repository logic of "just try to set it." I figured there were too many "what ifs" to check and went with a simple approach. Is that going to cause issues? Is there an equivalent to virt_use_nfs for Debian? Or will this problem not occur there?
+ if error or returncode: + kimchi_log.error('KCHPOOL0037E') return name
def _clean_scan(self, pool_name):
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Regards, - Christy
On 2014年04月01日 22:02, Christy Perez wrote:
>
>
> On Tue, 2014-04-01 at 14:24 +0800, Royce Lv wrote:
>> On 2014年03月29日 05:20, 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@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."),
>>>
>>> "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'])
>> 1. what about turn this on when start kimchi? Cause we just need to
>> enable this for the first time.
> I'm okay with that too, but I figured setting it only if it'll be used
> made more sense. Is there a reason to set it at startup vs this?
>
>> 2. For Debian using apparmor, it does not have setsebool, I think this
>> need to be handled too.
> I was using the package repository logic of "just try to set it." I
> figured there were too many "what ifs" to check and went with a simple
> approach. Is that going to cause issues? Is there an equivalent to
> virt_use_nfs for Debian? Or will this problem not occur there?
I don't know there is an alternative for debian, but instead of on when
start kimchi, I think we can toggle this when installation, so that it
can distinguish distribution and do just once.
>
>>> + if error or returncode:
>>> + kimchi_log.error('KCHPOOL0037E')
>>> return name
>>>
>>> def _clean_scan(self, pool_name):
>
On 04/01/2014 03:24 AM, Royce Lv wrote:
> On 2014年03月29日 05:20, 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@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."),
>>
>> "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'])
> 1. what about turn this on when start kimchi? Cause we just need to
> enable this for the first time.
I don't think it is good.
If we modify it only on Kimchi startup and user or other application
revert our changes - the user will not be able to create the NFS pool
> 2. For Debian using apparmor, it does not have setsebool, I think this
> need to be handled too.
>> + if error or returncode:
>> + kimchi_log.error('KCHPOOL0037E')
>> return name
>>
>> def _clean_scan(self, pool_name):
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 2014年04月03日 03:40, Aline Manera wrote:
> On 04/01/2014 03:24 AM, Royce Lv wrote:
>> On 2014年03月29日 05:20, 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@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."),
>>>
>>> "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'])
>> 1. what about turn this on when start kimchi? Cause we just need to
>> enable this for the first time.
>
> I don't think it is good.
> If we modify it only on Kimchi startup and user or other application
> revert our changes - the user will not be able to create the NFS pool
Well, then what if other users change the firewall configuration? We
still cannot access kimchi through browser. And I can't find a reason
why user will intensionally toggle this off.
I can accept this if you think this is safer, still we need to handle
versions despite fedora/rhel.
>
>> 2. For Debian using apparmor, it does not have setsebool, I think
>> this need to be handled too.
>>> + if error or returncode:
>>> + kimchi_log.error('KCHPOOL0037E')
>>> return name
>>>
>>> def _clean_scan(self, pool_name):
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
On Thu, 2014-04-03 at 10:22 +0800, Royce Lv wrote:
> On 2014年04月03日 03:40, Aline Manera wrote:
> > On 04/01/2014 03:24 AM, Royce Lv wrote:
> >> On 2014年03月29日 05:20, 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@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."),
> >>>
> >>> "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'])
> >> 1. what about turn this on when start kimchi? Cause we just need to
> >> enable this for the first time.
> >
> > I don't think it is good.
> > If we modify it only on Kimchi startup and user or other application
> > revert our changes - the user will not be able to create the NFS pool
> Well, then what if other users change the firewall configuration? We
> still cannot access kimchi through browser. And I can't find a reason
> why user will intensionally toggle this off.
> I can accept this if you think this is safer, still we need to handle
> versions despite fedora/rhel.
> >
> >> 2. For Debian using apparmor, it does not have setsebool, I think
> >> this need to be handled too.
If a debian distro is using AppArmor, it won't be an issue like it is
with SELinux. AppArmor is FS agnostic (see
http://www.ibm.com/developerworks/security/library/l-selinux/index.html). So, I think that we can safely catch an exception here if there's no SELinux. If AppArmor is enabled instead of SELinux we don't need to do anything NFS-specific for libvirt.
Does that address your concern?
> >>> + if error or returncode:
> >>> + kimchi_log.error('KCHPOOL0037E')
> >>> return name
> >>>
> >>> def _clean_scan(self, pool_name):
> >>
> >> _______________________________________________
> >> Kimchi-devel mailing list
> >> Kimchi-devel@ovirt.org
> >> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
> >
>
On 2014年04月04日 06:18, Christy Perez wrote:
>
>
> On Thu, 2014-04-03 at 10:22 +0800, Royce Lv wrote:
>> On 2014年04月03日 03:40, Aline Manera wrote:
>>> On 04/01/2014 03:24 AM, Royce Lv wrote:
>>>> On 2014年03月29日 05:20, 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@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."),
>>>>>
>>>>> "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'])
>>>> 1. what about turn this on when start kimchi? Cause we just need to
>>>> enable this for the first time.
>>> I don't think it is good.
>>> If we modify it only on Kimchi startup and user or other application
>>> revert our changes - the user will not be able to create the NFS pool
>> Well, then what if other users change the firewall configuration? We
>> still cannot access kimchi through browser. And I can't find a reason
>> why user will intensionally toggle this off.
>> I can accept this if you think this is safer, still we need to handle
>> versions despite fedora/rhel.
>>>> 2. For Debian using apparmor, it does not have setsebool, I think
>>>> this need to be handled too.
> If a debian distro is using AppArmor, it won't be an issue like it is
> with SELinux. AppArmor is FS agnostic (see
> http://www.ibm.com/developerworks/security/library/l-selinux/index.html). So, I think that we can safely catch an exception here if there's no SELinux. If AppArmor is enabled instead of SELinux we don't need to do anything NFS-specific for libvirt.
>
> Does that address your concern?
Then ACK
>
>>>>> + if error or returncode:
>>>>> + kimchi_log.error('KCHPOOL0037E')
>>>>> return name
>>>>>
>>>>> def _clean_scan(self, pool_name):
>>>> _______________________________________________
>>>> Kimchi-devel mailing list
>>>> Kimchi-devel@ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
After all the discussion, I think we've decided this patch is okay as-is? 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@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."),
"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') return name
def _clean_scan(self, pool_name):
Regards, - Christy
participants (5)
-
Aline Manera -
Christy Perez -
Paulo Ricardo Paz Vital -
Rodrigo Trujillo -
Royce Lv