[Kimchi-devel] [PATCH] [Kimchi] Bug fix: Remove storage volume file while removing the storage volume

[Paulo Ricardo Paz Vital]

On 03/04/2016 11:45 AM, Rodrigo Trujillo wrote:
> 
> 
> On 03/04/2016 10:27 AM, Aline Manera wrote:
>>
>>
>> On 03/04/2016 10:23 AM, Aline Manera wrote:
>>>
>>>
>>> On 03/04/2016 08:06 AM, Paulo Ricardo Paz Vital wrote:
>>>> I think user should choose if he/she really wants remove the file from
>>>> the system. It's a common approach ask user before execute the deletion
>>>> if he/she wants.
>>>>
>>>> My suggestion is add an additional parameter as a flag to execute your
>>>> try block if True.
>>>
>>> I have thought about that. But it will be a new feature instead of a
>>> bug fix and my intention here was to fix the test case is failing.
>>>
>>
>> Sorry, I was thinking about the UI instead the backend when I replied
>> to you.
>>
>> The delete operation will have a confirmation on UI as we did for VM,
>> for example, but no additional parameter is needed on backend to
>> confirm this action.
>>
>> So when we will have the UI to manage storage volume, a confirmation
>> box will be shown to user but once confirmed a DELETE request is sent
>> without additional parameter.
>>
>> Does that make sense?
>>
>>
> I think that Paulo is saying to create a way to let user choose if he
> wants to delete or not the image file from the system.
> Like:
> 
>  def delete(self, pool, name, wipe=True):
> 
> If wipe is False, then you should keep the file. Thinking on backend
> perspective (API) it is good option.
> 
> Rodrigo
> 

Yeah, Rodrigo got the main idea, but my idea is to the opposite  - if
True then remove the file.

However, if the idea is really remove you can use what Rodrigo said.

>>>>
>>>> Paulo Vital.
>>>>
>>>> On 03/03/2016 05:42 PM, Aline Manera wrote:
>>>>> When removing a storage volume, it was only removed from a libvirt
>>>>> perspective, ie, the storage volume file kept in the system.
>>>>> It may confuse user as he/she will not be able to create a new storage
>>>>> volume with the same name from the storage volume removed before as
>>>>> the file
>>>>> exists in the system.
>>>>> To avoid it, remove the file from the system after removing it from a
>>>>> libvirt perspective.
>>>>>
>>>>> This issue was identified due a failure in the test suite.
>>>>> 2 files related to storage volume tests were kept on the system
>>>>> causing
>>>>> errors when running the test suite multiple times.
>>>>>
>>>>> To verify this patch, please, make sure to do not have any
>>>>> leftovers in the
>>>>> 'default' storage pool directory (/var/lib/libvirt/images).
>>>>> After that, run the test suite mutliple times and you will see no
>>>>> leftovers will be in the system when the test completes.
>>>>>
>>>>> Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
>>>>> ---
>>>>>   model/storagevolumes.py | 7 +++++++
>>>>>   1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/model/storagevolumes.py b/model/storagevolumes.py
>>>>> index d010bcd..f87738f 100644
>>>>> --- a/model/storagevolumes.py
>>>>> +++ b/model/storagevolumes.py
>>>>> @@ -367,12 +367,19 @@ class StorageVolumeModel(object):
>>>>>               raise InvalidParameter("KCHVOL0012E", {'type':
>>>>> pool_info['type']})
>>>>>
>>>>>           volume = StorageVolumeModel.get_storagevolume(pool, name,
>>>>> self.conn)
>>>>> +        vol_path = volume.path()
>>>>>           try:
>>>>>               volume.delete(0)
>>>>>           except libvirt.libvirtError as e:
>>>>>               raise OperationFailed("KCHVOL0010E",
>>>>>                                     {'name': name, 'err':
>>>>> e.get_error_message()})
>>>>>
>>>>> +        try:
>>>>> +            os.remove(vol_path)
>>>>> +        except OSError, e:
>>>>> +            wok_log.error("Unable to delete storage volume file: %s."
>>>>> +                          "Details: %s" % (pool_info['path'],
>>>>> e.message))
>>>>> +
>>>>>       def resize(self, pool, name, size):
>>>>>           volume = StorageVolumeModel.get_storagevolume(pool, name,
>>>>> self.conn)
>>>>>
>>>> _______________________________________________
>>>> Kimchi-devel mailing list
>>>> Kimchi-devel at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>
>>>
>>> _______________________________________________
>>> Kimchi-devel mailing list
>>> Kimchi-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
> 
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>