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

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(a)linux.vnet.ibm.com&gt; >>>>> --- >>>>> 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(a)ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >>>> >>> _______________________________________________ >>> Kimchi-devel mailing list >>> Kimchi-devel(a)ovirt.org >>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >>> >> _______________________________________________ >> Kimchi-devel mailing list >> Kimchi-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >> > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel