[Kimchi-devel] [PATCH V1] [Kimchi 1/2] Updated code to support VM ifaces on s390x/s390 architecture.

Archana Singh archus at linux.vnet.ibm.com
Fri Sep 2 03:57:34 UTC 2016


Hi Aline,

I have sent V2 incorporated your comments.

Thanks,

Archana Singh

On 09/01/2016 01:59 AM, Aline Manera wrote:
> Hi Archana,
>
> The overall patch looks good to me.
>
> I just made a suggestion below to simplify the code.
>
> On 08/31/2016 01:16 PM, archus at linux.vnet.ibm.com wrote:
>> From: Archana Singh <archus at linux.vnet.ibm.com>
>>
>> 1) Code ensures that additional type macvtap, ovs are only
>> supported on s390x/s390 architecture. On other arch it throws
>> Error for these two type.
>> 2) Code ensures that source is provided for type macvtap, ovs
>> and only applicable for s390x/s390 architecture.
>> 3) Code to create/get VM ifaces for source of type macvtap or ovs on
>> s390x/s390 architecture.
>>
>> Signed-off-by: Archana Singh <archus at linux.vnet.ibm.com>
>> ---
>>   API.json              | 16 +++++++++++---
>>   i18n.py               |  9 +++++++-
>>   model/vmifaces.py     | 60 
>> +++++++++++++++++++++++++++++++++++++++++++++------
>>   xmlutils/interface.py |  8 +++++++
>>   4 files changed, 82 insertions(+), 11 deletions(-)
>>
>> diff --git a/API.json b/API.json
>> index 8899dd2..adaa2f7 100644
>> --- a/API.json
>> +++ b/API.json
>> @@ -511,18 +511,28 @@
>>               "error": "KCHVMIF0007E",
>>               "properties": {
>>                   "type": {
>> -                    "description": "The type of VM network interface 
>> that libvirt supports",
>> +                    "description": "The type of VM network interface 
>> that libvirt supports. Type '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 or 'network' for libvirt virtual network to be connected to 
>> VM. ",
>>                       "type": "string",
>> -                    "pattern": "^network$",
>> +                    "pattern": "^network|macvtap|ovs$",
>>                       "required": true,
>>                       "error": "KCHVMIF0004E"
>>                   },
>>                   "network": {
>>                       "description": "the name of one available 
>> network",
>> -                    "minLength": 1,
>>                       "type": "string",
>>                       "error": "KCHVMIF0005E"
>>                   },
>> +                "source": {
>> +                    "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",
>> +                    "error": "KCHVMIF0016E"
>> +                },
>> +                "mode": {
>> +                    "description": "Only applicable for macvtap 
>> ifaces 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": "KCHVMIF0017E"
>> +                },
>>                   "model": {
>>                       "description": "model of emulated network 
>> interface card",
>>                       "type": "string",
>> diff --git a/i18n.py b/i18n.py
>> index 90dc67d..6731601 100644
>> --- a/i18n.py
>> +++ b/i18n.py
>> @@ -151,7 +151,7 @@ messages = {
>>
>>       "KCHVMIF0001E": _("Interface %(iface)s does not exist in 
>> virtual machine %(name)s"),
>>       "KCHVMIF0002E": _("Network %(network)s specified for virtual 
>> machine %(name)s does not exist"),
>> -    "KCHVMIF0004E": _("Supported virtual machine interfaces type is 
>> only network"),
>> +    "KCHVMIF0004E": _("Supported virtual machine interfaces type are 
>> network, ovs and macvtap.Type ovs and macvtap are only supported for 
>> s390x/s390 architecture."),
>>       "KCHVMIF0005E": _("Network name for virtual machine interface 
>> must be a string"),
>>       "KCHVMIF0006E": _("Invalid network model card specified for 
>> virtual machine interface"),
>>       "KCHVMIF0007E": _("Specify type and network to add a new 
>> virtual machine interface"),
>> @@ -159,6 +159,13 @@ 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": _("Type macvtap and ovs are only supported on 
>> s390x/s390 architecture."),
>> +    "KCHVMIF0013E": _("Source attribute is only supported on 
>> s390x/s390 architecture."),
>> +    "KCHVMIF0014E": _("If source is provided, only type supported 
>> are macvtap and ovs."),
>> +    "KCHVMIF0015E": _("For type macvtap and ovs, source has to be 
>> provided"),
>> +    "KCHVMIF0016E": _("Source name for virtual machine interface 
>> must be string"),
>> +    "KCHVMIF0017E": _("Invalid source mode. Valid options are: 
>> bridge or vepa."),
>> +
>>
>>       "KCHTMPL0001E": _("Template %(name)s already exists"),
>>       "KCHTMPL0002E": _("Source media %(path)s not found"),
>> diff --git a/model/vmifaces.py b/model/vmifaces.py
>> index 0ec89b6..d2939ba 100644
>> --- a/model/vmifaces.py
>> +++ b/model/vmifaces.py
>> @@ -18,6 +18,7 @@
>>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
>> 02110-1301 USA
>>
>>   import libvirt
>> +import os
>>   import random
>>   from lxml import etree, objectify
>>
>> @@ -42,8 +43,6 @@ class VMIfacesModel(object):
>>
>>       def create(self, vm, params):
>>           conn = self.conn.get()
>> -        networks = conn.listNetworks() + conn.listDefinedNetworks()
>> -        networks = map(lambda x: x.decode('utf-8'), networks)
>>
>>           if params['type'] == 'network':
>>               network = params.get("network")
>> @@ -51,10 +50,43 @@ class VMIfacesModel(object):
>>               if network is None:
>>                   raise MissingParameter('KCHVMIF0007E')
>>
>> +            networks = conn.listNetworks() + conn.listDefinedNetworks()
>> +            networks = map(lambda x: x.decode('utf-8'), networks)
>> +
>>               if network not in networks:
>>                   raise InvalidParameter('KCHVMIF0002E',
>>                                          {'name': vm, 'network': 
>> network})
>>
>> +        # For architecture other than s390x/s390 type ovs/macvtap
>> +        # and source interface are not supported.
>> +        if os.uname()[4] not in ['s390x', 's390']:
>
>> +            if params['type'] == 'ovs' or params['type'] == 'macvtap':
>
> Tip:
>
> if params['type'] in ['ovs', 'macvtap']
>
>> +                raise InvalidParameter('KCHVMIF0012E')
>> +            if params.get('source'):
>> +                raise InvalidParameter('KCHVMIF0013E')
>> +
>> +        # For s390x/s390 architecture
>> +        if os.uname()[4] in ['s390x', 's390']:
>> +            params['name'] = params.get("source", None)
>> +
>
>> +            # For type ovs and mavtap, source interface has to be 
>> provided.
>> +            if params['name'] is None and (params['type'] == 'ovs' or
>> +                                           params['type'] == 
>> 'macvtap'):
>
> Same suggestion here.
>
>> +                raise InvalidParameter('KCHVMIF0015E')
>> +            # If source interface provided, only type supported are ovs
>> +            # and mavtap.
>
>> +            if params['name'] is not None and params['type'] != 
>> 'ovs' and \
>> +               params['type'] != 'macvtap':
>> +                raise InvalidParameter('KCHVMIF0014E')
>
> And here.
>
>> +
>> +            # FIXME: Validation if source interface exists.
>> +            if params['type'] == 'macvtap':
>> +                params['type'] = 'direct'
>> +                params['mode'] = params.get('mode', None)
>> +            elif params['type'] == 'ovs':
>> +                params['type'] = 'bridge'
>> +                params['virtualport_type'] = 'openvswitch'
>> +
>>           macs = (iface.mac.get('address')
>>                   for iface in self.get_vmifaces(vm, self.conn))
>>
>> @@ -83,7 +115,6 @@ class VMIfacesModel(object):
>>               flags |= libvirt.VIR_DOMAIN_AFFECT_CONFIG
>>           if DOM_STATE_MAP[dom.info()[0]] != "shutoff":
>>               flags |= libvirt.VIR_DOMAIN_AFFECT_LIVE
>> -
>>           dom.attachDeviceFlags(xml, flags)
>>
>>           return params['mac']
>> @@ -126,13 +157,28 @@ class VMIfaceModel(object):
>>
>>           info['type'] = iface.attrib['type']
>>           info['mac'] = iface.mac.get('address')
>> -        info['network'] = iface.source.get('network')
>> +
>> +        if iface.attrib.get('virtualport'):
>> +            info['virtualport'] = iface.virtualport.get('type')
>> +
>> +        if info['type'] == 'direct':
>> +            info['source'] = iface.source.get('dev')
>> +            info['mode'] = iface.source.get('mode')
>> +            info['type'] = 'macvtap'
>> +        elif (info['type'] == 'bridge' and
>> +              info.get('virtualport') == 'openvswitch'):
>> +            info['source'] = iface.source.get('bridge')
>> +            info['type'] = 'ovs'
>> +        else:
>> +            info['network'] = iface.source.get('network')
>> +
>>           if iface.find("model") is not None:
>>               info['model'] = iface.model.get('type')
>> -        if info['type'] == 'bridge':
>> +        if info['type'] == 'bridge' and \
>> +           info.get('virtualport') != 'openvswitch':
>>               info['bridge'] = iface.source.get('bridge')
>> -        info['ips'] = self._get_ips(vm, info['mac'], info['network'])
>> -
>> +        if info.get('network'):
>> +            info['ips'] = self._get_ips(vm, info['mac'], 
>> info['network'])
>>           return info
>>
>>       def _get_ips(self, vm, mac, network):
>> diff --git a/xmlutils/interface.py b/xmlutils/interface.py
>> index 4b9fe10..3b8ad91 100644
>> --- a/xmlutils/interface.py
>> +++ b/xmlutils/interface.py
>> @@ -98,6 +98,10 @@ def get_iface_macvtap_xml(params, arch=None, 
>> os_distro=None, os_version=None):
>>
>>       interface.append(E.model(type=model))
>>
>> +    mac = params.get('mac', None)
>> +    if mac is not None:
>> +        interface.append(E.mac(address=mac))
>> +
>>       return ET.tostring(interface, encoding='utf-8', pretty_print=True)
>>
>>
>> @@ -124,4 +128,8 @@ def get_iface_ovs_xml(params, arch=None, 
>> os_distro=None, os_version=None):
>>
>>       interface.append(E.model(type=model))
>>
>> +    mac = params.get('mac', None)
>> +    if mac is not None:
>> +        interface.append(E.mac(address=mac))
>> +
>>       return ET.tostring(interface, encoding='utf-8', pretty_print=True)
>




More information about the Kimchi-devel mailing list