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

On 04/28/2014 07:20 AM, lvroyce(a)linux.vnet.ibm.com wrote: > From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; > > 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(a)linux.vnet.ibm.com&gt; > --- > 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

> @@ -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(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel