[PATCH V3 0/5] Fix 'disk full' issue
V3: - Fix issues with tests - Rebase with 1.2 V2: Address Aline's comments: - Change error message - Aggregate 'if's V1: If the disk where objectstore (Kimchi database) is full, then a lot of errors will be raised without any special treatment. This can lead the system to an unexpected state. This patchset modifies kimchi in order to give the right treatment to exceptions, showing the error to the user or hidding when possible. Rodrigo Trujillo (5): Fix 'disk full' issue: Change objectstore exception handling Fix 'disk full' issue: Fix Templates db store/delete error handling Fix 'disk full' issue: Fix storage volume error handling Fix 'disk full' issue: Fix storagepool and asynctasks error handling Fix 'disk full' issue: Fix vms/screenshot db store/delete error handling src/kimchi/asynctask.py | 7 ++-- src/kimchi/i18n.py | 5 +++ src/kimchi/model/storagepools.py | 8 +++-- src/kimchi/model/storagevolumes.py | 49 ++++++++++++++++---------- src/kimchi/model/templates.py | 27 +++++++++++---- src/kimchi/model/vms.py | 70 +++++++++++++++++++++++++++----------- src/kimchi/objectstore.py | 7 ++++ 7 files changed, 123 insertions(+), 50 deletions(-) -- 1.8.5.3
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@linux.vnet.ibm.com> --- src/kimchi/objectstore.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/kimchi/objectstore.py b/src/kimchi/objectstore.py index d960ca9..b1c1bdd 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,8 @@ class ObjectStore(object): def __exit__(self, type, value, tb): self._lock.release() + if type is not None and issubclass(type, sqlite3.DatabaseError): + # Logs the error and return False, which makes __exit__ raise + # exception again + kimchi_log.error(traceback.format_exc()) + return False -- 1.8.5.3
On 2014年03月29日 00:40, 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@linux.vnet.ibm.com> --- src/kimchi/objectstore.py | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/kimchi/objectstore.py b/src/kimchi/objectstore.py index d960ca9..b1c1bdd 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,8 @@ class ObjectStore(object):
def __exit__(self, type, value, tb): self._lock.release() + if type is not None and issubclass(type, sqlite3.DatabaseError): + # Logs the error and return False, which makes __exit__ raise + # exception again + kimchi_log.error(traceback.format_exc()) + return False I appreciate the idea of this patch. Howerver, I wrote a demo and it seems if I don't explicitly return True, it raises error as its default behavior And as __exit__ accepted tb as its param, instead of format a traceback for ourselves, we need to use the passed in tb. And I'm not sure why we just raise DBerror?
On 04/01/2014 04:04 AM, Royce Lv 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@linux.vnet.ibm.com> --- src/kimchi/objectstore.py | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/kimchi/objectstore.py b/src/kimchi/objectstore.py index d960ca9..b1c1bdd 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,8 @@ class ObjectStore(object):
def __exit__(self, type, value, tb): self._lock.release() + if type is not None and issubclass(type, sqlite3.DatabaseError): + # Logs the error and return False, which makes __exit__ raise + # exception again + kimchi_log.error(traceback.format_exc()) + return False I appreciate the idea of this patch. Howerver, I wrote a demo and it seems if I don't explicitly return True, it raises error as its default behavior And as __exit__ accepted tb as its param, instead of format a
On 2014年03月29日 00:40, Rodrigo Trujillo wrote: traceback for ourselves, we need to use the passed in tb. And I'm not sure why we just raise DBerror?
Hi Royce, yeap, the __exit__ is a function of "with" command that will always be called in the end of "with" command, or when an error happens. If "type, value, tb" are None, means everything went ok in "with". Otherwise, __exit__ is going to re-raise the exception received in function end or re-raise when its code return a False. My idea here is: the __exit__ of objectstore should log only database errors... so, I check for this type of exception, log and make sure the exception is re-raised (the "return False" is kind of redundant, but its ok). Any other problem/exception generated by "with" is going to re-raised without treatment. Python internally checks if "type, value and tb" are None, if not, it re-raises in the end... so, for instance, "NotFoundError" exception (raised when an item is not found in the database) are always returned to "with" ... which is going to deal with it. Let me know if its more clear now.
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 2014年04月01日 20:47, Rodrigo Trujillo wrote:
On 04/01/2014 04:04 AM, Royce Lv 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@linux.vnet.ibm.com> --- src/kimchi/objectstore.py | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/kimchi/objectstore.py b/src/kimchi/objectstore.py index d960ca9..b1c1bdd 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,8 @@ class ObjectStore(object):
def __exit__(self, type, value, tb): self._lock.release() + if type is not None and issubclass(type, sqlite3.DatabaseError): + # Logs the error and return False, which makes __exit__ raise + # exception again + kimchi_log.error(traceback.format_exc()) + return False I appreciate the idea of this patch. Howerver, I wrote a demo and it seems if I don't explicitly return True, it raises error as its default behavior And as __exit__ accepted tb as its param, instead of format a
On 2014年03月29日 00:40, Rodrigo Trujillo wrote: traceback for ourselves, we need to use the passed in tb. And I'm not sure why we just raise DBerror?
Hi Royce,
yeap, the __exit__ is a function of "with" command that will always be called in the end of "with" command, or when an error happens. If "type, value, tb" are None, means everything went ok in "with". Otherwise, __exit__ is going to re-raise the exception received in function end or re-raise when its code return a False. My idea here is: the __exit__ of objectstore should log only database errors... so, I check for this type of exception, log and make sure the exception is re-raised (the "return False" is kind of redundant, but its ok). Any other problem/exception generated by "with" is going to re-raised without treatment. Python internally checks if "type, value and tb" are None, if not, it re-raises in the end... so, for instance, "NotFoundError" exception (raised when an item is not found in the database) are always returned to "with" ... which is going to deal with it.
Let me know if its more clear now. So idea is to distinguish DBError and normal error, just log dberror, and raise both.I think it's reasonable. I mean, the 'tb' argument passed to 'exit' function is traceback object when exception raised,this helps us locate error point, we use that instead of format an excecution stack on the exit point. Does that make sense?
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 04/02/2014 03:33 AM, Royce Lv wrote:
On 04/01/2014 04:04 AM, Royce Lv 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@linux.vnet.ibm.com> --- src/kimchi/objectstore.py | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/kimchi/objectstore.py b/src/kimchi/objectstore.py index d960ca9..b1c1bdd 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,8 @@ class ObjectStore(object):
def __exit__(self, type, value, tb): self._lock.release() + if type is not None and issubclass(type, sqlite3.DatabaseError): + # Logs the error and return False, which makes __exit__ raise + # exception again + kimchi_log.error(traceback.format_exc()) + return False I appreciate the idea of this patch. Howerver, I wrote a demo and it seems if I don't explicitly return True, it raises error as its default behavior And as __exit__ accepted tb as its param, instead of format a
On 2014年03月29日 00:40, Rodrigo Trujillo wrote: traceback for ourselves, we need to use the passed in tb. And I'm not sure why we just raise DBerror?
Hi Royce,
yeap, the __exit__ is a function of "with" command that will always be called in the end of "with" command, or when an error happens. If "type, value, tb" are None, means everything went ok in "with". Otherwise, __exit__ is going to re-raise the exception received in function end or re-raise when its code return a False. My idea here is: the __exit__ of objectstore should log only database errors... so, I check for this type of exception, log and make sure the exception is re-raised (the "return False" is kind of redundant, but its ok). Any other problem/exception generated by "with" is going to re-raised without treatment. Python internally checks if "type, value and tb" are None, if not, it re-raises in the end... so, for instance, "NotFoundError" exception (raised when an item is not found in the database) are always returned to "with" ... which is going to deal with it.
Let me know if its more clear now. So idea is to distinguish DBError and normal error, just log dberror, and raise both.I think it's reasonable. I mean, the 'tb' argument passed to 'exit' function is traceback object when exception raised,this helps us locate error point, we use
On 2014年04月01日 20:47, Rodrigo Trujillo wrote: that instead of format an excecution stack on the exit point. Does that make sense?
No, does not make sense. I am not understanding your point here. Notice. I did not change anything in the __exit__ behaviour, I am just logging db error. What is the problem on this ?
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
If disk is full and user tries to create a new template, Kimchi is going to fail with "server unexpected error". The same problem happens if user tries to delete a template. This patch modifies Templates code in order to handle errors properly. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 2 ++ src/kimchi/model/templates.py | 27 ++++++++++++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index d45f607..b3c3313 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -107,6 +107,8 @@ messages = { "KCHTMPL0017E": _("All networks for the template must be specified in a list."), "KCHTMPL0018E": _("Must specify a volume to a template, when storage pool is iscsi or scsi"), "KCHTMPL0019E": _("The volume: %(volume)s in not in storage pool %(pool)s"), + "KCHTMPL0020E": _("Unable to create template due error: %(err)s"), + "KCHTMPL0021E": _("Unable to delete template due error: %(err)s"), "KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index 0e5c2b2..60f4de5 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -25,6 +25,7 @@ import libvirt from kimchi import xmlutils from kimchi.exception import InvalidOperation, InvalidParameter +from kimchi.exception import NotFoundError, OperationFailed from kimchi.kvmusertests import UserTests from kimchi.utils import pool_name_from_uri from kimchi.utils import probe_file_permission_as_user @@ -73,12 +74,19 @@ class TemplatesModel(object): except Exception: raise InvalidParameter("KCHTMPL0003E", {'network': net_name, 'template': name}) + # Creates the template class with necessary information + # Checkings will be done while creating this class, so any exception + # will be raised here + t = LibvirtVMTemplate(params, scan=True) + + try: + with self.objstore as session: + if name in session.get_list('template'): + raise InvalidOperation("KCHTMPL0001E", {'name': name}) + session.store('template', name, t.info) + except Exception as e: + raise OperationFailed('KCHTMPL0020E', {'err': e.message}) - with self.objstore as session: - if name in session.get_list('template'): - raise InvalidOperation("KCHTMPL0001E", {'name': name}) - t = LibvirtVMTemplate(params, scan=True) - session.store('template', name, t.info) return name def get_list(self): @@ -143,8 +151,13 @@ class TemplateModel(object): return ident def delete(self, name): - with self.objstore as session: - session.delete('template', name) + try: + with self.objstore as session: + session.delete('template', name) + except NotFoundError: + raise + except Exception as e: + raise OperationFailed('KCHTMPL0021E', {'err': e.message}) def update(self, name, params): old_t = self.lookup(name) -- 1.8.5.3
Reviewed-by: Royce Lv<lvroyce@linux.vnet.ibm.com> On 2014年03月29日 00:40, Rodrigo Trujillo wrote:
If disk is full and user tries to create a new template, Kimchi is going to fail with "server unexpected error". The same problem happens if user tries to delete a template. This patch modifies Templates code in order to handle errors properly.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 2 ++ src/kimchi/model/templates.py | 27 ++++++++++++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index d45f607..b3c3313 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -107,6 +107,8 @@ messages = { "KCHTMPL0017E": _("All networks for the template must be specified in a list."), "KCHTMPL0018E": _("Must specify a volume to a template, when storage pool is iscsi or scsi"), "KCHTMPL0019E": _("The volume: %(volume)s in not in storage pool %(pool)s"), + "KCHTMPL0020E": _("Unable to create template due error: %(err)s"), + "KCHTMPL0021E": _("Unable to delete template due error: %(err)s"),
"KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index 0e5c2b2..60f4de5 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -25,6 +25,7 @@ import libvirt
from kimchi import xmlutils from kimchi.exception import InvalidOperation, InvalidParameter +from kimchi.exception import NotFoundError, OperationFailed from kimchi.kvmusertests import UserTests from kimchi.utils import pool_name_from_uri from kimchi.utils import probe_file_permission_as_user @@ -73,12 +74,19 @@ class TemplatesModel(object): except Exception: raise InvalidParameter("KCHTMPL0003E", {'network': net_name, 'template': name}) + # Creates the template class with necessary information + # Checkings will be done while creating this class, so any exception + # will be raised here + t = LibvirtVMTemplate(params, scan=True) + + try: + with self.objstore as session: + if name in session.get_list('template'): + raise InvalidOperation("KCHTMPL0001E", {'name': name}) + session.store('template', name, t.info) + except Exception as e: + raise OperationFailed('KCHTMPL0020E', {'err': e.message})
- with self.objstore as session: - if name in session.get_list('template'): - raise InvalidOperation("KCHTMPL0001E", {'name': name}) - t = LibvirtVMTemplate(params, scan=True) - session.store('template', name, t.info) return name
def get_list(self): @@ -143,8 +151,13 @@ class TemplateModel(object): return ident
def delete(self, name): - with self.objstore as session: - session.delete('template', name) + try: + with self.objstore as session: + session.delete('template', name) + except NotFoundError: + raise + except Exception as e: + raise OperationFailed('KCHTMPL0021E', {'err': e.message})
def update(self, name, params): old_t = self.lookup(name)
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@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/storagevolumes.py | 49 ++++++++++++++++++++++++-------------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index b3c3313..4f11e8a 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..6a91c86 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -79,8 +79,14 @@ 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 + kimchi_log.warning('Unable to store storage volume id in ' + 'objectstore due error: %s', e.message) return name @@ -117,22 +123,29 @@ 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: + # This exception is going to catch errors returned by 'with', + # specially ones generated by 'session.store'. It is outside + # to avoid conflict with the __exit__ function of 'with' + raise OperationFailed('KCHVOL0017E',{'err': e.message}) return ref_cnt -- 1.8.5.3
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@linux.vnet.ibm.com> --- 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 4f11e8a..86ab5d6 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -32,6 +32,7 @@ messages = { "KCHAPI0007E": _("This API only supports JSON"), "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 with deep scan information 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 92b2496..9badb8f 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): -- 1.8.5.3
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@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 70 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index c8dfc90..12e780b 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 @@ -176,6 +177,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 = [] @@ -184,12 +197,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') stream_protocols = self.caps.libvirt_stream_protocols xml = t.to_vm_xml(name, vm_uuid, @@ -350,9 +357,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) @@ -408,9 +419,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): @@ -427,18 +443,32 @@ 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: + # The 'except' outside of 'with' is necessary to catch possible + # exception from '__exit__' when calling 'session.store' + # 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) -- 1.8.5.3
make-check fails /usr/bin/pep8 --filename '*.py,*.py.in' plugins/__init__.py plugins/sample/__init__.py plugins/sample/model.py src/kimchi/asynctask.py src/kimchi/auth.py src/kimchi/cachebust.py src/kimchi/config.py.in src/kimchi/control/*.py src/kimchi/control/vm/*.py src/kimchi/disks.py src/kimchi/distroloader.py src/kimchi/exception.py src/kimchi/featuretests.py src/kimchi/iscsi.py src/kimchi/isoinfo.py src/kimchi/kvmusertests.py src/kimchi/mockmodel.py src/kimchi/model/*.py src/kimchi/osinfo.py src/kimchi/repositories.py src/kimchi/rollbackcontext.py src/kimchi/root.py src/kimchi/server.py src/kimchi/swupdate.py src/kimchi/utils.py tests/test_config.py.in tests/test_mockmodel.py tests/test_model.py tests/test_osinfo.py tests/test_plugin.py tests/test_rest.py tests/test_rollbackcontext.py tests/test_storagepool.py tests/utils.py src/kimchi/model/storagepools.py:168:1: E302 expected 2 blank lines, found 1 src/kimchi/model/storagevolumes.py:143:34: E128 continuation line under-indented for visual indent src/kimchi/model/storagevolumes.py:148:48: E231 missing whitespace after ',' src/kimchi/model/vms.py:190:21: E128 continuation line under-indented for visual indent src/kimchi/model/vms.py:429:17: E128 continuation line under-indented for visual indent src/kimchi/model/vms.py:431:1: E302 expected 2 blank lines, found 1 src/kimchi/model/vms.py:453:17: E128 continuation line under-indented for visual indent src/kimchi/model/vms.py:471:17: E128 continuation line under-indented for visual indent make[2]: *** [check-local] Error 1 make[2]: Leaving directory `/home/alinefm/kimchi' make[1]: *** [check-am] Error 2 make[1]: Leaving directory `/home/alinefm/kimchi' make: *** [check-recursive] Error 1 On 03/28/2014 01:40 PM, Rodrigo Trujillo wrote:
V3: - Fix issues with tests - Rebase with 1.2
V2: Address Aline's comments: - Change error message - Aggregate 'if's
V1: If the disk where objectstore (Kimchi database) is full, then a lot of errors will be raised without any special treatment. This can lead the system to an unexpected state. This patchset modifies kimchi in order to give the right treatment to exceptions, showing the error to the user or hidding when possible.
Rodrigo Trujillo (5): Fix 'disk full' issue: Change objectstore exception handling Fix 'disk full' issue: Fix Templates db store/delete error handling Fix 'disk full' issue: Fix storage volume error handling Fix 'disk full' issue: Fix storagepool and asynctasks error handling Fix 'disk full' issue: Fix vms/screenshot db store/delete error handling
src/kimchi/asynctask.py | 7 ++-- src/kimchi/i18n.py | 5 +++ src/kimchi/model/storagepools.py | 8 +++-- src/kimchi/model/storagevolumes.py | 49 ++++++++++++++++---------- src/kimchi/model/templates.py | 27 +++++++++++---- src/kimchi/model/vms.py | 70 +++++++++++++++++++++++++++----------- src/kimchi/objectstore.py | 7 ++++ 7 files changed, 123 insertions(+), 50 deletions(-)
participants (3)
-
Aline Manera -
Rodrigo Trujillo -
Royce Lv