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

Archana Singh archus at linux.vnet.ibm.com
Fri Sep 2 04:00:49 UTC 2016


On 09/01/2016 10:26 PM, Aline Manera wrote:
>
> 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?
Sure, I will sent separate for removing  os_version and os_distro from 
the upstream code.
>
> 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