[Kimchi-devel] [PATCH] [Kimchi] Ignore pool.refresh if it cannot be called

Jose Ricardo Ziviani joserz at linux.vnet.ibm.com
Thu Dec 17 12:38:21 UTC 2015


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 at 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 at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>

-- 
Jose Ricardo Ziviani
-----------------------------
Software Engineer
Linux Technology Center - IBM




More information about the Kimchi-devel mailing list