[Kimchi-devel] [PATCH 5/5] Fix 'disk full' issue: Fix vms/screenshot db store/delete error handling

[Aline Manera]

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 at 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.