[PATCH] [Kimchi] Issue 797: show volumes even if pool is busy
This commit fixes the issue https://github.com/kimchi-project/kimchi/issues/797. It will display all volumes of a pool even if that particular pool is busy. Jose Ricardo Ziviani (1): Ignore pool.refresh if it cannot be called i18n.py | 2 -- model/storagepools.py | 27 +++++++++------------------ model/storagevolumes.py | 8 +++----- 3 files changed, 12 insertions(+), 25 deletions(-) -- 1.9.1
- When creating full allocated images in older filesystems such ext3, Libvirt will do a huge number of IO operations by writing 0's in the whole image file. During this time, the pool where the volume is being allocated cannot be refreshed due to some concurrent issue that Libvirt fixes by raising an exception to any call made to refresh. This commit simply ignores such refresh calls if Libvirt raises exception from it. Thus, the frontend will still be able to list all volumes in the pool, include the volume being created. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- i18n.py | 2 -- model/storagepools.py | 27 +++++++++------------------ model/storagevolumes.py | 8 +++----- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/i18n.py b/i18n.py index cf67085..7f11245 100644 --- a/i18n.py +++ b/i18n.py @@ -187,7 +187,6 @@ messages = { "KCHPOOL0005E": _("Unable to delete active storage pool %(name)s"), "KCHPOOL0006E": _("Unable to list storage pools. Details: %(err)s"), "KCHPOOL0007E": _("Unable to create storage pool %(name)s. Details: %(err)s"), - "KCHPOOL0008E": _("Unable to get number of storage volumes in storage pool %(name)s. Details: %(err)s"), "KCHPOOL0009E": _("Unable to activate storage pool %(name)s. Details: %(err)s"), "KCHPOOL0010E": _("Unable to deactivate storage pool %(name)s. Details: %(err)s"), "KCHPOOL0011E": _("Unable to delete storage pool %(name)s. Details: %(err)s"), @@ -225,7 +224,6 @@ messages = { "KCHVOL0004E": _("Specify %(item)s in order to create storage volume %(volume)s"), "KCHVOL0006E": _("Unable to list storage volumes because storage pool %(pool)s is not active"), "KCHVOL0007E": _("Unable to create storage volume %(name)s in storage pool %(pool)s. Details: %(err)s"), - "KCHVOL0008E": _("Unable to list storage volumes in storage pool %(pool)s. Details: %(err)s"), "KCHVOL0009E": _("Unable to wipe storage volumes %(name)s. Details: %(err)s"), "KCHVOL0010E": _("Unable to delete storage volume %(name)s. Details: %(err)s"), "KCHVOL0011E": _("Unable to resize storage volume %(name)s. Details: %(err)s"), diff --git a/model/storagepools.py b/model/storagepools.py index ddfa7fa..4847c0c 100644 --- a/model/storagepools.py +++ b/model/storagepools.py @@ -283,25 +283,16 @@ class StoragePoolModel(object): raise def _get_storagepool_vols_num(self, pool): - try: - if pool.isActive(): - pool.refresh(0) - return pool.numOfVolumes() - else: - return 0 - except libvirt.libvirtError as e: - # If something (say a busy pool) prevents the refresh, - # throwing an Exception here would prevent all pools from - # displaying information -- so return None for busy - wok_log.error("ERROR: Storage Pool get vol count: %s " - % e.get_error_message()) - wok_log.error("ERROR: Storage Pool get vol count error no: %s " - % e.get_error_code()) + if not pool.isActive(): return 0 - except Exception as e: - raise OperationFailed("KCHPOOL0008E", - {'name': pool.name(), - 'err': e.get_error_message()}) + + try: + pool.refresh(0) + + except: + pass + + return pool.numOfVolumes() def _get_storage_source(self, pool_type, pool_xml): source = {} diff --git a/model/storagevolumes.py b/model/storagevolumes.py index 4e28712..f791d7e 100644 --- a/model/storagevolumes.py +++ b/model/storagevolumes.py @@ -261,11 +261,9 @@ class StorageVolumesModel(object): raise InvalidOperation("KCHVOL0006E", {'pool': pool_name}) try: pool.refresh(0) - return sorted(map(lambda x: x.decode('utf-8'), pool.listVolumes())) - except libvirt.libvirtError as e: - raise OperationFailed("KCHVOL0008E", - {'pool': pool_name, - 'err': e.get_error_message()}) + except libvirt.libvirtError: + pass + return sorted(map(lambda x: x.decode('utf-8'), pool.listVolumes())) class StorageVolumeModel(object): -- 1.9.1
On 16/12/2015 12:45, Jose Ricardo Ziviani wrote:
- When creating full allocated images in older filesystems such ext3, Libvirt will do a huge number of IO operations by writing 0's in the whole image file. During this time, the pool where the volume is being allocated cannot be refreshed due to some concurrent issue that Libvirt fixes by raising an exception to any call made to refresh.
This commit simply ignores such refresh calls if Libvirt raises exception from it. Thus, the frontend will still be able to list all volumes in the pool, include the volume being created.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- i18n.py | 2 -- model/storagepools.py | 27 +++++++++------------------ model/storagevolumes.py | 8 +++----- 3 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/i18n.py b/i18n.py index cf67085..7f11245 100644 --- a/i18n.py +++ b/i18n.py @@ -187,7 +187,6 @@ messages = { "KCHPOOL0005E": _("Unable to delete active storage pool %(name)s"), "KCHPOOL0006E": _("Unable to list storage pools. Details: %(err)s"), "KCHPOOL0007E": _("Unable to create storage pool %(name)s. Details: %(err)s"), - "KCHPOOL0008E": _("Unable to get number of storage volumes in storage pool %(name)s. Details: %(err)s"), "KCHPOOL0009E": _("Unable to activate storage pool %(name)s. Details: %(err)s"), "KCHPOOL0010E": _("Unable to deactivate storage pool %(name)s. Details: %(err)s"), "KCHPOOL0011E": _("Unable to delete storage pool %(name)s. Details: %(err)s"), @@ -225,7 +224,6 @@ messages = { "KCHVOL0004E": _("Specify %(item)s in order to create storage volume %(volume)s"), "KCHVOL0006E": _("Unable to list storage volumes because storage pool %(pool)s is not active"), "KCHVOL0007E": _("Unable to create storage volume %(name)s in storage pool %(pool)s. Details: %(err)s"), - "KCHVOL0008E": _("Unable to list storage volumes in storage pool %(pool)s. Details: %(err)s"), "KCHVOL0009E": _("Unable to wipe storage volumes %(name)s. Details: %(err)s"), "KCHVOL0010E": _("Unable to delete storage volume %(name)s. Details: %(err)s"), "KCHVOL0011E": _("Unable to resize storage volume %(name)s. Details: %(err)s"), diff --git a/model/storagepools.py b/model/storagepools.py index ddfa7fa..4847c0c 100644 --- a/model/storagepools.py +++ b/model/storagepools.py @@ -283,25 +283,16 @@ class StoragePoolModel(object): raise
def _get_storagepool_vols_num(self, pool): - try: - if pool.isActive(): - pool.refresh(0) - return pool.numOfVolumes() - else: - return 0 - except libvirt.libvirtError as e: - # If something (say a busy pool) prevents the refresh, - # throwing an Exception here would prevent all pools from - # displaying information -- so return None for busy - wok_log.error("ERROR: Storage Pool get vol count: %s " - % e.get_error_message()) - wok_log.error("ERROR: Storage Pool get vol count error no: %s " - % e.get_error_code()) + if not pool.isActive(): return 0 - except Exception as e: - raise OperationFailed("KCHPOOL0008E", - {'name': pool.name(), - 'err': e.get_error_message()}) + + try: + pool.refresh(0) +
+ except: + pass
Please, log the exception for debug matters.
+ + return pool.numOfVolumes()
def _get_storage_source(self, pool_type, pool_xml): source = {} diff --git a/model/storagevolumes.py b/model/storagevolumes.py index 4e28712..f791d7e 100644 --- a/model/storagevolumes.py +++ b/model/storagevolumes.py @@ -261,11 +261,9 @@ class StorageVolumesModel(object): raise InvalidOperation("KCHVOL0006E", {'pool': pool_name}) try: pool.refresh(0) - return sorted(map(lambda x: x.decode('utf-8'), pool.listVolumes())) - except libvirt.libvirtError as e: - raise OperationFailed("KCHVOL0008E", - {'pool': pool_name, - 'err': e.get_error_message()})
+ except libvirt.libvirtError: + pass
Same here.
+ return sorted(map(lambda x: x.decode('utf-8'), pool.listVolumes()))
class StorageVolumeModel(object):
Hi Ziviani, I think this patch is good and really fixes the problem. However I have one concern: It will skip all errors that would be returned by Libvirt. Would be good if it was possible to keep the error returns and only pass if we get the "asynchronous jobs running" error. Once the error type will always be libvirt.libvirterror, a solution would be add a checking for the phrase above in the error message, inside the exception and then pass if True. (I know this is not the most beautiful solution) Rodrigo On 12/16/2015 12:45 PM, Jose Ricardo Ziviani wrote:
- When creating full allocated images in older filesystems such ext3, Libvirt will do a huge number of IO operations by writing 0's in the whole image file. During this time, the pool where the volume is being allocated cannot be refreshed due to some concurrent issue that Libvirt fixes by raising an exception to any call made to refresh.
This commit simply ignores such refresh calls if Libvirt raises exception from it. Thus, the frontend will still be able to list all volumes in the pool, include the volume being created.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- i18n.py | 2 -- model/storagepools.py | 27 +++++++++------------------ model/storagevolumes.py | 8 +++----- 3 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/i18n.py b/i18n.py index cf67085..7f11245 100644 --- a/i18n.py +++ b/i18n.py @@ -187,7 +187,6 @@ messages = { "KCHPOOL0005E": _("Unable to delete active storage pool %(name)s"), "KCHPOOL0006E": _("Unable to list storage pools. Details: %(err)s"), "KCHPOOL0007E": _("Unable to create storage pool %(name)s. Details: %(err)s"), - "KCHPOOL0008E": _("Unable to get number of storage volumes in storage pool %(name)s. Details: %(err)s"), "KCHPOOL0009E": _("Unable to activate storage pool %(name)s. Details: %(err)s"), "KCHPOOL0010E": _("Unable to deactivate storage pool %(name)s. Details: %(err)s"), "KCHPOOL0011E": _("Unable to delete storage pool %(name)s. Details: %(err)s"), @@ -225,7 +224,6 @@ messages = { "KCHVOL0004E": _("Specify %(item)s in order to create storage volume %(volume)s"), "KCHVOL0006E": _("Unable to list storage volumes because storage pool %(pool)s is not active"), "KCHVOL0007E": _("Unable to create storage volume %(name)s in storage pool %(pool)s. Details: %(err)s"), - "KCHVOL0008E": _("Unable to list storage volumes in storage pool %(pool)s. Details: %(err)s"), "KCHVOL0009E": _("Unable to wipe storage volumes %(name)s. Details: %(err)s"), "KCHVOL0010E": _("Unable to delete storage volume %(name)s. Details: %(err)s"), "KCHVOL0011E": _("Unable to resize storage volume %(name)s. Details: %(err)s"), diff --git a/model/storagepools.py b/model/storagepools.py index ddfa7fa..4847c0c 100644 --- a/model/storagepools.py +++ b/model/storagepools.py @@ -283,25 +283,16 @@ class StoragePoolModel(object): raise
def _get_storagepool_vols_num(self, pool): - try: - if pool.isActive(): - pool.refresh(0) - return pool.numOfVolumes() - else: - return 0 - except libvirt.libvirtError as e: - # If something (say a busy pool) prevents the refresh, - # throwing an Exception here would prevent all pools from - # displaying information -- so return None for busy - wok_log.error("ERROR: Storage Pool get vol count: %s " - % e.get_error_message()) - wok_log.error("ERROR: Storage Pool get vol count error no: %s " - % e.get_error_code()) + if not pool.isActive(): return 0 - except Exception as e: - raise OperationFailed("KCHPOOL0008E", - {'name': pool.name(), - 'err': e.get_error_message()}) + + try: + pool.refresh(0) + + except: + pass + + return pool.numOfVolumes()
def _get_storage_source(self, pool_type, pool_xml): source = {} diff --git a/model/storagevolumes.py b/model/storagevolumes.py index 4e28712..f791d7e 100644 --- a/model/storagevolumes.py +++ b/model/storagevolumes.py @@ -261,11 +261,9 @@ class StorageVolumesModel(object): raise InvalidOperation("KCHVOL0006E", {'pool': pool_name}) try: pool.refresh(0) - return sorted(map(lambda x: x.decode('utf-8'), pool.listVolumes())) - except libvirt.libvirtError as e: - raise OperationFailed("KCHVOL0008E", - {'pool': pool_name, - 'err': e.get_error_message()}) + except libvirt.libvirtError: + pass + return sorted(map(lambda x: x.decode('utf-8'), pool.listVolumes()))
class StorageVolumeModel(object):
Hello Rodrigo, If you agree I'll log the error in this particular case. That's because there is no much failure points in the refresh(), looking at the source code I see that it throws if the pool is not active (which we're covered) and that race condition. On 16-12-2015 17:58, Rodrigo Trujillo wrote:
Hi Ziviani,
I think this patch is good and really fixes the problem. However I have one concern: It will skip all errors that would be returned by Libvirt. Would be good if it was possible to keep the error returns and only pass if we get the "asynchronous jobs running" error. Once the error type will always be libvirt.libvirterror, a solution would be add a checking for the phrase above in the error message, inside the exception and then pass if True. (I know this is not the most beautiful solution)
Rodrigo
On 12/16/2015 12:45 PM, Jose Ricardo Ziviani wrote:
- When creating full allocated images in older filesystems such ext3, Libvirt will do a huge number of IO operations by writing 0's in the whole image file. During this time, the pool where the volume is being allocated cannot be refreshed due to some concurrent issue that Libvirt fixes by raising an exception to any call made to refresh.
This commit simply ignores such refresh calls if Libvirt raises exception from it. Thus, the frontend will still be able to list all volumes in the pool, include the volume being created.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- i18n.py | 2 -- model/storagepools.py | 27 +++++++++------------------ model/storagevolumes.py | 8 +++----- 3 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/i18n.py b/i18n.py index cf67085..7f11245 100644 --- a/i18n.py +++ b/i18n.py @@ -187,7 +187,6 @@ messages = { "KCHPOOL0005E": _("Unable to delete active storage pool %(name)s"), "KCHPOOL0006E": _("Unable to list storage pools. Details: %(err)s"), "KCHPOOL0007E": _("Unable to create storage pool %(name)s. Details: %(err)s"), - "KCHPOOL0008E": _("Unable to get number of storage volumes in storage pool %(name)s. Details: %(err)s"), "KCHPOOL0009E": _("Unable to activate storage pool %(name)s. Details: %(err)s"), "KCHPOOL0010E": _("Unable to deactivate storage pool %(name)s. Details: %(err)s"), "KCHPOOL0011E": _("Unable to delete storage pool %(name)s. Details: %(err)s"), @@ -225,7 +224,6 @@ messages = { "KCHVOL0004E": _("Specify %(item)s in order to create storage volume %(volume)s"), "KCHVOL0006E": _("Unable to list storage volumes because storage pool %(pool)s is not active"), "KCHVOL0007E": _("Unable to create storage volume %(name)s in storage pool %(pool)s. Details: %(err)s"), - "KCHVOL0008E": _("Unable to list storage volumes in storage pool %(pool)s. Details: %(err)s"), "KCHVOL0009E": _("Unable to wipe storage volumes %(name)s. Details: %(err)s"), "KCHVOL0010E": _("Unable to delete storage volume %(name)s. Details: %(err)s"), "KCHVOL0011E": _("Unable to resize storage volume %(name)s. Details: %(err)s"), diff --git a/model/storagepools.py b/model/storagepools.py index ddfa7fa..4847c0c 100644 --- a/model/storagepools.py +++ b/model/storagepools.py @@ -283,25 +283,16 @@ class StoragePoolModel(object): raise
def _get_storagepool_vols_num(self, pool): - try: - if pool.isActive(): - pool.refresh(0) - return pool.numOfVolumes() - else: - return 0 - except libvirt.libvirtError as e: - # If something (say a busy pool) prevents the refresh, - # throwing an Exception here would prevent all pools from - # displaying information -- so return None for busy - wok_log.error("ERROR: Storage Pool get vol count: %s " - % e.get_error_message()) - wok_log.error("ERROR: Storage Pool get vol count error no: %s " - % e.get_error_code()) + if not pool.isActive(): return 0 - except Exception as e: - raise OperationFailed("KCHPOOL0008E", - {'name': pool.name(), - 'err': e.get_error_message()}) + + try: + pool.refresh(0) + + except: + pass + + return pool.numOfVolumes()
def _get_storage_source(self, pool_type, pool_xml): source = {} diff --git a/model/storagevolumes.py b/model/storagevolumes.py index 4e28712..f791d7e 100644 --- a/model/storagevolumes.py +++ b/model/storagevolumes.py @@ -261,11 +261,9 @@ class StorageVolumesModel(object): raise InvalidOperation("KCHVOL0006E", {'pool': pool_name}) try: pool.refresh(0) - return sorted(map(lambda x: x.decode('utf-8'), pool.listVolumes())) - except libvirt.libvirtError as e: - raise OperationFailed("KCHVOL0008E", - {'pool': pool_name, - 'err': e.get_error_message()}) + except libvirt.libvirtError: + pass + return sorted(map(lambda x: x.decode('utf-8'), pool.listVolumes()))
class StorageVolumeModel(object):
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Jose Ricardo Ziviani ----------------------------- Software Engineer Linux Technology Center - IBM
Applied. Thanks. Regards, Aline Manera
participants (3)
-
Aline Manera -
Jose Ricardo Ziviani -
Rodrigo Trujillo