On 03/24/2014 05:53 PM, Rodrigo Trujillo wrote: > This patch changes __exit__ function of objectstore in order to log the > error information. It also changes the return to make python raise the > exception again, which will be caught by the functions that are using > the objectstore. > > Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; > --- > src/kimchi/objectstore.py | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/src/kimchi/objectstore.py b/src/kimchi/objectstore.py > index d960ca9..e9baad5 100644 > --- a/src/kimchi/objectstore.py > +++ b/src/kimchi/objectstore.py > @@ -19,6 +19,7 @@ > import json > import sqlite3 > import threading > +import traceback > > > try: > @@ -29,6 +30,7 @@ except ImportError: > > from kimchi import config > from kimchi.exception import NotFoundError > +from kimchi.utils import kimchi_log > > > class ObjectStoreSession(object): > @@ -116,3 +118,9 @@ class ObjectStore(object): > > def __exit__(self, type, value, tb): > self._lock.release() > + if type is not None: > + if issubclass(type, sqlite3.DatabaseError): > + # Logs the error and return False, which makes > __exit__ raise > + # exception again > + kimchi_log.error(traceback.format_exc()) > + return False You can avoid multiples indentation levels: if type is not None and issubclass(type, sqlite3.DatabaseError): ...

_______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

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&gt; > --- > 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.

> > 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()

> > return ref_cnt > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

