[Kimchi-devel] [PATCHv4 4/7] Guest disks: Abstract vm disk functions

Aline Manera alinefm at linux.vnet.ibm.com
Mon May 5 21:07:31 UTC 2014


On 05/05/2014 05:35 PM, Aline Manera wrote:
> On 04/28/2014 07:20 AM, lvroyce at linux.vnet.ibm.com wrote:
>> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>
>> Vmstorages will refer to storagevolume functions to get
>> storage volume format and reference count,
>> Meanwhile storage volume will refer to vm storages to see if
>> a path is refered by a vm to prevent a disk be used by multiple
>> vm.
>> To prevent circular reference, abstract vm disk list and lookup
>> function to a single module.
>>
>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>> ---
>>   src/kimchi/model/storagevolumes.py |  9 +++---
>>   src/kimchi/model/vmstorages.py     | 55 
>> +++++-------------------------------
>>   src/kimchi/vmdisks.py              | 58 
>> ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 70 insertions(+), 52 deletions(-)
>>   create mode 100644 src/kimchi/vmdisks.py
>>
>> diff --git a/src/kimchi/model/storagevolumes.py 
>> b/src/kimchi/model/storagevolumes.py
>> index 8a568eb..9a56a63 100644
>> --- a/src/kimchi/model/storagevolumes.py
>> +++ b/src/kimchi/model/storagevolumes.py
>> @@ -28,8 +28,8 @@ from kimchi.exception import MissingParameter, 
>> NotFoundError, OperationFailed
>>   from kimchi.isoinfo import IsoImage
>>   from kimchi.model.storagepools import StoragePoolModel
>>   from kimchi.utils import kimchi_log
>> -from kimchi.model.vms import VMsModel
>> -from kimchi.model.vmstorages import VMStoragesModel, VMStorageModel
>> +from kimchi.model.vms import VMsModel, VMModel
>> +from kimchi.vmdisks import get_vm_disk, get_vm_disk_list
>>
>>
>>   VOLUME_TYPE_MAP = {0: 'file',
>> @@ -134,9 +134,10 @@ class StorageVolumeModel(object):
>>                       # try to find this volume in exsisted vm
>>                       vms = VMsModel.get_vms(self.conn)
>>                       for vm in vms:
>> -                        storages = VMStoragesModel(**args).get_list(vm)
>> +                        dom = VMModel.get_vm(vm, self.conn)
>> +                        storages = get_vm_disk_list(dom)
>>                           for disk in storages:
>> -                            d_info = 
>> VMStorageModel(**args).lookup(vm, disk)
>> +                            d_info = get_vm_disk(dom, disk)
>>                               if path == d_info['path']:
>>                                   ref_cnt = ref_cnt + 1
>>                       session.store('storagevolume', vol_id,
>> diff --git a/src/kimchi/model/vmstorages.py 
>> b/src/kimchi/model/vmstorages.py
>> index b265e6a..c04d511 100644
>> --- a/src/kimchi/model/vmstorages.py
>> +++ b/src/kimchi/model/vmstorages.py
>> @@ -33,22 +33,12 @@ from kimchi.exception import OperationFailed
>>   from kimchi.model.vms import DOM_STATE_MAP, VMModel
>>   from kimchi.utils import check_url_path
>>   from kimchi.osinfo import lookup
>> +from kimchi.vmdisks import get_device_xml, get_vm_disk, 
>> get_vm_disk_list
>> +from kimchi.vmdisks import DEV_TYPE_SRC_ATTR_MAP
>>
>> -DEV_TYPE_SRC_ATTR_MAP = {'file': 'file',
>> -                         'block': 'dev'}
>>   HOTPLUG_TYPE = ['scsi', 'virtio']
>>
>>
>> -def _get_device_xml(dom, dev_name):
>> -    # Get VM xml and then devices xml
>> -    xml = dom.XMLDesc(0)
>> -    devices = objectify.fromstring(xml).devices
>> -    disk = devices.xpath("./disk/target[@dev='%s']/.." % dev_name)
>> -    if not disk:
>> -        return None
>> -    return disk[0]
>> -
>> -
>>   def _get_device_bus(dev_type, dom):
>>       try:
>>           version, distro = VMModel.vm_get_os_metadata(dom)
>> @@ -114,6 +104,7 @@ def _check_cdrom_path(path):
>>   class VMStoragesModel(object):
>>       def __init__(self, **kargs):
>>           self.conn = kargs['conn']
>> +        self.objstore = kargs['objstore']
>>
>>       def _get_available_bus_address(self, bus_type, vm_name):
>>           if bus_type not in ['ide']:
>> @@ -125,7 +116,7 @@ class VMStoragesModel(object):
>>           valid_id = [('0', '0'), ('0', '1'), ('1', '0'), ('1', '1')]
>>           controller_id = '0'
>>           for dev_name in disks:
>> -            disk = _get_device_xml(dom, dev_name)
>> +            disk = get_device_xml(dom, dev_name)
>>               if disk.target.attrib['bus'] == 'ide':
>>                   controller_id = disk.address.attrib['controller']
>>                   bus_id = disk.address.attrib['bus']
>> @@ -156,7 +147,6 @@ class VMStoragesModel(object):
>>           # There is no need to cover this case here.
>>           path = params['path']
>>           params['src_type'] = _check_cdrom_path(path)
>> -
>>           params.setdefault(
>>               'bus', _get_device_bus(params['type'], dom))
>>           if (params['bus'] not in HOTPLUG_TYPE
>> @@ -187,13 +177,7 @@ class VMStoragesModel(object):
>>
>>       def get_list(self, vm_name):
>>           dom = VMModel.get_vm(vm_name, self.conn)
>> -        xml = dom.XMLDesc(0)
>> -        devices = objectify.fromstring(xml).devices
>> -        storages = [disk.target.attrib['dev']
>> -                    for disk in 
>> devices.xpath("./disk[@device='disk']")]
>> -        storages += [disk.target.attrib['dev']
>> -                     for disk in 
>> devices.xpath("./disk[@device='cdrom']")]
>> -        return storages
>> +        return get_vm_disk_list(dom)
>>
>>
>>   class VMStorageModel(object):
>> @@ -203,32 +187,7 @@ class VMStorageModel(object):
>>       def lookup(self, vm_name, dev_name):
>>           # Retrieve disk xml and format return dict
>>           dom = VMModel.get_vm(vm_name, self.conn)
>> -        disk = _get_device_xml(dom, dev_name)
>> -        if disk is None:
>> -            raise NotFoundError("KCHVMSTOR0007E", {'dev_name': 
>> dev_name,
>> -                                                  'vm_name': vm_name})
>> -        path = ""
>> -        dev_bus = 'ide'
>> -        try:
>> -            source = disk.source
>> -            if source is not None:
>> -                src_type = disk.attrib['type']
>> -                if src_type == 'network':
>> -                    host = source.host
>> -                    path = (source.attrib['protocol'] + '://' +
>> -                            host.attrib['name'] + ':' +
>> -                            host.attrib['port'] + 
>> source.attrib['name'])
>> -                else:
>> -                    path = 
>> source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]]
>> -            # Retrieve storage bus type
>> -            dev_bus = disk.target.attrib['bus']
>> -        except:
>> -            pass
>> -        dev_type = disk.attrib['device']
>> -        return {'dev': dev_name,
>> -                'type': dev_type,
>> -                'path': path,
>> -                'bus': dev_bus}
>> +        return get_vm_disk(dom, dev_name)
>>
>>       def delete(self, vm_name, dev_name):
>>           # Get storage device xml
>> @@ -246,7 +205,7 @@ class VMStorageModel(object):
>>           try:
>>               conn = self.conn.get()
>>               dom = conn.lookupByName(vm_name)
>> -            disk = _get_device_xml(dom, dev_name)
>> +            disk = get_device_xml(dom, dev_name)
>>               dom.detachDeviceFlags(etree.tostring(disk),
>> libvirt.VIR_DOMAIN_AFFECT_CURRENT)
>>           except Exception as e:
>> diff --git a/src/kimchi/vmdisks.py b/src/kimchi/vmdisks.py
>> new file mode 100644
>> index 0000000..73269ee
>> --- /dev/null
>> +++ b/src/kimchi/vmdisks.py
>
> Missing the license header
>

I will add it before applying.

>> @@ -0,0 +1,58 @@
>> +from lxml import objectify
>> +
>> +from kimchi.exception import NotFoundError
>> +
>> +DEV_TYPE_SRC_ATTR_MAP = {'file': 'file',
>> +                         'block': 'dev'}
>> +
>> +
>> +def get_device_xml(dom, dev_name):
>> +    # Get VM xml and then devices xml
>> +    xml = dom.XMLDesc(0)
>> +    devices = objectify.fromstring(xml).devices
>> +    disk = devices.xpath("./disk/target[@dev='%s']/.." % dev_name)
>> +    if not disk:
>> +        return None
>> +    return disk[0]
>> +
>> +
>> +def get_vm_disk(dom, dev_name):
>> +    # Retrieve disk xml and format return dict
>> +    disk = get_device_xml(dom, dev_name)
>> +    if disk is None:
>> +        raise NotFoundError(
>> +                "KCHVMSTOR0007E",
>> +                {'dev_name': dev_name, 'vm_name': dom.name()})
>> +    path = ""
>> +    dev_bus = 'ide'
>> +    try:
>> +        source = disk.source
>> +        if source is not None:
>> +            src_type = disk.attrib['type']
>> +            if src_type == 'network':
>> +                host = source.host
>> +                path = (source.attrib['protocol'] + '://' +
>> +                        host.attrib['name'] + ':' +
>> +                        host.attrib['port'] + source.attrib['name'])
>> +            else:
>> +                path = source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]]
>> +        # Retrieve storage bus type
>> +        dev_bus = disk.target.attrib['bus']
>> +    except:
>> +        pass
>> +    dev_type = disk.attrib['device']
>> +    return {'dev': dev_name,
>> +            'type': dev_type,
>> +            'path': path,
>> +            'format': disk.driver.attrib['type'],
>> +            'bus': dev_bus}
>> +
>> +
>> +def get_vm_disk_list(dom):
>> +    xml = dom.XMLDesc(0)
>> +    devices = objectify.fromstring(xml).devices
>> +    storages = [disk.target.attrib['dev']
>> +                for disk in devices.xpath("./disk[@device='disk']")]
>> +    storages += [disk.target.attrib['dev']
>> +                 for disk in devices.xpath("./disk[@device='cdrom']")]
>> +    return storages
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>




More information about the Kimchi-devel mailing list