Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 04/11/2014 12:00 AM, lvroyce@linux.vnet.ibm.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Tested: 1.make check 2.adding more than 4 cdroms report error 3.less than 4 cdroms successful boot and see all cdroms. 4.When a vm does not have cdrom, adding succeed.

When we only assign source and target type, bus information, libvirt may generate another controller for ide disk appending, this will be conflict with libvirt's limitation of 1 ide controller. This patch choose available bus_id and unit_id for ide disk to make sure we can reach the limit of 4 ide devices per vm. Address of 4 ide devices are: <address type='drive' controller='0' bus='1' target='0' unit='1'/> <address type='drive' controller='0' bus='0' target='0' unit='1'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/>

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/vmstorages.py | 58 ++++++++++++++++++++++++++++++++---------- 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 86ab5d6..2443b3e 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -230,6 +230,7 @@ messages = { "KCHCDROM0011E": _("Do not support guest CDROM hot plug attachment"), "KCHCDROM0012E": _("Specify type and path to add a new virtual machine disk"), "KCHCDROM0013E": _("Specify path to update virtual machine disk"), + "KCHCDROM0014E": _("Controller type %(type)s limitation of %(limit)s devices reached"),

"KCHREPOS0001E": _("YUM Repository ID must be one word only string."), "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// or file:// URL."), diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 3b5f99e..1cffa84 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -37,6 +37,16 @@ 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_storage_xml(params): src_type = params.get('src_type') disk = E.disk(type=src_type, device=params.get('type')) @@ -56,6 +66,12 @@ def _get_storage_xml(params): disk.append(source)

disk.append(E.target(dev=params.get('dev'), bus=params.get('bus', 'ide'))) + if params.get('address'): + # ide disk target id is always '0' + disk.append(E.address( + type='drive', controller=params['address']['controller'], + bus=params['address']['bus'], target='0', + unit=params['address']['unit'])) return ET.tostring(disk)

@@ -89,6 +105,29 @@ class VMStoragesModel(object): def __init__(self, **kargs): self.conn = kargs['conn']

+ def _get_available_ide_address(self, vm_name): + # libvirt limitation of just 1 ide controller + # each controller have at most 2 buses and each bus 2 units. + dom = VMModel.get_vm(vm_name, self.conn) + disks = self.get_list(vm_name) + 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) + if disk.target.attrib['bus'] == 'ide': + controller_id = disk.address.attrib['controller'] + bus_id = disk.address.attrib['bus'] + unit_id = disk.address.attrib['unit'] + if (bus_id, unit_id) in valid_id: + valid_id.remove((bus_id, unit_id)) + continue + if not valid_id: + raise OperationFailed('KCHCDROM0014E', {'type': 'ide', 'limit': 4}) + else: + address = {'controller': controller_id, + 'bus': valid_id[0][0], 'unit': valid_id[0][1]} + return dict(address=address) + def create(self, vm_name, params): dom = VMModel.get_vm(vm_name, self.conn) if DOM_STATE_MAP[dom.info()[0]] != 'shutoff': @@ -109,8 +148,7 @@ class VMStoragesModel(object): path = params['path'] params['src_type'] = _check_cdrom_path(path)

- # Check if path is an url - + params.update(self._get_available_ide_address(vm_name)) # Add device to VM dev_xml = _get_storage_xml(params) try: @@ -147,19 +185,10 @@ class VMStorageModel(object): def __init__(self, **kargs): self.conn = kargs['conn']

- def _get_device_xml(self, vm_name, dev_name): - # Get VM xml and then devices xml - dom = VMModel.get_vm(vm_name, self.conn) - 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 lookup(self, vm_name, dev_name): # Retrieve disk xml and format return dict - disk = self._get_device_xml(vm_name, dev_name) + dom = VMModel.get_vm(vm_name, self.conn) + disk = _get_device_xml(dom, dev_name) if disk is None: raise NotFoundError("KCHCDROM0007E", {'dev_name': dev_name, 'vm_name': vm_name}) @@ -188,7 +217,8 @@ class VMStorageModel(object):

def delete(self, vm_name, dev_name): # Get storage device xml - disk = self._get_device_xml(vm_name, dev_name) + dom = VMModel.get_vm(vm_name, self.conn) + disk = _get_device_xml(dom, dev_name) if disk is None: raise NotFoundError("KCHCDROM0007E", {'dev_name': dev_name,