[Kimchi-devel] [PATCH V1] [Kimchi] Updated code to support add, remove and list host network interface(s) to exiting template.
Aline Manera
alinefm at linux.vnet.ibm.com
Tue Aug 30 18:33:06 UTC 2016
On 08/22/2016 02:49 PM, archus at linux.vnet.ibm.com wrote:
> From: Archana Singh <archus at linux.vnet.ibm.com>
>
> interfaces is an additional optional attribute and hence exiting functionalities to add,
> remove and list networks from/to template are not changed.
> Below are the changes:
>
> 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.
>
> Signed-off-by: Archana Singh <archus at linux.vnet.ibm.com>
> ---
> API.json | 29 ++++++++++++++++++++-
> control/templates.py | 1 +
> i18n.py | 2 ++
> vmtemplate.py | 24 +++++++++++++++++
> xmlutils/interface.py | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 127 insertions(+), 1 deletion(-)
>
> diff --git a/API.json b/API.json
> index a3af02d..e323649 100644
> --- a/API.json
> +++ b/API.json
> @@ -89,7 +89,29 @@
> },
> "additionalProperties": false,
> "error": "KCHTMPL0030E"
> - }
> + },
> + "interface": {
> + "description": "Host network interface, this indicates whether to configure the host network interface(Ethernet, Bond, VLAN) as direct MacVTap or to configure interface(OVS) as virtual switch to a VM",
Just rephrasing the 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": "Type of host network interface. Type should be 'macvtap' for host network interface(Ethernet, Bond, VLAN) to be connected as direct MacVTap and type ovs for openvswitch host network interface to be connected as virtual switch to a VM.",
" 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)$"
> + },
> + "name": {
> + "description": "The host network interface. It should be name of host network interface(Ethernet, Bond, VLAN) for type 'macvtap' and name of host openvswitch bridge interface for type ovs",
"The host network interface name."
> + "type": "string"
> + },
> + "mode": {
> + "description": "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). Optional.",
"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)."
You don't need to explain by text that the option is optional.
Instead of that, for the required parameters you should add the
configuration "required: True" to make sure the data structure will be
valid correctly by jsonschema.
So if name and type are not optional, please, add "required: True" for
each one.
Also for each element, you need to add an error code. This error will be
raised in the user enters a invalid value. For example, the jsonschema
is expecting an string and receive an number.
Usually those messages are like "Invalid interface type. Valid options
are: macvtap or ovs."
> + "type": "string",
> + "pattern": "^(bridge|vepa)$"
> + }
> + },
> + "additionalProperties": false,
> + "error": "KCHTMPL0033E"
> + }
> },
> "properties": {
> "storagepools_create": {
> @@ -783,6 +805,11 @@
> "items": { "type": "string" },
> "error": "KCHTMPL0017E"
> },
> + "interfaces": {
> + "description": "list of host interfaces to be assigned to new VM",
> + "type": "array",
> + "items": { "ref": "#/kimchitype/interface" }
'error' is missing in this configuration too.
> + },
> "folder": {
> "description": "Folder",
> "type": "array",
> diff --git a/control/templates.py b/control/templates.py
> index bb2e068..d81398a 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['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 e8d9c05..7cda46d 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -159,6 +159,7 @@ messages = {
> "KCHVMIF0009E": _("MAC Address %(mac)s already exists in virtual machine %(name)s"),
> "KCHVMIF0010E": _("Invalid MAC Address"),
> "KCHVMIF0011E": _("Cannot change MAC address of a running virtual machine"),
> + "KCHVMIF0012E": _("Invalid host network interface type (%(type)s). Type should be 'macvtap' for host network interface(Ethernet, Bond, VLAN) to be connected as direct MacVTap and type 'ovs' for openvswitch host network interface to be connected as virtual switch to a VM."),
"Invalid host network interface type (%(type)s). 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. "
>
> "KCHTMPL0001E": _("Template %(name)s already exists"),
> "KCHTMPL0002E": _("Source media %(path)s not found"),
> @@ -190,6 +191,7 @@ 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"),
> + "KCHTMPL0033E": _("Interface expects an object with parameters: 'name', 'type' and 'mode'. Name should be name of host network interface(Ethernet, Bond, VLAN) for type 'macvtap' and name of host openvswitch bridge interface for type ovs. Mode 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) and is Optional."),
"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)."
> "KCHPOOL0001E": _("Storage pool %(name)s already exists"),
> "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
> diff --git a/vmtemplate.py b/vmtemplate.py
> index 7ac0541..fed6d28 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -22,6 +22,7 @@ import stat
> import time
> import urlparse
> import uuid
> +
> from lxml import etree
> from lxml.builder import E
>
> @@ -295,6 +296,27 @@ 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'
> + else:
> + raise InvalidParameter('KCHVMIF0012E', {'type':
> + params['type']})
> +
The 'else' statement is not needed. The data validation will be done by
jsonschema as you have set there.
So when the data comes to _get_interfaces_xml() it has valid inputs.
> + params['name'] = interface['name']
> + interfaces += get_iface_xml(params, self.info['arch'],
> + self.info['os_distro'],
> + self.info['os_version'])
> + return unicode(interfaces, 'utf-8')
> +
> def _get_input_output_xml(self):
> sound = """
> <sound model='%(sound_model)s' />
> @@ -341,6 +363,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'] = ''
> @@ -432,6 +455,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..255d491 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):
> + 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)
> +
> +
> +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,65 @@ 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)
> + # no model specified; let's try querying osinfo
> + if model is None:
> + # if os_distro and os_version are invalid, nic_model will also be None
> + model = osinfo.lookup(os_distro, os_version).get('nic_model')
> +
You should set a default model parameter in the vmtemplate. In
vmtemplate.py, the osinfo.lookup() is called and all information stores
in self.info.
When you call the function to build the XML, you already know that
information so you can pass it as parameter to the XML function instead
of calling the osinfo.lookup() again.
> + # only append 'model' to the XML if it's been specified as a parameter or
> + # returned by osinfo.lookup; 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)
> + # no model specified; let's try querying osinfo
> + if model is None:
> + # if os_distro and os_version are invalid, nic_model will also be None
> + model = osinfo.lookup(os_distro, os_version).get('nic_model')
> +
The same I commented before.
> + # only append 'model' to the XML if it's been specified as a parameter or
> + # returned by osinfo.lookup; 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