This patch fixes the error handling when storagepool.py and asynctask.py try to update the objectstore (database) with task and scanning information. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; --- src/kimchi/asynctask.py | 7 +++++-- src/kimchi/i18n.py | 2 ++ src/kimchi/model/storagepools.py | 8 +++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/kimchi/asynctask.py b/src/kimchi/asynctask.py index 8f0d96c..33e9ba1 100644 --- a/src/kimchi/asynctask.py +++ b/src/kimchi/asynctask.py @@ -59,8 +59,11 @@ class AsyncTask(object): obj = {} for attr in ('id', 'target_uri', 'message', 'status'): obj[attr] = getattr(self, attr) - with self.objstore as session: - session.store('task', self.id, obj) + try: + with self.objstore as session: + session.store('task', self.id, obj) + except Exception as e: + raise OperationFailed('KCHASYNC0002E', {'err': e.message}) def _run_helper(self, opaque, cb): try: diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 81a74c4..ab01160 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -32,6 +32,7 @@ messages = { "KCHAPI0007E": _("This API only supports"), "KCHASYNC0001E": _("Datastore is not initiated in the model object."), + "KCHASYNC0002E": _("Unable to start task due error: %(err)s"), "KCHAUTH0001E": _("Authentication failed for user '%(userid)s'. [Error code: %(code)s]"), "KCHAUTH0002E": _("You are not authorized to access Kimchi"), @@ -146,6 +147,7 @@ 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 update database due error: %(err)s"),

"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 fea19f6..a3a9e25 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -158,10 +158,12 @@ class StoragePoolsModel(object): task_id = add_task('', self.scanner.start_scan, self.objstore, scan_params) # Record scanning-task/storagepool mapping for future querying - with self.objstore as session: + try: + with self.objstore as session: session.store('scanning', params['name'], task_id) - return task_id - + return task_id + except Exception as e: + raise OperationFailed('KCHPOOL0037E', {'err': e.message}) class StoragePoolModel(object): def __init__(self, **kargs):

On 03/24/2014 05:53 PM, Rodrigo Trujillo wrote: > When creating, running or deleting a vm, it uses the database to store > vm and screenshot information. If the disk is full, Kimchi will raise > errors constantly and UI will be useless. This patch fixes this problem, > omitting from UI and logging in the backend. So, user is still able to > create, start and stop a vm. > > Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; > --- > src/kimchi/model/vms.py | 68 > ++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 48 insertions(+), 20 deletions(-) > > diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py > index d29b811..05fd67e 100644 > --- a/src/kimchi/model/vms.py > +++ b/src/kimchi/model/vms.py > @@ -34,6 +34,7 @@ from kimchi.model.config import CapabilitiesModel > from kimchi.model.templates import TemplateModel > from kimchi.model.utils import get_vm_name > from kimchi.screenshot import VMScreenshot > +from kimchi.utils import kimchi_log > from kimchi.utils import run_setfacl_set_attr, template_name_from_uri > > > @@ -181,6 +182,18 @@ class VMsModel(object): > > t.validate() > > + # Store the icon for displaying later > + icon = t.info.get('icon') > + if icon: > + try: > + with self.objstore as session: > + session.store('vm', vm_uuid, {'icon': icon}) > + except Exception as e: > + # It is possible to continue Kimchi executions > without store > + # vm icon info > + kimchi_log.error('Error trying to update database > with guest ' > + 'icon information due error: %s', e.message) > + > # If storagepool is SCSI, volumes will be LUNs and must be > passed by > # the user from UI or manually. > vol_list = [] > @@ -196,12 +209,6 @@ class VMsModel(object): > else: > vol_list = t.fork_vm_storage(vm_uuid) > > - # Store the icon for displaying later > - icon = t.info.get('icon') > - if icon: > - with self.objstore as session: > - session.store('vm', vm_uuid, {'icon': icon}) > - > graphics = params.get('graphics') > xml = t.to_vm_xml(name, vm_uuid, > libvirt_stream_protocols=self.caps.libvirt_stream_protocols, > @@ -361,9 +368,13 @@ class VMModel(object): > pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] > if pool_type not in READONLY_POOL_TYPE: > vol.delete(0) > - > - with self.objstore as session: > - session.delete('vm', dom.UUIDString(), ignore_missing=True) > + try: > + with self.objstore as session: > + session.delete('vm', dom.UUIDString(), > ignore_missing=True) > + except Exception as e: > + # It is possible to delete vm without delete its > database info > + kimchi_log.error('Error deleting vm information from > database: ' > + '%s', e.message) > > vnc.remove_proxy_token(name) > > @@ -419,9 +430,14 @@ class VMModel(object): > screenshot = VMScreenshotModel.get_screenshot(vm_uuid, > self.objstore, > self.conn) > screenshot.delete() > - with self.objstore as session: > - session.delete('screenshot', vm_uuid) > - > + try: > + with self.objstore as session: > + session.delete('screenshot', vm_uuid) > + except Exception as e: > + # It is possible to continue Kimchi executions without > delete > + # screenshots > + kimchi_log.error('Error trying to delete vm screenshot > from ' > + 'database due error: %s', e.message) > > class VMScreenshotModel(object): > def __init__(self, **kargs): > @@ -438,18 +454,30 @@ class VMScreenshotModel(object): > screenshot = self.get_screenshot(vm_uuid, self.objstore, > self.conn) > img_path = screenshot.lookup() > # screenshot info changed after scratch generation > - with self.objstore as session: > - session.store('screenshot', vm_uuid, screenshot.info) > + try: > + with self.objstore as session: > + session.store('screenshot', vm_uuid, screenshot.info) > + except Exception as e: > + # It is possible to continue Kimchi executions without > store > + # screenshots > + kimchi_log.error('Error trying to update database with > guest ' > + 'screenshot information due error: %s', e.message) > return img_path > > @staticmethod > def get_screenshot(vm_uuid, objstore, conn): > - with objstore as session: > - try: > - params = session.get('screenshot', vm_uuid) > - except NotFoundError: > - params = {'uuid': vm_uuid} > - session.store('screenshot', vm_uuid, params) > + try: > + with objstore as session: > + try: > + params = session.get('screenshot', vm_uuid) > + except NotFoundError: > + params = {'uuid': vm_uuid} > + session.store('screenshot', vm_uuid, params) > + except Exception as e: > + # It is possible to continue Kimchi vm executions without > + # screenshots > + kimchi_log.error('Error trying to update database with > guest ' > + 'screenshot information due error: %s', e.message) > return LibvirtVMScreenshot(params, conn) > Same I commented in a previous patch. Use the existent try/except block and only add a new "except" statement for your proposals.

_______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel