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

[Aline Manera]

On 03/12/2014 11:53 AM, Sheldon wrote:
> Not sure we still need this.
> https://github.com/kimchi-project/kimchi/wiki/Todo-1.2#wiki-12-sprint-4-ends-031214
> *Backend* Implement integrity verification:
>
>   * don't disable network/storagpool used by guest/template
>

This is also important.
In addition only pointing the invalid fields we should also avoid user 
to create inconsistent resources.

The integrity verification may help us identify inconsistent resources 
that were managed by other tool
(like virt-manager, virsh, etc)

>
> On 03/12/2014 10:50 PM, 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.
>>
>> 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)
>> +
>>       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",
>
>
> -- 
> Thanks and best regards!
>
> Sheldon Feng(???)<shaohef at linux.vnet.ibm.com>
> IBM Linux Technology Center
>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140312/b6079351/attachment.html>