3:53 p.m.
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 | 45 +++++++++++++++----------
src/kimchi/model/templates.py | 22 ++++++++----
src/kimchi/model/vms.py | 68 +++++++++++++++++++++++++++-----------
src/kimchi/objectstore.py | 8 +++++
7 files changed, 113 insertions(+), 50 deletions(-)
--
1.8.5.3
3:53 p.m.
New subject: [PATCH 1/5] Fix 'disk full' issue: Change objectstore exception handling
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>
---
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
--
1.8.5.3
3:53 p.m.
New subject: [PATCH 2/5] Fix 'disk full' issue: Fix Templates db store/delete error handling
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(a)linux.vnet.ibm.com>
---
src/kimchi/i18n.py | 2 ++
src/kimchi/model/templates.py | 22 +++++++++++++++-------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index 589c8cb..231ea25 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -107,6 +107,8 @@ messages = {
"KCHTMPL0015E": _("Invalid storage pool URI %(value)s specified for
template"),
"KCHTMPL0016E": _("Specify an ISO image as CDROM to create a
template"),
"KCHTMPL0017E": _("All networks for the template must be specified in
a list."),
+ "KCHTMPL0018E": _("Unable to create template due error:
%(err)s"),
+ "KCHTMPL0019E": _("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 5376b6c..ecdc907 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 OperationFailed
from kimchi.kvmusertests import UserTests
from kimchi.utils import pool_name_from_uri
from kimchi.utils import probe_file_permission_as_user
@@ -71,11 +72,15 @@ class TemplatesModel(object):
raise InvalidParameter("KCHTMPL0003E", {'network':
net_name,
'template': name})
- 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)
+ try:
+ 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)
+ except Exception as e:
+ raise OperationFailed('KCHTMPL0018E', {'err': e.message})
+
return name
def get_list(self):
@@ -118,8 +123,11 @@ 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 Exception as e:
+ raise OperationFailed('KCHTMPL0019E', {'err': e.message})
def update(self, name, params):
old_t = self.lookup(name)
--
1.8.5.3
3:53 p.m.
New subject: [PATCH 3/5] Fix 'disk full' issue: Fix storage volume error handling
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
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})
return ref_cnt
--
1.8.5.3
3:53 p.m.
New subject: [PATCH 4/5] Fix 'disk full' issue: Fix storagepool and asynctasks error handling
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>
---
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):
--
1.8.5.3
3:53 p.m.
New subject: [PATCH 5/5] Fix 'disk full' issue: Fix vms/screenshot db store/delete error handling
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>
---
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)
--
1.8.5.3
1:40 p.m.
New subject: [PATCH 3/5] Fix 'disk full' issue: Fix storage volume error handling
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.
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
1:43 p.m.
New subject: [PATCH 1/5] Fix 'disk full' issue: Change objectstore exception handling
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>
---
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):
...
1:45 p.m.
New subject: [PATCH 4/5] Fix 'disk full' issue: Fix storagepool and asynctasks error handling
On 03/24/2014 05:53 PM, Rodrigo Trujillo wrote:
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>
---
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"),
I think I've already seen this message before.
Try to reuse the existent messages.
If they are common use KCHAPI[XXXXX]
"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):
1:48 p.m.
New subject: [PATCH 5/5] Fix 'disk full' issue: Fix vms/screenshot db store/delete error handling
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>
---
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.
2:52 p.m.
New subject: [PATCH 1/5] Fix 'disk full' issue: Change objectstore exception handling
On 03/25/2014 03:43 PM, Aline Manera wrote:
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>
> ---
> 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):
...
Indeed ! Thanks
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
3:06 p.m.
New subject: [PATCH 3/5] Fix 'disk full' issue: Fix storage volume error handling
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
3:06 p.m.
New subject: [PATCH 4/5] Fix 'disk full' issue: Fix storagepool and asynctasks error handling
On 03/25/2014 03:45 PM, Aline Manera wrote:
On 03/24/2014 05:53 PM, Rodrigo Trujillo wrote:
> 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>
> ---
> 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"),
I think I've already seen this message before.
Try to reuse the existent messages.
If they are common use KCHAPI[XXXXX]
ACK
>
> "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):
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
3:07 p.m.
New subject: [PATCH 5/5] Fix 'disk full' issue: Fix vms/screenshot db store/delete error handling
On 03/25/2014 03:48 PM, Aline Manera wrote:
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>
> ---
> 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.
Same answer as before :)
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
3707
days inactive
3708
days old
13 comments
2 participants
participants (2)
-
Aline Manera -
Rodrigo Trujillo