Re: [Kimchi-devel] [PATCH V2] [Kimchi 1/2] Updated code to support 'interfaces' parameter to template API only on s390x/s390 architecture.

Hi Aline, I have sent V3 patch. Since the current upstream code uses self.info['os_distro'], self.info['os_version'] in get_iface_xml for type network, so not to change any code related to type network I have still pass these params but remove for type macvtap and ovs.

Thanks, Archana Singh On 09/01/2016 01:48 AM, Aline Manera wrote: > > > On 08/30/2016 06:44 PM, archus(a)linux.vnet.ibm.com wrote: >> From: Archana Singh <archus(a)linux.vnet.ibm.com&gt; >> >> 1) Added i18n error codes. >> 2) Added interface as new kimchitype in API.json. And ref added in >> update template. >> -Interface expects an object with parameters: 'name', 'type' >> and 'mode'. >> -Name should be name of host network interface(Ethernet, Bond, >> VLAN) for type 'macvtap' >> or name should be name of host openvswitch bridge interface >> for type ovs. >> -Mode only applicable for interface type macvtap to indicates >> whether packets >> has to be delivered directly to target device(bridge) or to >> the external bridge(vepa-capable bridge). >> -Mode is Optional. >> 3) Added interfaces as optional attribute in template control. >> -This attribute returns to be list of host interface attached >> to template, >> each interface will have above attribute. >> -Empty list if no host network interface is attached to template. >> 4) Added method to generate valid interface xml from the interface >> based on above attribute. >> -This will use below method to generate interface xml based on >> type and other attribute. >> -Update vm xml generate method to add generated interfaces xml >> to vm xml. >> 5) Modified xmlutils which generates interface xml based on >> interface type and attributes. >> 6) Added validate_interfaces method to validate that interfaces are >> only >> supported on s390x/s390 architecture. >> >> Signed-off-by: Archana Singh <archus(a)linux.vnet.ibm.com&gt; >> --- >> API.json | 41 ++++++++++++++++++++++++++++++++- >> control/templates.py | 1 + >> i18n.py | 6 +++++ >> model/templates.py | 17 ++++++++++++++ >> vmtemplate.py | 20 ++++++++++++++++ >> xmlutils/interface.py | 63 >> +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 6 files changed, 147 insertions(+), 1 deletion(-) >> >> diff --git a/API.json b/API.json >> index a3af02d..8899dd2 100644 >> --- a/API.json >> +++ b/API.json >> @@ -89,7 +89,34 @@ >> }, >> "additionalProperties": false, >> "error": "KCHTMPL0030E" >> - } >> + }, >> + "interface": { >> + "description": "Host network interface. This indicates >> how to configure the host network interface (Ethernet, Bond, VLAN) >> as direct macvtap or as OVS interface to a VM.", >> + "type": "object", >> + "properties": { >> + "type": { >> + "description": "Host network interface type. >> Valid types are: 'macvtap' for host network interface (Ethernet, >> Bond, VLAN) to be connected as direct MacVTap or 'ovs' for >> openvswitch host network interface to be connected as virtual switch >> to a VM.", >> + "type": "string", >> + "pattern": "^(macvtap|ovs)$", >> + "required": "True", >> + "error": "KCHTMPL0034E" >> + }, >> + "name": { >> + "description": "The host network interface >> name. It should be name of host network interface(Ethernet, Bond, >> VLAN) for type 'macvtap' and name of host openvswitch bridge >> interface for type ovs", >> + "type": "string", >> + "required": "True", >> + "error": "KCHTMPL0035E" >> + }, >> + "mode": { >> + "description": "Only applicable for macvtap >> interface type. That indicates whether packets will be delivered >> directly to target device (bridge) or to the external bridge >> (vepa-capable bridge).", >> + "type": "string", >> + "pattern": "^(bridge|vepa)$", >> + "error": "KCHTMPL0036E" >> + } >> + }, >> + "additionalProperties": false, >> + "error": "KCHTMPL0038E" >> + } >> }, >> "properties": { >> "storagepools_create": { >> @@ -616,6 +643,12 @@ >> "items": { "type": "string" }, >> "error": "KCHTMPL0017E" >> }, >> + "interfaces": { >> + "description": "list of host interfaces to be >> assigned to new VM", >> + "type": "array", >> + "items": { "ref": "#/kimchitype/interface"}, >> + "error": "KCHTMPL0037E" >> + }, >> "folder": { >> "description": "Folder", >> "type": "array", >> @@ -783,6 +816,12 @@ >> "items": { "type": "string" }, >> "error": "KCHTMPL0017E" >> }, >> + "interfaces": { >> + "description": "list of host interfaces to be >> assigned to new VM", >> + "type": "array", >> + "items": { "ref": "#/kimchitype/interface"}, >> + "error": "KCHTMPL0037E" >> + }, >> "folder": { >> "description": "Folder", >> "type": "array", >> diff --git a/control/templates.py b/control/templates.py >> index 2dd8601..5974737 100644 >> --- a/control/templates.py >> +++ b/control/templates.py >> @@ -68,6 +68,7 @@ class Template(Resource): >> 'cdrom': self.info.get('cdrom', None), >> 'disks': self.info['disks'], >> 'networks': self.info.get('networks', []), >> + 'interfaces': self.info.get('interfaces', []), >> 'folder': self.info.get('folder', []), >> 'graphics': self.info['graphics'], >> 'cpu_info': self.info.get('cpu_info') >> diff --git a/i18n.py b/i18n.py >> index ea2c4ab..90dc67d 100644 >> --- a/i18n.py >> +++ b/i18n.py >> @@ -190,6 +190,12 @@ messages = { >> "KCHTMPL0031E": _("Memory value (%(mem)sMiB) must be equal or >> lesser than maximum memory value (%(maxmem)sMiB)"), >> "KCHTMPL0032E": _("Unable to update template due error: >> %(err)s"), >> "KCHTMPL0033E": _("Parameter 'disks' requires at least one >> disk object"), >> + "KCHTMPL0034E": _("Invalid interface type. Type should be >> 'macvtap' for host network interface (Ethernet, Bond, VLAN) to be >> connected as direct MacVTap or 'ovs' for openvswitch host network >> interface to be connected as virtual switch to a VM."), >> + "KCHTMPL0035E": _("Interface name should be string."), >> + "KCHTMPL0036E": _("Invalid interface mode. Valid options are: >> bridge or vepa."), >> + "KCHTMPL0037E": _("Interfaces should be list of interfaces. >> Each interface should have name, type and mode(optional, only >> applicable for interfcae type 'macvtap'."), >> + "KCHTMPL0038E": _("Interface expects an object with parameters: >> 'name', 'type' and 'mode'. Name should be name of host network >> interface (Ethernet, Bond, VLAN) for type 'macvtap' or the name of >> host openvswitch bridge interface for type 'ovs'. Mode (optional) is >> only applicable for interface type 'macvtap' to indicates whether >> packets will be delivered directly to target device (bridge) or to >> the external bridge (vepa-capable bridge)."), >> + "KCHTMPL0039E": _("Interfaces parameter only supported on s390x >> or s390 architecture."), >> >> "KCHPOOL0001E": _("Storage pool %(name)s already exists"), >> "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), >> diff --git a/model/templates.py b/model/templates.py >> index 8df8c3b..04e6626 100644 >> --- a/model/templates.py >> +++ b/model/templates.py >> @@ -61,6 +61,9 @@ class TemplatesModel(object): >> except Exception: >> raise InvalidParameter("KCHTMPL0003E", {'network': >> net_name, >> 'template': name}) >> + # Valid interfaces >> + interfaces = params.get('interfaces', []) >> + validate_interfaces(interfaces) >> >> # get source_media >> source_media = params.pop("source_media") >> @@ -222,6 +225,10 @@ class TemplateModel(object): >> def update(self, name, params): >> edit_template = self.lookup(name) >> >> + # Valid interfaces >> + interfaces = params.get('interfaces', []) >> + validate_interfaces(interfaces) >> + >> # Merge graphics settings >> graph_args = params.get('graphics') >> if graph_args: >> @@ -273,6 +280,16 @@ class TemplateModel(object): >> return params['name'] >> >> >> +def validate_interfaces(interfaces): >> + # >> + # Interfaces only supported on s390x or s390 architecture. >> + # Otherwise FIXME to valid interfaces exist on system. >> + # >> + if os.uname()[4] not in ['s390x', 's390'] and interfaces: >> + raise InvalidParameter("KCHTMPL0039E") >> + # FIXME to valid interfaces on system. >> + >> + >> def validate_memory(memory): >> # >> # All checking are made in Mib, so, expects memory values in Mib >> diff --git a/vmtemplate.py b/vmtemplate.py >> index babf050..07cebb9 100644 >> --- a/vmtemplate.py >> +++ b/vmtemplate.py >> @@ -298,6 +298,24 @@ class VMTemplate(object): >> self.info['os_version']) >> return unicode(networks, 'utf-8') >> >> + def _get_interfaces_xml(self): >> + interfaces = "" >> + params = {'model': self.info['nic_model']} >> + for interface in self.info.get('interfaces', []): >> + typ = interface['type'] >> + if typ == 'macvtap': >> + params['type'] = 'direct' >> + params['mode'] = interface.get('mode', None) >> + elif typ == 'ovs': >> + params['type'] = 'bridge' >> + params['virtualport_type'] = 'openvswitch' >> + >> + params['name'] = interface['name'] >> + interfaces += get_iface_xml(params, self.info['arch'], > >> + self.info['os_distro'], >> + self.info['os_version']) > > You don't need to pass os_distro and os_version. > The model value is stored in params so you can get it from there. > >> + return unicode(interfaces, 'utf-8') >> + >> def _get_input_output_xml(self): >> sound = """ >> <sound model='%(sound_model)s' /> >> @@ -344,6 +362,7 @@ class VMTemplate(object): >> params['name'] = vm_name >> params['uuid'] = vm_uuid >> params['networks'] = self._get_networks_xml() >> + params['interfaces'] = self._get_interfaces_xml() >> params['input_output'] = self._get_input_output_xml() >> params['qemu-namespace'] = '' >> params['cdroms'] = '' >> @@ -435,6 +454,7 @@ class VMTemplate(object): >> %(disks)s >> %(cdroms)s >> %(networks)s >> + %(interfaces)s >> %(graphics)s >> %(input_output)s >> %(serial)s >> diff --git a/xmlutils/interface.py b/xmlutils/interface.py >> index 677ed81..4b9fe10 100644 >> --- a/xmlutils/interface.py >> +++ b/xmlutils/interface.py >> @@ -24,6 +24,16 @@ from wok.plugins.kimchi import osinfo >> > > >> def get_iface_xml(params, arch=None, os_distro=None, >> os_version=None): > > os_distro and os_version is not needed anymore. > You can access the model value by params.get(model) > >> + typ = params.get('type', 'network') > >> + if typ == 'network': >> + return get_iface_network_xml(params, arch, os_distro, >> os_version) >> + elif typ == 'bridge': >> + return get_iface_ovs_xml(params, arch, os_distro, os_version) >> + elif typ == 'direct': >> + return get_iface_macvtap_xml(params, arch, os_distro, >> os_version) >> + > > The same I commented above related to os_version and os_distro > >> + >> +def get_iface_network_xml(params, arch=None, os_distro=None, >> os_version=None): >> """ >> <interface type='network' name='ethX'> >> <start mode='onboot'/> >> @@ -62,3 +72,56 @@ def get_iface_xml(params, arch=None, >> os_distro=None, os_version=None): >> interface.append(E.mac(address=mac)) >> >> return ET.tostring(interface, encoding='utf-8', >> pretty_print=True) >> + >> + >> +def get_iface_macvtap_xml(params, arch=None, os_distro=None, >> os_version=None): >> + """ >> + <interface type="direct"> >> + <source dev="bondX" mode="bridge"/> >> + <model type="virtio"/> >> + </interface> >> + """ >> + device = params['name'] >> + interface = E.interface(type=params['type']) >> + mode = params.get('mode', None) >> + if mode is not None: >> + interface.append(E.source(dev=device, mode=mode)) >> + else: >> + interface.append(E.source(dev=device)) >> + >> + model = params.get('model', None) >> + >> + # only append 'model' to the XML if it's been specified as a >> parameter >> + # otherwise let libvirt use its default value >> + if model is not None: >> + interface.append(E.model(type=model)) >> + >> + interface.append(E.model(type=model)) >> + >> + return ET.tostring(interface, encoding='utf-8', pretty_print=True) >> + >> + >> +def get_iface_ovs_xml(params, arch=None, os_distro=None, >> os_version=None): >> + """ >> + <interface type="bridge"> >> + <source bridge="vswitchX"/> >> + <virtualport type="openvswitch"/> >> + <model type="virtio"/> >> + </interface> >> + """ >> + device = params['name'] >> + interface = E.interface(type=params['type']) >> + interface.append(E.source(bridge=device)) >> + virtualport_type = params.get('virtualport_type', 'openvswitch') >> + interface.append(E.virtualport(type=virtualport_type)) >> + >> + model = params.get('model', None) >> + >> + # only append 'model' to the XML if it's been specified as a >> parameter >> + # otherwise let libvirt use its default value >> + if model is not None: >> + interface.append(E.model(type=model)) >> + >> + interface.append(E.model(type=model)) >> + >> + return ET.tostring(interface, encoding='utf-8', pretty_print=True) >