[Kimchi-devel] [PATCH] [Kimchi] Issue #626: Snapshot revert does not release storage volume

[Aline Manera]

On 08/29/2016 04:44 PM, bianca at linux.vnet.ibm.com wrote:
> From: Bianca Carvalho <bianca at linux.vnet.ibm.com>
>
> Edited diskutils.py file to remove used_by verification in
> objstore, now this field will be only verified in the vm
> when get_disk_used_by is used by StorageVolumeModel lookup.
>
> Signed-off-by: Bianca Carvalho <bianca at linux.vnet.ibm.com>
> ---
>   model/diskutils.py | 50 ++++++++++++++++++++++----------------------------
>   1 file changed, 22 insertions(+), 28 deletions(-)
>
> diff --git a/model/diskutils.py b/model/diskutils.py
> index 6faab7c..3b8f441 100644
> --- a/model/diskutils.py
> +++ b/model/diskutils.py
> @@ -17,7 +17,7 @@
>   # License along with this library; if not, write to the Free Software
>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
>
> -from wok.exception import OperationFailed, NotFoundError
> +from wok.exception import OperationFailed
>   from wok.utils import wok_log
>
>   from wok.plugins.kimchi.config import get_kimchi_version
> @@ -34,34 +34,28 @@ from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks
>   def get_disk_used_by(objstore, conn, path):


>       try:
>           with objstore as session:

The objectstore is not needed anymore.
You can remove this 'with' statement.

Also, the above 'try' statement is not needed anymore.
The correspondent except statement has the following comment:

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

So as we are not checking the objectstore anymore, we don't need the 
'with' statement and also that 'try/expect' statement.

> +            used_by = []
> +            # try to find this volume in existing vm
> +            vms_list = VMsModel.get_vms(conn)
> +            for vm in vms_list:
> +                dom = VMModel.get_vm(vm, conn)
> +                storages = get_vm_disks(dom)
> +                for disk in storages.keys():
> +                    d_info = get_vm_disk_info(dom, disk)
> +                    if path == d_info['path']:
> +                        used_by.append(vm)
>               try:
> -                used_by = session.get('storagevolume', path)['used_by']
> -            except (KeyError, NotFoundError):
> -                wok_log.info('Volume %s not found in obj store.' % path)
> -                used_by = []
> -                # try to find this volume in existing vm
> -                vms_list = VMsModel.get_vms(conn)
> -                for vm in vms_list:
> -                    dom = VMModel.get_vm(vm, conn)
> -                    storages = get_vm_disks(dom)
> -                    for disk in storages.keys():
> -                        d_info = get_vm_disk_info(dom, disk)
> -                        if path == d_info['path']:
> -                            used_by.append(vm)
> -                try:
> -                    session.store('storagevolume', path,
> -                                  {'used_by': used_by},
> -                                  get_kimchi_version())
> -                except Exception as e:
> -                    # Let the exception be raised. If we allow disks'
> -                    #   used_by to be out of sync, data corruption could
> -                    #   occour if a disk is added to two guests
> -                    #   unknowingly.
> -                    wok_log.error('Unable to store storage volume id in'
> -                                  ' objectstore due error: %s',
> -                                  e.message)
> -                    raise OperationFailed('KCHVOL0017E',
> -                                          {'err': e.message})
> +                session.store('storagevolume', path, {'used_by': used_by},
> +                              get_kimchi_version())
> +            except Exception as e:
> +                # Let the exception be raised. If we allow disks'
> +                #   used_by to be out of sync, data corruption could
> +                #   occour if a disk is added to two guests
> +                #   unknowingly.
> +                wok_log.error('Unable to store storage volume id in'
> +                              ' objectstore due error: %s',
> +                              e.message)
> +                raise OperationFailed('KCHVOL0017E', {'err': e.message})
>       except Exception as e:
>           # This exception is going to catch errors returned by 'with',
>           # specially ones generated by 'session.store'. It is outside