[Kimchi-devel] [PATCH v6 1/2] Update VCPU by using libvirt function
Aline Manera
alinefm at linux.vnet.ibm.com
Fri Oct 16 13:54:15 UTC 2015
Sorry about the delay....
Comments inline:
On 06/10/2015 09:53, Jose Ricardo Ziviani wrote:
> From: Crístian Deives <cristiandeives at gmail.com>
>
> Currently, the VCPU count is updated by editing the configuration XML
> (tag: <vcpu>). However, in some cases, editing that tag will only update
> the maximum VCPU count, not the current one. For example, if the VM has
> the following XML tag:
>
> <vcpu current='2'>8</vcpu>
>
> and the user updates the VCPU value to 10, Kimchi will update the XML
> tag to:
>
> <vcpu current='2'>10</vcpu>
>
> which is probably not what the user wanted.
>
> Use the libvirt function "setVcpusFlags" to update the current and the
> maximum VCPU values to the one specified by the user.
>
> Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
> Signed-off-by: Crístian Deives <cristiandeives at gmail.com>
> Signed-off-by: Jose Ricardo Ziviani <joserz at linux.vnet.ibm.com>
> ---
> src/wok/plugins/kimchi/i18n.py | 5 ++
> src/wok/plugins/kimchi/mockmodel.py | 49 +++++++---------
> src/wok/plugins/kimchi/model/vms.py | 93 +++++++++++++++++++++++++++++--
> src/wok/plugins/kimchi/tests/test_rest.py | 3 +-
> 4 files changed, 113 insertions(+), 37 deletions(-)
>
> diff --git a/src/wok/plugins/kimchi/i18n.py b/src/wok/plugins/kimchi/i18n.py
> index ea325b8..2207e9e 100644
> --- a/src/wok/plugins/kimchi/i18n.py
> +++ b/src/wok/plugins/kimchi/i18n.py
> @@ -107,6 +107,11 @@ messages = {
> "KCHVM0049E": _("Cannot power off %(name)s. Virtual machine is shut off."),
> "KCHVM0050E": _("Cannot shutdown %(name)s. Virtual machine is shut off."),
> "KCHVM0051E": _("Cannot reset %(name)s. Virtual machine is already shut off."),
> + "KCHVM0052E": _("Unable to update the following parameters while the VM is offline: %(params)s"),
> + "KCHVM0053E": _("Unable to update the following parameters while the VM is online: %(params)s"),
> + "KCHVM0054E": _("VM '%(vm)s' cannot have more than %(cpus)d CPUs. Please update the CPU value when the VM is not running."),
> + "KCHVM0055E": _("VM '%(vm)s' cannot have less than %(cpus)d CPUs. Please update the CPU value when the VM is not running."),
> + "KCHVM0056E": _("Unable to hotplug CPUs. Details: %(err)s"),
>
> "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."),
> "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."),
> diff --git a/src/wok/plugins/kimchi/mockmodel.py b/src/wok/plugins/kimchi/mockmodel.py
> index 0832b20..b55974d 100644
> --- a/src/wok/plugins/kimchi/mockmodel.py
> +++ b/src/wok/plugins/kimchi/mockmodel.py
> @@ -22,6 +22,8 @@ import lxml.etree as ET
> import os
> import random
> import time
> +
> +from collections import defaultdict
> from lxml import objectify
> from lxml.builder import E
>
> @@ -60,10 +62,9 @@ storagevolumes.VALID_RAW_CONTENT = ['dos/mbr boot sector',
>
>
> class MockModel(Model):
> - _mock_vms = {}
> + _mock_vms = defaultdict(list)
> _mock_snapshots = {}
> _XMLDesc = libvirt.virDomain.XMLDesc
> - _defineXML = libvirt.virConnect.defineXML
> _undefineDomain = libvirt.virDomain.undefine
> _libvirt_get_vol_path = LibvirtVMTemplate._get_volume_path
>
> @@ -82,7 +83,6 @@ class MockModel(Model):
>
> cpuinfo.get_topo_capabilities = MockModel.get_topo_capabilities
> vmifaces.getDHCPLeases = MockModel.getDHCPLeases
> - libvirt.virConnect.defineXML = MockModel.domainDefineXML
> libvirt.virDomain.XMLDesc = MockModel.domainXMLDesc
> libvirt.virDomain.undefine = MockModel.undefineDomain
> libvirt.virDomain.attachDeviceFlags = MockModel.attachDeviceFlags
> @@ -125,7 +125,7 @@ class MockModel(Model):
> imageinfo.probe_image = self._probe_image
>
> def reset(self):
> - MockModel._mock_vms = {}
> + MockModel._mock_vms = defaultdict(list)
> MockModel._mock_snapshots = {}
> self._mock_swupdate = MockSoftwareUpdate()
> self._mock_repositories = MockRepositories()
> @@ -160,21 +160,15 @@ class MockModel(Model):
> return ET.fromstring(xml)
>
> @staticmethod
> - def domainDefineXML(conn, xml):
> - name = objectify.fromstring(xml).name.text
> - try:
> - dom = conn.lookupByName(name)
> - if not dom.isActive():
> - MockModel._mock_vms[name] = xml
> - except:
> - pass
> + def domainXMLDesc(dom, flags=0):
> + xml = MockModel._XMLDesc(dom, flags)
> + root = objectify.fromstring(xml)
>
> - return MockModel._defineXML(conn, xml)
> + for dev_xml in MockModel._mock_vms.get(dom.name(), []):
> + dev = objectify.fromstring(dev_xml)
> + root.devices.append(dev)
>
> - @staticmethod
> - def domainXMLDesc(dom, flags=0):
> - return MockModel._mock_vms.get(dom.name(),
> - MockModel._XMLDesc(dom, flags))
> + return ET.tostring(root, encoding="utf-8")
>
> @staticmethod
> def undefineDomain(dom):
> @@ -185,12 +179,7 @@ class MockModel(Model):
>
> @staticmethod
> def attachDeviceFlags(dom, xml, flags=0):
> - old_xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)
> - root = objectify.fromstring(old_xml)
> - dev = objectify.fromstring(xml)
> - root.devices.append(dev)
> -
> - MockModel._mock_vms[dom.name()] = ET.tostring(root, encoding="utf-8")
> + MockModel._mock_vms[dom.name()].append(xml)
>
> @staticmethod
> def _get_device_node(dom, xml):
> @@ -216,16 +205,16 @@ class MockModel(Model):
>
> @staticmethod
> def detachDeviceFlags(dom, xml, flags=0):
> - root, dev = MockModel._get_device_node(dom, xml)
> - root.devices.remove(dev)
> -
> - MockModel._mock_vms[dom.name()] = ET.tostring(root, encoding="utf-8")
> + if xml in MockModel._mock_vms[dom.name()]:
> + MockModel._mock_vms[dom.name()].remove(xml)
>
> @staticmethod
> def updateDeviceFlags(dom, xml, flags=0):
> - root, old_dev = MockModel._get_device_node(dom, xml)
> - root.devices.replace(old_dev, objectify.fromstring(xml))
> - MockModel._mock_vms[dom.name()] = ET.tostring(root, encoding="utf-8")
> + _, old_dev = MockModel._get_device_node(dom, xml)
> + old_xml = ET.tostring(old_dev, encoding="utf-8")
> + if old_xml in MockModel._mock_vms[dom.name()]:
> + MockModel._mock_vms[dom.name()].remove(old_xml)
> + MockModel._mock_vms[dom.name()].append(xml)
>
> @staticmethod
> def volResize(vol, size, flags=0):
> diff --git a/src/wok/plugins/kimchi/model/vms.py b/src/wok/plugins/kimchi/model/vms.py
> index a42e8af..9e6a4d7 100644
> --- a/src/wok/plugins/kimchi/model/vms.py
> +++ b/src/wok/plugins/kimchi/model/vms.py
> @@ -44,6 +44,7 @@ from wok.plugins.kimchi import model
> from wok.plugins.kimchi import vnc
> from wok.plugins.kimchi.config import READONLY_POOL_TYPE
> from wok.plugins.kimchi.kvmusertests import UserTests
> +from wok.plugins.kimchi.model.cpuinfo import CPUInfoModel
> from wok.plugins.kimchi.screenshot import VMScreenshot
> from wok.plugins.kimchi.utils import template_name_from_uri
> from wok.plugins.kimchi.xmlutils.cpu import get_cpu_xml, get_numa_xml
> @@ -64,10 +65,15 @@ DOM_STATE_MAP = {0: 'nostate',
> 6: 'crashed',
> 7: 'pmsuspended'}
>
> -VM_STATIC_UPDATE_PARAMS = {'name': './name',
> - 'cpus': './vcpu'}
> +VM_STATIC_UPDATE_PARAMS = {'name': './name'}
> VM_LIVE_UPDATE_PARAMS = {}
>
> +# update parameters which are updatable when the VM is online
> +VM_ONLINE_UPDATE_PARAMS = ['graphics', 'groups', 'memory', 'users']
> +# update parameters which are updatable when the VM is offline
> +VM_OFFLINE_UPDATE_PARAMS = ['cpus', 'graphics', 'groups', 'memory', 'name',
> + 'users']
> +
> XPATH_DOMAIN_DISK = "/domain/devices/disk[@device='disk']/source/@file"
> XPATH_DOMAIN_DISK_BY_FILE = "./devices/disk[@device='disk']/source[@file='%s']"
> XPATH_DOMAIN_NAME = '/domain/name'
> @@ -77,6 +83,7 @@ XPATH_DOMAIN_MAC_BY_ADDRESS = "./devices/interface[@type='network']/"\
> XPATH_DOMAIN_MEMORY = '/domain/memory'
> XPATH_DOMAIN_MEMORY_UNIT = '/domain/memory/@unit'
> XPATH_DOMAIN_UUID = '/domain/uuid'
> +XPATH_DOMAIN_DEV_CPU_ID = '/domain/devices/spapr-cpu-socket/@id'
>
> XPATH_NUMA_CELL = './cpu/numa/cell'
>
> @@ -231,8 +238,20 @@ class VMModel(object):
>
> with lock:
> dom = self.get_vm(name, self.conn)
> - vm_name, dom = self._static_vm_update(name, dom, params)
> +
> + if DOM_STATE_MAP[dom.info()[0]] == 'shutoff':
> + ext_params = set(params.keys()) - set(VM_OFFLINE_UPDATE_PARAMS)
> + if len(ext_params) > 0:
> + raise InvalidParameter('KCHVM0052E',
> + {'params': ', '.join(ext_params)})
> + else:
> + ext_params = set(params.keys()) - set(VM_ONLINE_UPDATE_PARAMS)
> + if len(ext_params) > 0:
> + raise InvalidParameter('KCHVM0053E',
> + {'params': ', '.join(ext_params)})
> +
> self._live_vm_update(dom, params)
> + vm_name, dom = self._static_vm_update(name, dom, params)
> return vm_name
>
> def clone(self, name):
> @@ -757,8 +776,7 @@ class VMModel(object):
> vcpus = params.get('cpus')
> if numa_mem == []:
> if vcpus is None:
> - vcpus = int(xpath_get_text(xml,
> - VM_STATIC_UPDATE_PARAMS['cpus'])[0])
> + vcpus = int(xpath_get_text(xml, 'vcpu')[0])
> cpu = root.find('./cpu')
> if cpu is None:
> cpu = get_cpu_xml(vcpus, params['memory'] << 10)
> @@ -819,6 +837,67 @@ class VMModel(object):
> if 'memory' in params and dom.isActive():
> self._update_memory_live(dom, params)
>
> + if 'cpus' in params:
> + cpus = params['cpus']
> +
> + if DOM_STATE_MAP[dom.info()[0]] == 'shutoff':
> + try:
> + # set maximum VCPU count
> + cpu_model = CPUInfoModel(conn=self.conn)
> + max_vcpus = cpu_model.cores_available *\
> + cpu_model.threads_per_core
> + dom.setVcpusFlags(max_vcpus,
> + libvirt.VIR_DOMAIN_AFFECT_CONFIG |
> + libvirt.VIR_DOMAIN_VCPU_MAXIMUM)
> +
> + # set current VCPU count
> + dom.setVcpusFlags(cpus, libvirt.VIR_DOMAIN_AFFECT_CONFIG)
> + except libvirt.libvirtError, e:
> + raise OperationFailed('KCHVM0008E', {'name': dom.name(),
> + 'err': e.message})
> + else:
> + try:
> + dev_cpu_ids = xpath_get_text(dom.XMLDesc(0),
> + XPATH_DOMAIN_DEV_CPU_ID)
> + vcpu_count = dom.vcpus(libvirt.VIR_DOMAIN_AFFECT_LIVE)
> + total_cpu_count = vcpu_count + len(dev_cpu_ids)
> + new_cpu_count = int(params['cpus'])
> + max_cpu_count = dom.maxVcpus()
> +
> + if new_cpu_count > total_cpu_count: # add CPUs
> + if new_cpu_count > max_cpu_count:
> + raise InvalidOperation('KCHVM0054E',
> + {'cpus': max_cpu_count})
> +
> + if len(dev_cpu_ids) == 0:
> + # there are no CPU devices yet; start from 0
> + dev_id = 0
> + else:
> + # there are some CPU devices;
> + # start from the last ID + 1
> + dev_cpu_ids.sort()
> + dev_id = int(dev_cpu_ids[-1]) + 1
> +
> + for i in xrange(new_cpu_count - total_cpu_count):
> + xml = E('spapr-cpu-socket', id=str(dev_id))
> + dom.attachDevice(etree.tostring(xml))
> + dev_id += 1
> + elif new_cpu_count < total_cpu_count: # remove CPUs
> + if new_cpu_count < vcpu_count:
> + raise InvalidOperation('KCHVM0055E',
> + {'cpus': vcpu_count})
> +
> + # the CPU IDs must be removed in descending order
> + dev_cpu_ids.sort(reverse=True)
> + last_id_idx = total_cpu_count - new_cpu_count
> + to_remove = dev_cpu_ids[:last_id_idx]
> +
> + for dev_id in to_remove:
> + xml = E('spapr-cpu-socket', id=dev_id)
> + dom.detachDevice(etree.tostring(xml))
> + except (libvirt.libvirtError, ValueError), e:
> + raise OperationFailed('KCHVM0056E', {'err': e.message})
> +
This 'else' statement is for CPU hot plug and I don't think you intend
to do that in this patch, right?
Could you remove this piece of code for now?
> def _update_memory_live(self, dom, params):
> # Check if host supports memory device
> if not self.caps.mem_hotplug_support:
> @@ -1017,12 +1096,14 @@ class VMModel(object):
> else:
> memory = info[2] >> 10
>
> + cpu_devs = xpath_get_text(dom.XMLDesc(0), XPATH_DOMAIN_DEV_CPU_ID)
> +
> return {'name': name,
> 'state': state,
> 'stats': res,
> 'uuid': dom.UUIDString(),
> 'memory': memory,
> - 'cpus': info[3],
> + 'cpus': info[3] + len(cpu_devs),
> 'screenshot': screenshot,
> 'icon': icon,
> # (type, listen, port, passwd, passwdValidTo)
> diff --git a/src/wok/plugins/kimchi/tests/test_rest.py b/src/wok/plugins/kimchi/tests/test_rest.py
> index 8cf4bd3..6752457 100644
> --- a/src/wok/plugins/kimchi/tests/test_rest.py
> +++ b/src/wok/plugins/kimchi/tests/test_rest.py
> @@ -156,7 +156,7 @@ class RestTests(unittest.TestCase):
>
> req = json.dumps({'cpus': 3})
> resp = self.request('/plugins/kimchi/vms/vm-1', req, 'PUT')
> - self.assertEquals(200, resp.status)
> + self.assertEquals(400, resp.status)
>
> # Check if there is support to memory hotplug, once vm is running
> resp = self.request('/plugins/kimchi/config/capabilities').read()
> @@ -170,6 +170,7 @@ class RestTests(unittest.TestCase):
>
> req = json.dumps({"graphics": {'passwd': "abcdef"}})
> resp = self.request('/plugins/kimchi/vms/vm-1', req, 'PUT')
> + self.assertEquals(200, resp.status)
> info = json.loads(resp.read())
> self.assertEquals('abcdef', info["graphics"]["passwd"])
> self.assertEquals(None, info["graphics"]["passwdValidTo"])
More information about the Kimchi-devel
mailing list