[Kimchi-devel] [PATCH V2 3/5] Implement backend code to edit MAC address of a guest

[Aline Manera]

On 13/05/2015 15:39, Jose Ricardo Ziviani wrote:
> Signed-off-by: Jose Ricardo Ziviani <joserz at linux.vnet.ibm.com>
> ---
>   src/kimchi/API.json          | 13 +++-------
>   src/kimchi/i18n.py           |  4 ++-
>   src/kimchi/model/vmifaces.py | 60 +++++++++++++++++++++++++++++++-------------
>   3 files changed, 49 insertions(+), 28 deletions(-)
>
> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
> index 474661c..a2b759b 100644
> --- a/src/kimchi/API.json
> +++ b/src/kimchi/API.json
> @@ -378,19 +378,12 @@
>               "type": "object",
>               "error": "KCHVMIF0008E",
>               "properties": {
> -                "network": {
> -                    "description": "the name of one available network",
> -                    "minLength": 1,
> +                "mac": {
> +                    "description": "Network Interface Card MAC address",
>                       "type": "string",
>                       "error": "KCHVMIF0005E"

You should add the pattern string here so it will be validate using the 
JSON Validator API.

Also the error message should be updated as the 'network' parameter was 
removed.

"KCHVMIF0005E": _("Network name for virtual machine interface must be a string"),


> -                },
> -                "model": {
> -                    "description": "model of emulated network interface card",
> -                    "enum": ["ne2k_pci", "i82551", "i82557b", "i82559er", "rtl8139", "e1000", "pcnet", "virtio", "spapr-vlan"],
> -                    "error": "KCHVMIF0006E"
>                   }
> -            },

As the 'model' parameter will be removed you can also remove the error 
message "KCHVMIF0006E" from i18n.py

> -            "additionalProperties": false

You should keep this configuration which means to do not allow 
additional and non-expected parameters.

> +            }
>           },
>           "templates_create": {
>               "type": "object",
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index 18e84bc..f69e067 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -126,7 +126,9 @@ messages = {
>       "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"),
> -    "KCHVMIF0008E": _("Specify type and network to update a virtual machine interface"),
> +    "KCHVMIF0008E": _("MAC Address must respect this format FF:FF:FF:FF:FF:FF"),
> +    "KCHVMIF0009E": _("MAC Address %(mac)s already exists in virtual machine %(name)s"),
> +    "KCHVMIF0010E": _("Invalid MAC Address %(mac)s"),
>
>       "KCHTMPL0001E": _("Template %(name)s already exists"),
>       "KCHTMPL0003E": _("Network '%(network)s' specified for template %(template)s does not exist"),
> diff --git a/src/kimchi/model/vmifaces.py b/src/kimchi/model/vmifaces.py
> index 8bf6b3d..4c0a2a9 100644
> --- a/src/kimchi/model/vmifaces.py
> +++ b/src/kimchi/model/vmifaces.py
> @@ -20,6 +20,7 @@
>   import random
>
>   import libvirt
> +import re
>   from lxml import etree, objectify
>
>   from kimchi.exception import InvalidParameter, MissingParameter, NotFoundError
> @@ -27,6 +28,8 @@ from kimchi.model.config import CapabilitiesModel
>   from kimchi.model.vms import DOM_STATE_MAP, VMModel
>   from kimchi.xmlutils.interface import get_iface_xml

> +RE_MACADDR = '^([0-9a-fA-F]{2}:){5}[0-9a-fA-F]{2}$'
> +

Move this to API.json with the appropriated error code.

>   class VMIfacesModel(object):
>       def __init__(self, **kargs):
> @@ -57,10 +60,24 @@ class VMIfacesModel(object):
>           macs = (iface.mac.get('address')
>                   for iface in self.get_vmifaces(vm, self.conn))
>
> -        while True:
> -            params['mac'] = VMIfacesModel.random_mac()
> -            if params['mac'] not in macs:
> -                break
> +        # user defined customized mac address
> +        if 'mac' in params and params['mac']:
> +            # make sure it is unique
> +            if params['mac'] in macs:
> +                raise InvalidParameter('KCHVMIF0009E',
> +                                       {'name': vm, 'mac': params['mac']})
> +


> +            # invalid mac address
> +            if not re.match(RE_MACADDR, params['mac']):
> +                raise InvalidParameter('KCHVMIF0010E',
> +                                       {'name': vm, 'mac': params['mac']})
> +

This verification will be done by API.json

> +        # otherwise choose a random mac address
> +        else:
> +            while True:
> +                params['mac'] = VMIfacesModel.random_mac()
> +                if params['mac'] not in macs:
> +                    break
>
>           dom = VMModel.get_vm(vm, self.conn)
>
> @@ -147,22 +164,31 @@ class VMIfaceModel(object):
>           if iface is None:
>               raise NotFoundError("KCHVMIF0001E", {'name': vm, 'iface': mac})
>
> +        # mac address is a required parameter
> +        if 'mac' not in params:
> +            raise MissingParameter('KCHVMIF0008E')
> +

> +        # invalid mac address
> +        if not re.match(RE_MACADDR, params['mac']):
> +            raise InvalidParameter('KCHVMIF0010E',
> +                                   {'name': vm, 'mac': params['mac']})
> +

Same here.

> +        # new mac address must be unique
> +        if self._get_vmiface(vm, params['mac']):
> +            raise InvalidParameter('KCHVMIF0009E',
> +                                   {'name': vm, 'mac': params['mac']})
> +
>           flags = 0
>           if dom.isPersistent():
>               flags |= libvirt.VIR_DOMAIN_AFFECT_CONFIG
> -        if DOM_STATE_MAP[dom.info()[0]] != "shutoff":
> -            flags |= libvirt.VIR_DOMAIN_AFFECT_LIVE
> -
> -        if iface.attrib['type'] == 'network' and 'network' in params:
> -            iface.source.attrib['network'] = params['network']
> -            xml = etree.tostring(iface)
> -
> -            dom.updateDeviceFlags(xml, flags=flags)
>
> -        if 'model' in params:
> -            iface.model.attrib["type"] = params['model']
> -            xml = etree.tostring(iface)
> +        # remove the current nic
> +        xml = etree.tostring(iface)
> +        dom.detachDeviceFlags(xml, flags=flags)
>
> -            dom.updateDeviceFlags(xml, flags=flags)
> +        # add the nic with the desired mac address
> +        iface.mac.attrib['address'] = params['mac']
> +        xml = etree.tostring(iface)
> +        dom.attachDeviceFlags(xml, flags=flags)
>
> -        return mac
> +        return [vm, params['mac']]