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

[Paulo Ricardo Paz Vital]

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.

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