3:06 p.m.
On 03/25/2014 03:40 PM, Aline Manera wrote:
On 03/24/2014 05:53 PM, Rodrigo Trujillo wrote:
> This patch fixes the error handling when storagevolume.py tries to
> update the database with reference count information.
>
> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com>
> ---
> src/kimchi/i18n.py | 1 +
> src/kimchi/model/storagevolumes.py | 45
> +++++++++++++++++++++++---------------
> 2 files changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index 231ea25..81a74c4 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -163,6 +163,7 @@ messages = {
> "KCHVOL0014E": _("Storage volume allocation must be an integer
> number"),
> "KCHVOL0015E": _("Storage volume format not supported"),
> "KCHVOL0016E": _("Storage volume requires a volume name"),
> + "KCHVOL0017E": _("Unable to update database with storage volume
> information due error: %(err)s"),
>
> "KCHIFACE0001E": _("Interface %(name)s does not exist"),
>
> diff --git a/src/kimchi/model/storagevolumes.py
> b/src/kimchi/model/storagevolumes.py
> index 1253823..d4fbb0e 100644
> --- a/src/kimchi/model/storagevolumes.py
> +++ b/src/kimchi/model/storagevolumes.py
> @@ -79,8 +79,13 @@ class StorageVolumesModel(object):
> {'name': name, 'pool': pool,
> 'err': e.get_error_message()})
>
> - with self.objstore as session:
> - session.store('storagevolume', vol_id, {'ref_cnt': 0})
> + try:
> + with self.objstore as session:
> + session.store('storagevolume', vol_id, {'ref_cnt':
0})
> + except Exception as e:
> + # If the storage volume was created flawlessly, then
> lets hide this
> + # error to avoid more error in the VM creation process
> + pass
Log the error to help on debug when needed.
I did not log here because the
'__exit__' from 'with' would do it. But
I can include a message in storage volume context, makes sense. Thanks
>
> return name
>
> @@ -117,22 +122,26 @@ class StorageVolumeModel(object):
>
> def _get_ref_cnt(self, pool, name, path):
> vol_id = '%s:%s' % (pool, name)
> - with self.objstore as session:
> - try:
> - ref_cnt = session.get('storagevolume',
> vol_id)['ref_cnt']
> - except NotFoundError:
> - # Fix storage volume created outside kimchi scope
> - ref_cnt = 0
> - args = {'conn': self.conn, 'objstore':
self.objstore}
> - # try to find this volume in exsisted vm
> - vms = VMsModel.get_vms(self.conn)
> - for vm in vms:
> - storages = VMStoragesModel(**args).get_list(vm)
> - for disk in storages:
> - d_info = VMStorageModel(**args).lookup(vm,
> disk)
> - if path == d_info['path']:
> - ref_cnt = ref_cnt + 1
> - session.store('storagevolume', vol_id, {'ref_cnt':
> ref_cnt})
> + try:
> + with self.objstore as session:
> + try:
> + ref_cnt = session.get('storagevolume',
> vol_id)['ref_cnt']
> + except NotFoundError:
> + # Fix storage volume created outside kimchi scope
> + ref_cnt = 0
> + args = {'conn': self.conn, 'objstore':
> self.objstore}
> + # try to find this volume in exsisted vm
> + vms = VMsModel.get_vms(self.conn)
> + for vm in vms:
> + storages = VMStoragesModel(**args).get_list(vm)
> + for disk in storages:
> + d_info =
> VMStorageModel(**args).lookup(vm, disk)
> + if path == d_info['path']:
> + ref_cnt = ref_cnt + 1
> + session.store('storagevolume', vol_id,
> + {'ref_cnt': ref_cnt})
> + except Exception as e:
> + raise OperationFailed('KCHVOL0017E',{'err': e.message})
Use existent try statement inside the with block:
with self.objstore as session:
try:
...
except NotFoundError:
...
except Exception:
raise OperationFailed()
I know it is ugly, but I had to do this way, I mean, include the 'with'
inside a new 'try'.
In order to remove the outside 'try', I would have to do something like:
with:
...
try:
...
except:
....
try:
session.store('storagevolume', vol_id, {'ref_cnt': ref_cnt})
except:
<print/log db error> --> Because the __exit__ is going
to receive the exception below and lose the sqlite3 error
raise OperationFailed()
This is feasible. Did not implement this way to make the code more
standard (I mean, just __exit__ would log the sqlite3 errors)
Do you prefer this new way ? I can change.
Thanks
>
> return ref_cnt
>
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel