[Kimchi-devel] [PATCH 4/8] Update get_disk_xml() to get the device same according to bus and index values

Aline Manera alinefm at linux.vnet.ibm.com
Mon Oct 27 15:40:23 UTC 2014


When generating the disk XML, the most common information to generate the
device name are: 1) the bus - to identify the right prefix, and 2) the disk
index.
Add this logic to get_disk_xml() so when no 'dev' name is specified the
device name will be automatically generated according to bus and index
value.

Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
---
 src/kimchi/model/storagevolumes.py |  6 ++--
 src/kimchi/model/vmstorages.py     | 67 ++++++++++++++++++--------------------
 src/kimchi/xmlutils/disk.py        | 25 +++++++++-----
 3 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
index db596c3..9ff43e6 100644
--- a/src/kimchi/model/storagevolumes.py
+++ b/src/kimchi/model/storagevolumes.py
@@ -32,7 +32,7 @@ from kimchi.model.storagepools import StoragePoolModel
 from kimchi.model.tasks import TaskModel
 from kimchi.model.vms import VMsModel, VMModel
 from kimchi.utils import add_task, kimchi_log
-from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disk_list
+from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks
 from kimchi.xmlutils.utils import xpath_get_text
 
 
@@ -265,8 +265,8 @@ class StorageVolumeModel(object):
                     vms = VMsModel.get_vms(self.conn)
                     for vm in vms:
                         dom = VMModel.get_vm(vm, self.conn)
-                        storages = get_vm_disk_list(dom)
-                        for disk in storages:
+                        storages = get_vm_disks(dom)
+                        for disk in storages.keys():
                             d_info = get_vm_disk_info(dom, disk)
                             if path == d_info['path']:
                                 ref_cnt = ref_cnt + 1
diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py
index 3a8b6b3..b3b1514 100644
--- a/src/kimchi/model/vmstorages.py
+++ b/src/kimchi/model/vmstorages.py
@@ -31,10 +31,9 @@ from kimchi.model.utils import get_vm_config_flag
 from kimchi.utils import check_url_path
 from kimchi.osinfo import lookup
 from kimchi.xmlutils.disk import get_device_node, get_disk_xml
-from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disk_list
+from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks
 
 HOTPLUG_TYPE = ['scsi', 'virtio']
-PREFIX_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'}
 
 
 def _get_device_bus(dev_type, dom):
@@ -95,14 +94,28 @@ class VMStoragesModel(object):
             return dict(address=address)
 
     def create(self, vm_name, params):
-        dom = VMModel.get_vm(vm_name, self.conn)
-        params['bus'] = _get_device_bus(params['type'], dom)
-        self._get_storage_device_name(vm_name, params)
         # Path will never be blank due to API.json verification.
         # There is no need to cover this case here.
-        params['format'] = 'raw'
         if not ('vol' in params) ^ ('path' in params):
             raise InvalidParameter("KCHVMSTOR0017E")
+
+        dom = VMModel.get_vm(vm_name, self.conn)
+        params['bus'] = _get_device_bus(params['type'], dom)
+        params['format'] = 'raw'
+
+        dev_list = [dev for dev, bus in get_vm_disks(dom).iteritems()
+                    if bus == params['bus']]
+        dev_list.sort()
+        if len(dev_list) == 0:
+            params['index'] = 0
+        else:
+            char = dev_list.pop()[2]
+            params['index'] = string.ascii_lowercase.index(char) + 1
+
+        if (params['bus'] not in HOTPLUG_TYPE
+                and DOM_STATE_MAP[dom.info()[0]] != 'shutoff'):
+            raise InvalidOperation('KCHVMSTOR0011E')
+
         if params.get('vol'):
             try:
                 pool = params['pool']
@@ -119,48 +132,30 @@ class VMStoragesModel(object):
             supported_format = {
                 "disk": ["raw", "bochs", "qcow", "qcow2", "qed", "vmdk"],
                 "cdrom": "iso"}
-            if vol_info['format'] in supported_format[params['type']]:
-                if params['type'] == 'disk':
-                    params['format'] = vol_info['format']
-            else:
+            if vol_info['format'] not in supported_format[params['type']]:
                 raise InvalidParameter("KCHVMSTOR0018E",
                                        {"format": vol_info['format'],
                                         "type": params['type']})
-            params['path'] = vol_info['path']
 
-        if (params['bus'] not in HOTPLUG_TYPE
-                and DOM_STATE_MAP[dom.info()[0]] != 'shutoff'):
-            raise InvalidOperation('KCHVMSTOR0011E')
+            if params['type'] == 'disk':
+                params['format'] = vol_info['format']
+
+            params['path'] = vol_info['path']
 
         params.update(self._get_available_bus_address(params['bus'], vm_name))
         # Add device to VM
-        dev_xml = get_disk_xml(_check_path(params['path']), params)
+        dev, xml = get_disk_xml(_check_path(params['path']), params)
         try:
             conn = self.conn.get()
             dom = conn.lookupByName(vm_name)
-            dom.attachDeviceFlags(dev_xml, get_vm_config_flag(dom, 'all'))
+            dom.attachDeviceFlags(xml, get_vm_config_flag(dom, 'all'))
         except Exception as e:
             raise OperationFailed("KCHVMSTOR0008E", {'error': e.message})
-        return params['dev']
-
-    def _get_storage_device_name(self, vm_name, params):
-        bus_prefix = PREFIX_MAP[params['bus']]
-        dev_list = [dev for dev in self.get_list(vm_name)
-                    if dev.startswith(bus_prefix)]
-        if len(dev_list) == 0:
-            params['dev'] = bus_prefix + 'a'
-        else:
-            dev_list.sort()
-            last_dev = dev_list.pop()
-            # TODO: Improve to device names "greater then" hdz
-            next_dev_letter_pos =\
-                string.ascii_lowercase.index(last_dev[2]) + 1
-            params['dev'] =\
-                bus_prefix + string.ascii_lowercase[next_dev_letter_pos]
+        return dev
 
     def get_list(self, vm_name):
         dom = VMModel.get_vm(vm_name, self.conn)
-        return get_vm_disk_list(dom)
+        return get_vm_disks(dom).keys()
 
 
 class VMStorageModel(object):
@@ -207,11 +202,11 @@ class VMStorageModel(object):
         dev_info = self.lookup(vm_name, dev_name)
         if dev_info['type'] != 'cdrom':
             raise InvalidOperation("KCHVMSTOR0006E")
-        dev_info.update(params)
 
-        xml = get_disk_xml(src_type, dev_info, ignore_source)
+        dev_info.update(params)
+        dev, xml = get_disk_xml(src_type, dev_info, ignore_source)
         try:
             dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all'))
         except Exception as e:
             raise OperationFailed("KCHVMSTOR0009E", {'error': e.message})
-        return dev_name
+        return dev
diff --git a/src/kimchi/xmlutils/disk.py b/src/kimchi/xmlutils/disk.py
index 800a64c..aadbfb8 100644
--- a/src/kimchi/xmlutils/disk.py
+++ b/src/kimchi/xmlutils/disk.py
@@ -19,6 +19,7 @@
 
 import lxml.etree as ET
 import socket
+import string
 import urlparse
 
 from lxml import objectify
@@ -26,6 +27,7 @@ from lxml.builder import E
 
 from kimchi.exception import NotFoundError
 
+BUS_TO_DEV_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'}
 DEV_TYPE_SRC_ATTR_MAP = {'file': 'file', 'block': 'dev'}
 
 
@@ -42,7 +44,11 @@ def get_disk_xml(src_type, params, ignore_src=False):
     """
     disk = E.disk(type=src_type, device=params['type'])
     disk.append(E.driver(name='qemu', type=params['format']))
-    disk.append(E.target(dev=params['dev'], bus=params['bus']))
+
+    # Get device name according to bus and index values
+    dev = params.get('dev', (BUS_TO_DEV_MAP[params['bus']] +
+                             string.lowercase[params.get('index', 0)]))
+    disk.append(E.target(dev=dev, bus=params['bus']))
 
     if params.get('address'):
         # ide disk target id is always '0'
@@ -52,7 +58,7 @@ def get_disk_xml(src_type, params, ignore_src=False):
             unit=params['address']['unit']))
 
     if ignore_src:
-        return ET.tostring(disk, encoding='utf-8', pretty_print=True)
+        return (dev, ET.tostring(disk, encoding='utf-8', pretty_print=True))
 
     if src_type == 'network':
         """
@@ -73,7 +79,7 @@ def get_disk_xml(src_type, params, ignore_src=False):
         source.set(DEV_TYPE_SRC_ATTR_MAP[src_type], params['path'])
 
     disk.append(source)
-    return ET.tostring(disk, encoding='utf-8', pretty_print=True)
+    return (dev, ET.tostring(disk, encoding='utf-8', pretty_print=True))
 
 
 def get_device_node(dom, dev_name):
@@ -116,11 +122,14 @@ def get_vm_disk_info(dom, dev_name):
             'bus': disk.target.attrib['bus']}
 
 
-def get_vm_disk_list(dom):
+def get_vm_disks(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']")]
+
+    storages = {}
+    all_disks = devices.xpath("./disk[@device='disk']")
+    all_disks.extend(devices.xpath("./disk[@device='cdrom']"))
+    for disk in all_disks:
+        storages[disk.target.attrib['dev']] = disk.target.attrib['bus']
+
     return storages
-- 
1.9.3




More information about the Kimchi-devel mailing list