[Kimchi-devel] [RFC][PATCH] Do not allow user deactivate/delete storage pool used by vms

[Aline Manera]

On 03/12/2014 11:50 AM, shaohef at linux.vnet.ibm.com wrote:
> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>
> Make a integrity verification when user try to deactivate or delete a
> pool.
> If a volume of a pool is used by a vm, the pool should not be deactivated
> or deleted to avoid problems.

> It is no harmful to allow to deactivate/delete dir pool, for
> kimchi does not delete the pool path when delete this pool.

We should allow the user deactivate/delete a dir pool if there is any 
volume used.
If reasonable we could add *later* an option to user ask Kimchi to also 
clean up file system while deleting the pool.
But it needs to be discussed as I don't have a right answer for that 
right now.

> But it is harmful for other type pools, such as logical or nfs pool.
>
> Royce has added a ref_cnt for volume, if the sum of ref_cnt in this pool
> larger than 0, means this pool is used by some vms.
>
> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
> ---
>   src/kimchi/i18n.py               |  2 ++
>   src/kimchi/model/storagepools.py | 23 +++++++++++++++++++++++
>   2 files changed, 25 insertions(+)
>
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index 1ae3889..7c8fbc7 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -144,6 +144,8 @@ messages = {
>       "KCHPOOL0034E": _("Unable to deactivate pool %(name)s as it is associated with some templates"),
>       "KCHPOOL0035E": _("Unable to delete pool %(name)s as it is associated with some templates"),
>       "KCHPOOL0036E": _("A volume group named '%(name)s' already exists. Please, choose another name to create the logical pool."),
> +    "KCHPOOL0037E": _("Unable to deactivate pool %(name)s as it is used by some vms"),
> +    "KCHPOOL0038E": _("Unable to delete pool %(name)s as it is used by some vms"),
>
>       "KCHVOL0001E": _("Storage volume %(name)s already exists"),
>       "KCHVOL0002E": _("Storage volume %(name)s does not exist in storage pool %(pool)s"),
> diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py
> index fea19f6..247c374 100644
> --- a/src/kimchi/model/storagepools.py
> +++ b/src/kimchi/model/storagepools.py
> @@ -179,6 +179,16 @@ class StoragePoolModel(object):
>               else:
>                   raise
>
> +    def _get_volumes_ref_cnt(self, name):
> +        # in order to avoid nested import
> +        from kimchi.model.storagevolumes import StorageVolumesModel
> +        from kimchi.model.storagevolumes import StorageVolumeModel
> +        kwargs = {"conn": self.conn, "objstore": self.objstore}
> +        cnts = [StorageVolumeModel(**kwargs).lookup(name, volume)["ref_cnt"]
> +                for volume in StorageVolumesModel(**kwargs).get_list(name)]
> +
> +        return sum(cnts)

We should expose this value to API.
So UI can disable the buttons when pool is in use.
As I did for network

And also marked default pool as always is use.

> +
>       def _get_storagepool_vols_num(self, pool):
>           try:
>               if pool.isActive():
> @@ -345,6 +355,9 @@ class StoragePoolModel(object):
>               raise InvalidOperation('KCHPOOL0034E', {'name': name})
>
>           pool = self.get_storagepool(name, self.conn)
> +        if pool.isActive() and self._get_volumes_ref_cnt(name) > 0:
> +            raise InvalidOperation('KCHPOOL0037E', {'name': name})
> +
>           #FIXME: nfs workaround - do not try to deactivate a NFS pool
>           # if the NFS server is not reachable.
>           xml = pool.XMLDesc(0)
> @@ -374,6 +387,16 @@ class StoragePoolModel(object):
>           if pool.isActive():
>               raise InvalidOperation("KCHPOOL0005E", {'name': name})
>           try:
> +            try:
> +                pool.create(0)
> +            except Exception:
> +                pass
> +            else:
> +                if self._get_volumes_ref_cnt(name) > 0:
> +                    pool.destroy()
> +                    raise InvalidOperation('KCHPOOL0038E', {'name': name})
> +                else:
> +                    pool.destroy()
>               pool.undefine()
>           except libvirt.libvirtError as e:
>               raise OperationFailed("KCHPOOL0011E",