Re: [Kimchi-devel] [PATCH] Choose available address for ide disk

Tested-By: Christy Perez <christy(a)linux.vnet.ibm.com&gt; I added 3 new CD ROM devices to total 4, and they got the following addresses: <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'/> Without the patch, I get the error "CHVM0019E: Unable to start virtual machine F19-Remote. Details: internal error: Only 1 ide controller is supported" when I try to start the VM. Two other small comments below...

On Thu, 2014-04-10 at 18:28 +0800, lvroyce(a)linux.vnet.ibm.com wrote: > From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; > > 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. > > Now when just pass 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 cdroms per vm. > > Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; > --- > src/kimchi/i18n.py | 1 + > src/kimchi/model/vmstorages.py | 56 +++++++++++++++++++++++++++++++----------- > 2 files changed, 43 insertions(+), 14 deletions(-) > > diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py > index 86ab5d6..4ca83d9 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 %(limit)s reached"), Kind of a small thing, but with this error, it's not really clear on what the limit is. Just the number kind of makes it sound like it might be a return code or something. I suggest "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..b3f22db 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'): > + print params['address'] Probably want to take that out.

> + 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,27 @@ class VMStoragesModel(object): > def __init__(self, **kargs): > self.conn = kargs['conn'] > > + def _get_available_ide_address(self, vm_name): > + 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 +146,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 +183,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 +215,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, Regards, - Christy _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel