[Kimchi-devel] [PATCH] Delete reference count when deleting storage volume

[Aline Manera]

On 12/01/2015 06:22, Royce Lv wrote:
> On 01/08/2015 08:47 AM, Aline Manera wrote:
>>
>> On 30/12/2014 01:21, lvroyce at linux.vnet.ibm.com wrote:
>>> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>>
>>> If ref count remains in system after volume delete, it will cause
>>> side effect in creating a volume of same name.
>>> Fix this by deleting reference count when deleting storage volume.
>>>
>>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>> ---
>>>   src/kimchi/model/diskutils.py      | 13 +++++++++++++
>>>   src/kimchi/model/storagevolumes.py |  3 ++-
>>>   2 files changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/kimchi/model/diskutils.py 
>>> b/src/kimchi/model/diskutils.py
>>> index 1d575cb..7eb24d2 100644
>>> --- a/src/kimchi/model/diskutils.py
>>> +++ b/src/kimchi/model/diskutils.py
>>> @@ -73,3 +73,16 @@ def set_disk_ref_cnt(objstore, path, new_count):
>>>               session.store('storagevolume', path, {'ref_cnt': 
>>> new_count})
>>>       except Exception as e:
>>>           raise OperationFailed('KCHVOL0017E', {'err': e.message})
>>> +
>>> +
>>> +def clean_disk_ref_cnt(objstore, path):
>>> +    try:
>>> +        with objstore as session:
>>> +            session.delete('storagevolume', path)
>>> +    except NotFoundError:
>>> +        pass
>>> +    except Exception as e:
>>> +        kimchi_log.error('Unable to delete storage volume in'
>>> +                         ' objectstore due error: %s',
>>> +                         e.message)
>>> +        raise OperationFailed('KCHVOL0017E', {'err': e.message})
>>
>> There already are functions in model/diskutils.py to deal with the 
>> ref_cnt: get_disk_ref_cnt() and set_disk_ref_cnt().
>> Please, use set_disk_ref_cnt() to do that clean up.
> I considered to use set_disk_ref_cnt, but it just stores 0 to the 
> path, instead of deleting this entry in objstore,
> since this volume is deleted, we don't need to store this entry in 
> objstore anymore, so I choosed clean_disk_ref_cnt to delete it from 
> objstore.

OK. Maybe we could do it more generic to remove any entry in the objstore.

Also I couldn't apply the patch:

alinefm at alinefm:~/kimchi$ git am -3 ../mail-patches/\[Kimchi-devel\]\ 
\[PATCH\]\ Delete\ reference\ count\ when\ deleting\ storage\ volume.eml
Applying: Delete reference count when deleting storage volume
Using index info to reconstruct a base tree...
M    src/kimchi/model/storagevolumes.py
Falling back to patching base and 3-way merge...
Auto-merging src/kimchi/model/storagevolumes.py
CONFLICT (content): Merge conflict in src/kimchi/model/storagevolumes.py
Failed to merge in the changes.
Patch failed at 0001 Delete reference count when deleting storage volume
The copy of the patch that failed is found in:
    /home/alinefm/kimchi/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


>>
>>> diff --git a/src/kimchi/model/storagevolumes.py 
>>> b/src/kimchi/model/storagevolumes.py
>>> index 406b38b..cf83b0e 100644
>>> --- a/src/kimchi/model/storagevolumes.py
>>> +++ b/src/kimchi/model/storagevolumes.py
>>> @@ -30,7 +30,7 @@ from kimchi.config import READONLY_POOL_TYPE
>>>   from kimchi.exception import InvalidOperation, InvalidParameter, 
>>> IsoFormatError
>>>   from kimchi.exception import MissingParameter, NotFoundError, 
>>> OperationFailed
>>>   from kimchi.isoinfo import IsoImage
>>> -from kimchi.model.diskutils import get_disk_ref_cnt
>>> +from kimchi.model.diskutils import get_disk_ref_cnt, 
>>> clean_disk_ref_cnt
>>>   from kimchi.model.storagepools import StoragePoolModel
>>>   from kimchi.model.tasks import TaskModel
>>>   from kimchi.utils import add_task, get_next_clone_name, kimchi_log
>>> @@ -306,6 +306,7 @@ class StorageVolumeModel(object):
>>>               raise InvalidParameter("KCHVOL0012E", {'type': 
>>> pool_info['type']})
>>>
>>>           volume = self._get_storagevolume(pool, name)
>>> +        clean_disk_ref_cnt(self.objstore, volume.path())
>>>           try:
>>>               volume.delete(0)
>>>           except libvirt.libvirtError as e:
>>
>