[Kimchi-devel] [PATCH V2] [Kimchi 1/2] Updated code to support 'interfaces' parameter to template API only on s390x/s390 architecture.
Aline Manera
alinefm at linux.vnet.ibm.com
Thu Sep 1 16:56:22 UTC 2016
Hi Archana,
On 09/01/2016 12:39 PM, Archana Singh wrote:
> 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.
>
I am OK with that by now, ie, I will apply your v3.
But as you are setting nic_model in params, we can also remove
os_version and os_distro from the upstream code.
Could you send a separated patch for that?
Thanks,
Aline Manera
> Thanks,
>
> Archana Singh
>
>
> On 09/01/2016 01:48 AM, Aline Manera wrote:
>>
>>
>> On 08/30/2016 06:44 PM, archus at linux.vnet.ibm.com wrote:
>>> From: Archana Singh <archus at linux.vnet.ibm.com>
>>>
>>> 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 at linux.vnet.ibm.com>
>>> ---
>>> 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)
>>
>
More information about the Kimchi-devel
mailing list