[PATCH v2 0/4] issue #548: Hotplug network interfaces
Here is the difference between this and the previous patchset: - Patches 1, 2 and 3 have been added; they're just bug fixes, not exactly related to the issue #548. Patch 4 remains the same. Crístian Deives (4): Parse osinfo.lookup return parameters correctly Handle missing parameter "network" when attaching a NIC Use default network model when attaching a NIC issue #548: Hotplug network interfaces src/kimchi/i18n.py | 1 - src/kimchi/model/vmifaces.py | 57 +++++++++++++++++--------- src/kimchi/xmlutils/interface.py | 15 ++++++- tests/test_model.py | 86 ++++++++++++++++++++-------------------- tests/test_rest.py | 5 +++ 5 files changed, 100 insertions(+), 64 deletions(-) -- 2.1.0
The function "osinfo.lookup" returns the tuple (OS version, OS distro). However, the function VMIfacesModel.create is parsing that tuple as (OS distro, OS version), which is wrong. Use the correct order (OS version, OS distro) of the osinfo.lookup return parameters. Signed-off-by: Crístian Deives <cristiandeives@gmail.com> --- src/kimchi/model/vmifaces.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kimchi/model/vmifaces.py b/src/kimchi/model/vmifaces.py index 0c10fa9..501336c 100644 --- a/src/kimchi/model/vmifaces.py +++ b/src/kimchi/model/vmifaces.py @@ -61,7 +61,7 @@ class VMIfacesModel(object): break os_data = VMModel.vm_get_os_metadata(dom, self.caps.metadata_support) - os_distro, os_version = os_data + os_version, os_distro = os_data xml = get_iface_xml(params, conn.getInfo()[0], os_distro, os_version) dom.attachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) -- 2.1.0
If the user doesn't provide the parameter "network" when attaching a network interface to a virtual machine, the server raises an unexpected exception because it assumes that parameter will always be provided. Check the existence of the parameter "network" instead of assuming it always exists. Signed-off-by: Crístian Deives <cristiandeives@gmail.com> --- src/kimchi/model/vmifaces.py | 15 +++++++++++---- tests/test_rest.py | 5 +++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/kimchi/model/vmifaces.py b/src/kimchi/model/vmifaces.py index 501336c..6a76d2d 100644 --- a/src/kimchi/model/vmifaces.py +++ b/src/kimchi/model/vmifaces.py @@ -22,7 +22,8 @@ import random import libvirt from lxml import etree, objectify -from kimchi.exception import InvalidOperation, InvalidParameter, NotFoundError +from kimchi.exception import InvalidOperation, InvalidParameter +from kimchi.exception import MissingParameter, NotFoundError from kimchi.model.config import CapabilitiesModel from kimchi.model.vms import DOM_STATE_MAP, VMModel from kimchi.xmlutils.interface import get_iface_xml @@ -44,9 +45,15 @@ class VMIfacesModel(object): networks = conn.listNetworks() + conn.listDefinedNetworks() networks = map(lambda x: x.decode('utf-8'), networks) - if params["type"] == "network" and params["network"] not in networks: - raise InvalidParameter("KCHVMIF0002E", - {'name': vm, 'network': params["network"]}) + if params['type'] == 'network': + network = params.get("network") + + if network is None: + raise MissingParameter('KCHVMIF0007E') + + if network not in networks: + raise InvalidParameter('KCHVMIF0002E', + {'name': vm, 'network': network}) dom = VMModel.get_vm(vm, self.conn) if DOM_STATE_MAP[dom.info()[0]] != "shutoff": diff --git a/tests/test_rest.py b/tests/test_rest.py index 16ff41d..5788886 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -762,6 +762,11 @@ class RestTests(unittest.TestCase): self.assertEquals(get_template_default('old', 'nic_model'), res['model']) + # try to attach an interface without specifying 'model' + req = json.dumps({'type': 'network'}) + resp = self.request('/vms/test-vm/ifaces', req, 'POST') + self.assertEquals(400, resp.status) + # attach network interface to vm req = json.dumps({"type": "network", "network": "test-network", -- 2.1.0
When attaching a network interface to a virtual machine, Kimchi lets libvirt choose the network model. However, there is an existing lookup table which can determine the best network model for each type of guest operating system, and Kimchi should use that information here as well. Use the osinfo lookup table to determine the best possible network interface model when attaching a NIC to a VM. Signed-off-by: Crístian Deives <cristiandeives@gmail.com> --- src/kimchi/xmlutils/interface.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/kimchi/xmlutils/interface.py b/src/kimchi/xmlutils/interface.py index 7b79584..2325eb9 100644 --- a/src/kimchi/xmlutils/interface.py +++ b/src/kimchi/xmlutils/interface.py @@ -22,6 +22,8 @@ import lxml.etree as ET from distutils.version import LooseVersion from lxml.builder import E +from kimchi import osinfo + def get_iface_xml(params, arch=None, os_distro=None, os_version=None): """ @@ -33,8 +35,17 @@ def get_iface_xml(params, arch=None, os_distro=None, os_version=None): interface = E.interface(type=params['type']) interface.append(E.source(network=params['network'])) - if 'model' in params.keys(): - interface.append(E.model(type=params['model'])) + model = params.get('model') + + # 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') + + # 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)) mac = params.get('mac', None) if mac is not None: -- 2.1.0
Currently, network interfaces can only be added to shutoff VMs. However, the use might want to attach an interface to a running VM as well. Allow network interfaces to be attached also while the VM is running. The related test cases have been updated to perform the same operations on both a shutoff and a running VM. Fix issue #548 (Kimchi does not support NIC hot plug) - backend only. Signed-off-by: Crístian Deives <cristiandeives@gmail.com> --- src/kimchi/i18n.py | 1 - src/kimchi/model/vmifaces.py | 46 +++++++++++++++--------- tests/test_model.py | 86 ++++++++++++++++++++++---------------------- 3 files changed, 73 insertions(+), 60 deletions(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index c8986cf..18e84bc 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -122,7 +122,6 @@ 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"), - "KCHVMIF0003E": _("Do not support guest interface hot plug attachment"), "KCHVMIF0004E": _("Supported virtual machine interfaces type is only network"), "KCHVMIF0005E": _("Network name for virtual machine interface must be a string"), "KCHVMIF0006E": _("Invalid network model card specified for virtual machine interface"), diff --git a/src/kimchi/model/vmifaces.py b/src/kimchi/model/vmifaces.py index 6a76d2d..b65c447 100644 --- a/src/kimchi/model/vmifaces.py +++ b/src/kimchi/model/vmifaces.py @@ -22,8 +22,7 @@ import random import libvirt from lxml import etree, objectify -from kimchi.exception import InvalidOperation, InvalidParameter -from kimchi.exception import MissingParameter, NotFoundError +from kimchi.exception import InvalidParameter, MissingParameter, NotFoundError from kimchi.model.config import CapabilitiesModel from kimchi.model.vms import DOM_STATE_MAP, VMModel from kimchi.xmlutils.interface import get_iface_xml @@ -55,10 +54,6 @@ class VMIfacesModel(object): raise InvalidParameter('KCHVMIF0002E', {'name': vm, 'network': network}) - dom = VMModel.get_vm(vm, self.conn) - if DOM_STATE_MAP[dom.info()[0]] != "shutoff": - raise InvalidOperation("KCHVMIF0003E") - macs = (iface.mac.get('address') for iface in self.get_vmifaces(vm, self.conn)) @@ -67,10 +62,19 @@ class VMIfacesModel(object): if params['mac'] not in macs: break + dom = VMModel.get_vm(vm, self.conn) + os_data = VMModel.vm_get_os_metadata(dom, self.caps.metadata_support) os_version, os_distro = os_data xml = get_iface_xml(params, conn.getInfo()[0], os_distro, os_version) - dom.attachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) + + 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 + + dom.attachDeviceFlags(xml, flags) return params['mac'] @@ -125,14 +129,16 @@ class VMIfaceModel(object): dom = VMModel.get_vm(vm, self.conn) iface = self._get_vmiface(vm, mac) - if DOM_STATE_MAP[dom.info()[0]] != "shutoff": - raise InvalidOperation("KCHVMIF0003E") - if iface is None: raise NotFoundError("KCHVMIF0001E", {'name': vm, 'iface': mac}) - dom.detachDeviceFlags(etree.tostring(iface), - libvirt.VIR_DOMAIN_AFFECT_CURRENT) + 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 + + dom.detachDeviceFlags(etree.tostring(iface), flags) def update(self, vm, mac, params): dom = VMModel.get_vm(vm, self.conn) @@ -141,16 +147,22 @@ class VMIfaceModel(object): if iface is None: raise NotFoundError("KCHVMIF0001E", {'name': vm, 'iface': mac}) - # FIXME we will support to change the live VM configuration later. + 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=libvirt.VIR_DOMAIN_AFFECT_CONFIG) - # change on the persisted VM configuration only. - if 'model' in params and dom.isPersistent(): + dom.updateDeviceFlags(xml, flags=flags) + + if 'model' in params: iface.model.attrib["type"] = params['model'] xml = etree.tostring(iface) - dom.updateDeviceFlags(xml, flags=libvirt.VIR_DOMAIN_AFFECT_CONFIG) + + dom.updateDeviceFlags(xml, flags=flags) return mac diff --git a/tests/test_model.py b/tests/test_model.py index f305a3c..e66d974 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -317,9 +317,6 @@ class ModelTests(unittest.TestCase): params = {'name': 'test', 'disks': [], 'cdrom': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') - params = {'name': 'kimchi-ifaces', 'template': '/templates/test'} - inst.vms_create(params) - rollback.prependDefer(inst.vm_delete, 'kimchi-ifaces') # Create a network net_name = 'test-network' @@ -331,45 +328,50 @@ class ModelTests(unittest.TestCase): inst.network_activate(net_name) rollback.prependDefer(inst.network_deactivate, net_name) - ifaces = inst.vmifaces_get_list('kimchi-ifaces') - self.assertEquals(1, len(ifaces)) - - iface = inst.vmiface_lookup('kimchi-ifaces', ifaces[0]) - self.assertEquals(17, len(iface['mac'])) - self.assertEquals("default", iface['network']) - self.assertIn("model", iface) - - # attach network interface to vm - iface_args = {"type": "network", - "network": "test-network", - "model": "virtio"} - mac = inst.vmifaces_create('kimchi-ifaces', iface_args) - # detach network interface from vm - rollback.prependDefer(inst.vmiface_delete, 'kimchi-ifaces', mac) - self.assertEquals(17, len(mac)) - - iface = inst.vmiface_lookup('kimchi-ifaces', mac) - self.assertEquals("network", iface["type"]) - self.assertEquals("test-network", iface['network']) - self.assertEquals("virtio", iface["model"]) - - # attach network interface to vm without providing model - iface_args = {"type": "network", - "network": "test-network"} - mac = inst.vmifaces_create('kimchi-ifaces', iface_args) - rollback.prependDefer(inst.vmiface_delete, 'kimchi-ifaces', mac) - - iface = inst.vmiface_lookup('kimchi-ifaces', mac) - self.assertEquals("network", iface["type"]) - self.assertEquals("test-network", iface['network']) - - # update vm interface - iface_args = {"network": "default", - "model": "e1000"} - inst.vmiface_update('kimchi-ifaces', mac, iface_args) - iface = inst.vmiface_lookup('kimchi-ifaces', mac) - self.assertEquals("default", iface['network']) - self.assertEquals("e1000", iface["model"]) + for vm_name in ['kimchi-ifaces', 'kimchi-ifaces-running']: + params = {'name': vm_name, 'template': '/templates/test'} + inst.vms_create(params) + rollback.prependDefer(inst.vm_delete, vm_name) + + ifaces = inst.vmifaces_get_list(vm_name) + self.assertEquals(1, len(ifaces)) + + iface = inst.vmiface_lookup(vm_name, ifaces[0]) + self.assertEquals(17, len(iface['mac'])) + self.assertEquals("default", iface['network']) + self.assertIn("model", iface) + + # attach network interface to vm + iface_args = {"type": "network", + "network": "test-network", + "model": "virtio"} + mac = inst.vmifaces_create(vm_name, iface_args) + # detach network interface from vm + rollback.prependDefer(inst.vmiface_delete, vm_name, mac) + self.assertEquals(17, len(mac)) + + iface = inst.vmiface_lookup(vm_name, mac) + self.assertEquals("network", iface["type"]) + self.assertEquals("test-network", iface['network']) + self.assertEquals("virtio", iface["model"]) + + # attach network interface to vm without providing model + iface_args = {"type": "network", + "network": "test-network"} + mac = inst.vmifaces_create(vm_name, iface_args) + rollback.prependDefer(inst.vmiface_delete, vm_name, mac) + + iface = inst.vmiface_lookup(vm_name, mac) + self.assertEquals("network", iface["type"]) + self.assertEquals("test-network", iface['network']) + + # update vm interface + iface_args = {"network": "default", + "model": "e1000"} + inst.vmiface_update(vm_name, mac, iface_args) + iface = inst.vmiface_lookup(vm_name, mac) + self.assertEquals("default", iface['network']) + self.assertEquals("e1000", iface["model"]) @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_disk(self): -- 2.1.0
participants (2)
-
Aline Manera -
Crístian Deives