10:33 a.m.
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
This patch series fixes the bug described in
https://github.com/kimchi-project/kimchi/issues/327. If there
are any NFS pools created and the NFS server is unreachable,
kimchi will hang due to a malfunction of libvirt in this
specific scenario.
The proposed workaround is to avoid any libvirt operation
in the NFS pool that fits this scenario, treating the pool as
'inaccessible' and warning the UI. Trying to activate/
deactivate the nfs pool at this case will take no effect,
warning the user that the NFS server is unreachable.
Daniel Henrique Barboza (3):
Github bug #327: NFS pool workaround: i18n changes
Github bug #327: NFS pool workaround: timeout adjustments
Github bug #327: NFS pool workaround: model changes
src/kimchi/i18n.py | 2 +
src/kimchi/model/libvirtstoragepool.py | 9 ++--
src/kimchi/model/storagepools.py | 75 +++++++++++++++++++++++++++++++++-
src/kimchi/utils.py | 2 +-
4 files changed, 83 insertions(+), 5 deletions(-)
--
1.8.3.1
10:33 a.m.
New subject: [PATCH 1/3] Github bug #327: NFS pool workaround: i18n changes
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
Adding the warning/status messages used by the workaround
Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
---
src/kimchi/i18n.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index fea0184..ca72a80 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -135,6 +135,8 @@ messages = {
"KCHPOOL0029E": _("The parameter disks only can be updated for logical
storage pool."),
"KCHPOOL0030E": _("The SCSI host adapter name must be a
string."),
"KCHPOOL0031E": _("The storage pool kimchi_isos is reserved for
internal use"),
+ "KCHPOOL0032E": _("Unable to activate NFS storage pool %(name)s. NFS
server %(server)s is unreachable."),
+ "KCHPOOL0033E": _("Unable to deactivate NFS storage pool %(name)s. NFS
server %(server)s is unreachable."),
"KCHVOL0001E": _("Storage volume %(name)s already exists"),
"KCHVOL0002E": _("Storage volume %(name)s does not exist in storage
pool %(pool)s"),
--
1.8.3.1
10:33 a.m.
New subject: [PATCH 2/3] Github bug #327: NFS pool workaround: timeout adjustments
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
Adding a smaller timeout in the 'prepare()' method of
libvirtstoragepool ('mount' and 'umount' commands).
Fixing an error in utils/run_command() that prevented the
TimeoutExpired exception to be raised sometimes.
Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
---
src/kimchi/model/libvirtstoragepool.py | 9 ++++++---
src/kimchi/utils.py | 2 +-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/model/libvirtstoragepool.py
b/src/kimchi/model/libvirtstoragepool.py
index dc38d50..e69971b 100644
--- a/src/kimchi/model/libvirtstoragepool.py
+++ b/src/kimchi/model/libvirtstoragepool.py
@@ -94,15 +94,18 @@ class NetfsPoolDef(StoragePoolDef):
export_path, mnt_point]
umount_cmd = ["umount", "-f", export_path]
mounted = False
+ # 2 seconds looks like a reasonable time to wait for a refresh
+ # in the UI and enough time to verify that the NFS server
+ # is down.
+ cmd_timeout = 2
with RollbackContext() as rollback:
rollback.prependDefer(os.rmdir, mnt_point)
try:
- run_command(mount_cmd, 30)
- rollback.prependDefer(run_command, umount_cmd)
+ run_command(mount_cmd, cmd_timeout)
+ rollback.prependDefer(run_command, umount_cmd, cmd_timeout)
except TimeoutExpired:
raise InvalidParameter("KCHPOOL0012E", {'path':
export_path})
-
with open("/proc/mounts", "rb") as f:
rawMounts = f.read()
output_items = ['dev_path', 'mnt_point', 'type']
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
index 6be1c04..d4ab1a1 100644
--- a/src/kimchi/utils.py
+++ b/src/kimchi/utils.py
@@ -177,7 +177,7 @@ def run_command(cmd, timeout=None):
"timeout %s seconds" % timeout)
kimchi_log.error(msg)
- msg_args = {'cmd': cmd, 'seconds': timeout}
+ msg_args = {'cmd': " ".join(cmd), 'seconds':
str(timeout)}
raise TimeoutExpired("KCHUTILS0002E", msg_args)
return out, error, proc.returncode
--
1.8.3.1
10:33 a.m.
New subject: [PATCH 3/3] Github bug #327: NFS pool workaround: model changes
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
Implementing a workaround in '_get_storagepool_vols_num',
'lookup', 'activate' and 'deactivate' methods to avoid
libvirt calls in NFS pools when the corresponding NFS server
is unreachable.
In the 'lookup' method the state of the pool returned in
such scenario is 'inaccessible' to warn the UI that the pool
isn't available for normal operation.
Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
---
src/kimchi/model/storagepools.py | 75 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py
index 011feb0..956ab08 100644
--- a/src/kimchi/model/storagepools.py
+++ b/src/kimchi/model/storagepools.py
@@ -168,6 +168,20 @@ class StoragePoolModel(object):
raise
def _get_storagepool_vols_num(self, pool):
+ #FIXME: nfs workaround - return 0 if 'pool' is a NFS pool
+ # and the corresponding NFS server is not reachable.
+ xml = pool.XMLDesc(0)
+ pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0]
+ if pool_type == 'netfs':
+ source = self._get_storage_source(pool_type, xml)
+ nfs_params = {}
+ nfs_params['name'] = name
+ nfs_params['type'] = pool_type
+ nfs_params['source'] = {'path': source['path'],
+ 'host': source['addr']}
+ if not self._nfs_status_online(pool, nfs_params):
+ return 0
+
try:
if pool.isActive():
pool.refresh(0)
@@ -194,15 +208,42 @@ class StoragePoolModel(object):
return source
+ def _nfs_status_online(self, pool, poolArgs):
+ conn = self.conn.get()
+ poolDef = StoragePoolDef.create(poolArgs)
+ try:
+ poolDef.prepare(conn)
+ return True
+ except Exception as e:
+ return False
+
def lookup(self, name):
pool = self.get_storagepool(name, self.conn)
info = pool.info()
- nr_volumes = self._get_storagepool_vols_num(pool)
autostart = True if pool.autostart() else False
xml = pool.XMLDesc(0)
path = xmlutils.xpath_get_text(xml, "/pool/target/path")[0]
pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0]
source = self._get_storage_source(pool_type, xml)
+ #FIXME: nfs workaround - prevent any libvirt operation
+ # for a nfs if the corresponding NFS server is down.
+ if pool_type == 'netfs':
+ nfs_params = {}
+ nfs_params['name'] = name
+ nfs_params['type'] = pool_type
+ nfs_params['source'] = {'path': source['path'],
+ 'host': source['addr']}
+ if not self._nfs_status_online(pool, nfs_params):
+ kimchi_log.debug("NFS pool %s is offline, reason: NFS "
+ "server %s is unreachable.", name,
+ source['addr'])
+ # Mark state as '4' => inaccessible.
+ info[0] = 4
+ # skip calculating volumes
+ nr_volumes = 0
+ else:
+ nr_volumes = self._get_storagepool_vols_num(pool)
+
res = {'state': POOL_STATE_MAP[info[0]],
'path': path,
'source': source,
@@ -267,6 +308,22 @@ class StoragePoolModel(object):
def activate(self, name):
pool = self.get_storagepool(name, self.conn)
+ #FIXME: nfs workaround - do not activate a NFS pool
+ # if the NFS server is not reachable.
+ xml = pool.XMLDesc(0)
+ pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0]
+ if pool_type == 'netfs':
+ source = self._get_storage_source(pool_type, xml)
+ nfs_params = {}
+ nfs_params['name'] = name
+ nfs_params['type'] = pool_type
+ nfs_params['source'] = {'path': source['path'],
+ 'host': source['addr']}
+ if not self._nfs_status_online(pool, nfs_params):
+ # block the user from activating the pool.
+ raise OperationFailed("KCHPOOL0032E",
+ {'name': name, 'server':
source['addr']})
+ return
try:
pool.create(0)
except libvirt.libvirtError as e:
@@ -275,6 +332,22 @@ class StoragePoolModel(object):
def deactivate(self, name):
pool = self.get_storagepool(name, self.conn)
+ #FIXME: nfs workaround - do not try to deactivate a NFS pool
+ # if the NFS server is not reachable.
+ xml = pool.XMLDesc(0)
+ pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0]
+ if pool_type == 'netfs':
+ source = self._get_storage_source(pool_type, xml)
+ nfs_params = {}
+ nfs_params['name'] = name
+ nfs_params['type'] = pool_type
+ nfs_params['source'] = {'path': source['path'],
+ 'host': source['addr']}
+ if not self._nfs_status_online(pool, nfs_params):
+ # block the user from dactivating the pool.
+ raise OperationFailed("KCHPOOL0033E",
+ {'name': name, 'server':
source['addr']})
+ return
try:
pool.destroy()
except libvirt.libvirtError as e:
--
1.8.3.1
11:26 a.m.
New subject: [PATCH 3/3] Github bug #327: NFS pool workaround: model changes
On 02/25/2014 01:33 PM, Daniel Barboza wrote:
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
Implementing a workaround in '_get_storagepool_vols_num',
'lookup', 'activate' and 'deactivate' methods to avoid
libvirt calls in NFS pools when the corresponding NFS server
is unreachable.
In the 'lookup' method the state of the pool returned in
such scenario is 'inaccessible' to warn the UI that the pool
isn't available for normal operation.
Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
---
src/kimchi/model/storagepools.py | 75 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py
index 011feb0..956ab08 100644
--- a/src/kimchi/model/storagepools.py
+++ b/src/kimchi/model/storagepools.py
@@ -168,6 +168,20 @@ class StoragePoolModel(object):
raise
def _get_storagepool_vols_num(self, pool):
+ #FIXME: nfs workaround - return 0 if 'pool' is a NFS pool
+ # and the corresponding NFS server is not reachable.
+ xml = pool.XMLDesc(0)
+ pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0]
+ if pool_type == 'netfs':
+ source = self._get_storage_source(pool_type, xml)
+ nfs_params = {}
+ nfs_params['name'] = name
+ nfs_params['type'] = pool_type
+ nfs_params['source'] = {'path': source['path'],
+ 'host': source['addr']}
+ if not self._nfs_status_online(pool, nfs_params):
+ return 0
+
This function is only called on lookup() in which you have already added
the workaround.
I think we can leave it just there
try:
if pool.isActive():
pool.refresh(0)
@@ -194,15 +208,42 @@ class StoragePoolModel(object):
return source
+ def _nfs_status_online(self, pool, poolArgs):
+ conn = self.conn.get()
+ poolDef = StoragePoolDef.create(poolArgs)
+ try:
+ poolDef.prepare(conn)
+ return True
+ except Exception as e:
+ return False
+
def lookup(self, name):
pool = self.get_storagepool(name, self.conn)
info = pool.info()
- nr_volumes = self._get_storagepool_vols_num(pool)
autostart = True if pool.autostart() else False
xml = pool.XMLDesc(0)
path = xmlutils.xpath_get_text(xml, "/pool/target/path")[0]
pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0]
source = self._get_storage_source(pool_type, xml)
+ #FIXME: nfs workaround - prevent any libvirt operation
+ # for a nfs if the corresponding NFS server is down.
+ if pool_type == 'netfs':
+ nfs_params = {}
+ nfs_params['name'] = name
+ nfs_params['type'] = pool_type
+ nfs_params['source'] = {'path': source['path'],
+ 'host': source['addr']}
+ if not self._nfs_status_online(pool, nfs_params):
+ kimchi_log.debug("NFS pool %s is offline, reason: NFS "
+ "server %s is unreachable.", name,
+ source['addr'])
+ # Mark state as '4' => inaccessible.
+ info[0] = 4
+ # skip calculating volumes
+ nr_volumes = 0
+ else:
+ nr_volumes = self._get_storagepool_vols_num(pool)
+
res = {'state': POOL_STATE_MAP[info[0]],
'path': path,
'source': source,
@@ -267,6 +308,22 @@ class StoragePoolModel(object):
def activate(self, name):
pool = self.get_storagepool(name, self.conn)
+ #FIXME: nfs workaround - do not activate a NFS pool
+ # if the NFS server is not reachable.
+ xml = pool.XMLDesc(0)
+ pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0]
+ if pool_type == 'netfs':
+ source = self._get_storage_source(pool_type, xml)
+ nfs_params = {}
+ nfs_params['name'] = name
+ nfs_params['type'] = pool_type
+ nfs_params['source'] = {'path': source['path'],
+ 'host': source['addr']}
+ if not self._nfs_status_online(pool, nfs_params):
+ # block the user from activating the pool.
+ raise OperationFailed("KCHPOOL0032E",
+ {'name': name, 'server':
source['addr']})
All the above coding is being used in some places.
What about create a function for it? _check_nfs_server() ?
def _check_nfs_server(self, pool):
....
return True|False
+ return
try:
pool.create(0)
except libvirt.libvirtError as e:
@@ -275,6 +332,22 @@ class StoragePoolModel(object):
def deactivate(self, name):
pool = self.get_storagepool(name, self.conn)
+ #FIXME: nfs workaround - do not try to deactivate a NFS pool
+ # if the NFS server is not reachable.
+ xml = pool.XMLDesc(0)
+ pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0]
+ if pool_type == 'netfs':
+ source = self._get_storage_source(pool_type, xml)
+ nfs_params = {}
+ nfs_params['name'] = name
+ nfs_params['type'] = pool_type
+ nfs_params['source'] = {'path': source['path'],
+ 'host': source['addr']}
+ if not self._nfs_status_online(pool, nfs_params):
+ # block the user from dactivating the pool.
+ raise OperationFailed("KCHPOOL0033E",
+ {'name': name, 'server':
source['addr']})
+ return
try:
pool.destroy()
except libvirt.libvirtError as e:
3952
days inactive
3952
days old
4 comments
2 participants
participants (2)
-
Aline Manera -
Daniel Barboza