[Kimchi-devel] [PATCH] [Kimchi 2/3] Add maxvcpus attribute to guests
Paulo Ricardo Paz Vital
pvital at linux.vnet.ibm.com
Fri Jan 29 16:58:05 UTC 2016
On 01/27/2016 07:14 PM, Lucio Correia wrote:
> - Enable update of maxvcpus, vcpus and CPU topology
> - Enable vCPU cold plug when topology is defined
> - Add cpu_info validation to guest update
> - Simplify _get_cpu_xml() code
> - Remove change_numa verification
> - Update docs and APIs with maxvcpus attribute
>
> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
> ---
> API.json | 7 ++++
> docs/API.md | 17 +++++++-
> i18n.py | 1 -
> model/vms.py | 128 +++++++++++++++++++++++++++++++++++++---------------------
> vmtemplate.py | 30 ++++----------
> 5 files changed, 110 insertions(+), 73 deletions(-)
>
> diff --git a/API.json b/API.json
> index 4876cc0..a3d99f8 100644
> --- a/API.json
> +++ b/API.json
> @@ -299,12 +299,19 @@
> }
> }
> },
> + "cpu_info": { "$ref": "#/kimchitype/cpu_info" },
> "cpus": {
> "description": "The new number of virtual CPUs for the VM",
> "type": "integer",
> "minimum": 1,
> "error": "KCHTMPL0012E"
> },
> + "maxvcpus": {
> + "description": "The maximum number of virtual CPUs that can be assigned to the VM",
> + "type": "integer",
> + "minimum": 1,
> + "error": "KCHTMPL0012E"
> + },
> "memory": {
> "description": "The new amount (MB) of memory for the VM",
> "type": "integer",
> diff --git a/docs/API.md b/docs/API.md
> index 4790d08..8dbc27c 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -118,6 +118,7 @@ server.
> * uuid: UUID of the VM.
> * memory: The amount of memory assigned to the VM (in MB)
> * cpus: The number of CPUs assigned to the VM
> + * maxvcpus: The maximum number of CPUs that can be assigned to the VM
Same as I asked on the first patch.
In API.json you said that maxvcpu is "The maximum number of virtual CPUs
that can be assigned to the VMs..." and here looks like you missed the
*virtual* word. Is it correct?
> * screenshot: A link to a recent capture of the screen in PNG format
> * icon: A link to an icon that represents the VM
> * graphics: A dict to show detail of VM graphics.
> @@ -138,12 +139,13 @@ server.
> * groups: A list of system groups whose users have permission to access
> the VM. Default is: empty (i.e. no groups given access).
> * **DELETE**: Remove the Virtual Machine
> -* **PUT**: update the parameters of existed VM
> +* **PUT**: update the parameters of existing VM
> * name: New name for this VM (only applied for shutoff VM)
> * users: New list of system users.
> * groups: New list of system groups.
> - * cpus: New number of virtual cpus for this VM (if VM is running, new value
> + * cpus: New number of CPUs for this VM (if VM is running, new value
> will take effect in next reboot)
> + * maxvcpus: The maximum number of CPUs that can be assigned to the VM
Same here.
> * memory: New amount of memory (MB) for this VM (if VM is running, new
> value will take effect in next reboot)
> * graphics: A dict to show detail of VM graphics.
> @@ -152,6 +154,17 @@ server.
> * passwdValidTo *(optional)*: lifetime for the console password. When
> omitted the password will be valid just
> for 30 seconds.
> + * cpu_info: CPU-specific information.
> + * topology: Specify sockets, threads, and cores to run the virtual CPU
> + threads on.
> + All three are required in order to specify cpu topology.
> + * sockets - The maximum number of sockets to use.
> + * cores - The number of cores per socket.
> + * threads - The number of threads per core.
> + If CPU topology is specified, *cpus* and *maxvcpus* also must be
> + specified. Make sure *maxvcpus* is equal to the product of sockets,
> + cores and threads, and *cpus* is a multiple of a product of cores
> + and threads.
>
> * **POST**: *See Virtual Machine Actions*
>
> diff --git a/i18n.py b/i18n.py
> index bb6f3d1..59870e8 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -130,7 +130,6 @@ messages = {
> "KCHVM0071E": _("Memory value %(mem)s must be aligned to %(alignment)sMiB."),
> "KCHVM0073E": _("Unable to update the following parameters while the VM is offline: %(params)s"),
> "KCHVM0074E": _("Unable to update the following parameters while the VM is online: %(params)s"),
> - "KCHVM0075E": _("Cannot change VCPU value because '%(vm)s' has a topology defined - sockets: %(sockets)s, cores: %(cores)s, threads: %(threads)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/model/vms.py b/model/vms.py
> index 9aaf530..fa3bcbb 100644
> --- a/model/vms.py
> +++ b/model/vms.py
> @@ -41,7 +41,7 @@ from wok.model.tasks import TaskModel
> from wok.rollbackcontext import RollbackContext
> from wok.utils import add_task, convert_data_size, get_next_clone_name
> from wok.utils import import_class, run_setfacl_set_attr, run_command, wok_log
> -from wok.xmlutils.utils import xpath_get_text, xml_item_update
> +from wok.xmlutils.utils import xpath_get_text, xml_item_remove, xml_item_update
> from wok.xmlutils.utils import dictize
>
> from wok.plugins.kimchi import model
> @@ -72,15 +72,12 @@ DOM_STATE_MAP = {0: 'nostate',
> 6: 'crashed',
> 7: 'pmsuspended'}
>
> -VM_STATIC_UPDATE_PARAMS = {'name': './name', 'cpus': './vcpu'}
> -
> -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']
> +VM_OFFLINE_UPDATE_PARAMS = ['cpus', 'cpu_info', 'graphics', 'groups',
> + 'maxvcpus', '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']"
> @@ -93,8 +90,10 @@ XPATH_DOMAIN_MEMORY_UNIT = '/domain/memory/@unit'
> XPATH_DOMAIN_UUID = '/domain/uuid'
> XPATH_DOMAIN_DEV_CPU_ID = '/domain/devices/spapr-cpu-socket/@id'
>
> +XPATH_NAME = './name'
> XPATH_NUMA_CELL = './cpu/numa/cell'
> XPATH_TOPOLOGY = './cpu/topology'
> +XPATH_VCPU = './vcpu'
>
> # key: VM name; value: lock object
> vm_locks = {}
> @@ -285,17 +284,6 @@ class VMModel(object):
> raise InvalidParameter('KCHVM0074E',
> {'params': ', '.join(ext_params)})
>
> - if 'cpus' in params and DOM_STATE_MAP[dom.info()[0]] == 'shutoff':
> - # user cannot change vcpu if topology is defined.
> - curr_vcpu = dom.vcpusFlags(libvirt.VIR_DOMAIN_AFFECT_CURRENT)
> - if self.has_topology(dom) and curr_vcpu != params['cpus']:
> - raise InvalidOperation(
> - 'KCHVM0075E',
> - {'vm': dom.name(),
> - 'sockets': self.get_vm_sockets(dom),
> - 'cores': self.get_vm_cores(dom),
> - 'threads': self.get_vm_threads(dom)})
> -
> self._live_vm_update(dom, params)
> vm_name, dom = self._static_vm_update(name, dom, params)
> return vm_name
> @@ -751,6 +739,8 @@ class VMModel(object):
> old_xml = new_xml = dom.XMLDesc(0)
>
> params = copy.deepcopy(params)
> +
> + # Update name
> name = params.get('name')
> nonascii_name = None
> if name is not None:
> @@ -760,37 +750,42 @@ class VMModel(object):
> raise InvalidParameter("KCHVM0003E", msg_args)
>
> params['name'], nonascii_name = get_ascii_nonascii_name(name)
> + new_xml = xml_item_update(new_xml, XPATH_NAME, nonascii_name, None)
> +
> + # Update vcpus
> + cpus = params.get('cpus', None)
> + if cpus:
> + attrib = 'current'
> + new_xml = xml_item_update(new_xml, XPATH_VCPU, str(cpus), attrib)
> +
> + # Update maxvcpus
> + maxvcpus = params.get('maxvcpus', None)
> + if maxvcpus:
> + new_xml = xml_item_update(new_xml, XPATH_VCPU, str(maxvcpus), None)
> +
> + # Update topology
> + if 'cpu_info' in params:
> + topology = params.get('cpu_info', {}).get('topology', None)
> +
> + if topology:
> + sockets = str(topology['sockets'])
> + cores = str(topology['cores'])
> + threads = str(topology['threads'])
> +
> + xpath = XPATH_TOPOLOGY
> + new_xml = xml_item_update(new_xml, xpath, sockets, 'sockets')
> + new_xml = xml_item_update(new_xml, xpath, cores, 'cores')
> + new_xml = xml_item_update(new_xml, xpath, threads, 'threads')
> + else:
> + # topology may have been removed
> + new_xml = xml_item_remove(new_xml, xpath)
>
> - for key, val in params.items():
> - change_numa = True
> - if key in VM_STATIC_UPDATE_PARAMS:
> - if type(val) == int:
> - val = str(val)
> - xpath = VM_STATIC_UPDATE_PARAMS[key]
> - attrib = None
> - if key == 'cpus':
> - if self.has_topology(dom) or dom.isActive():
> - change_numa = False
> - continue
> - # Update maxvcpu firstly
> - new_xml = xml_item_update(new_xml, xpath,
> - str(self._get_host_maxcpu()),
> - attrib)
> - # Update current vcpu
> - attrib = 'current'
> - new_xml = xml_item_update(new_xml, xpath, val, attrib)
> + # Revalidate cpu info - may raise CPUInfo exceptions
> + self._validate_cpu_info(new_xml, dom)
>
> # Updating memory and NUMA if necessary, if vm is offline
> - if not dom.isActive():
> - if 'memory' in params:
> - new_xml = self._update_memory_config(new_xml, params)
> - elif 'cpus' in params and change_numa and \
> - (xpath_get_text(new_xml, XPATH_NUMA_CELL + '/@memory') != []):
> - new_xml = xml_item_update(
> - new_xml,
> - XPATH_NUMA_CELL,
> - value='0',
> - attr='cpus')
> + if 'memory' in params and not dom.isActive():
> + new_xml = self._update_memory_config(new_xml, params)
>
> if 'graphics' in params:
> new_xml = self._update_graphics(dom, new_xml, params)
> @@ -928,6 +923,27 @@ class VMModel(object):
>
> return ET.tostring(root, encoding="utf-8")
>
> + def _validate_cpu_info(self, new_xml, dom):
> + topology = None
> + if self.has_topology(dom):
> + sockets = xpath_get_text(new_xml, XPATH_TOPOLOGY + '/@sockets')[0]
> + cores = xpath_get_text(new_xml, XPATH_TOPOLOGY + '/@cores')[0]
> + threads = xpath_get_text(new_xml, XPATH_TOPOLOGY + '/@threads')[0]
> + topology = {
> + 'sockets': int(sockets),
> + 'cores': int(cores),
> + 'threads': int(threads),
> + }
> +
> + params = {
> + 'maxvcpus': int(xpath_get_text(new_xml, 'vcpu')[0]),
> + 'cpus': int(xpath_get_text(new_xml, './vcpu/@current')[0]),
> + 'cpu_info': {'topology': topology}
> + }
> +
> + cpu_model = CPUInfoModel(conn=self.conn)
> + cpu_model.check_cpu_info(params)
> +
> def _live_vm_update(self, dom, params):
> self._vm_update_access_metadata(dom, params)
> if 'memory' in params and dom.isActive():
> @@ -1140,8 +1156,24 @@ class VMModel(object):
> res['io_throughput_peak'] = vm_stats.get('max_disk_io', 100)
> users, groups = self._get_access_info(dom)
>
> + xml = dom.XMLDesc(0)
> + maxvcpus = int(xpath_get_text(xml, XPATH_VCPU)[0])
> +
> + cpu_info = None
> + if self.has_topology(dom):
> + sockets = int(xpath_get_text(xml, XPATH_TOPOLOGY + '/@sockets')[0])
> + cores = int(xpath_get_text(xml, XPATH_TOPOLOGY + '/@cores')[0])
> + threads = int(xpath_get_text(xml, XPATH_TOPOLOGY + '/@threads')[0])
> +
> + cpu_info = {
> + 'topology': {
> + 'sockets': sockets,
> + 'cores': cores,
> + 'threads': threads,
> + }
> + }
> +
> if state == 'shutoff':
> - xml = dom.XMLDesc(0)
> val = xpath_get_text(xml, XPATH_DOMAIN_MEMORY)[0]
> unit_list = xpath_get_text(xml, XPATH_DOMAIN_MEMORY_UNIT)
> if len(unit_list) > 0:
> @@ -1158,6 +1190,8 @@ class VMModel(object):
> 'uuid': dom.UUIDString(),
> 'memory': memory,
> 'cpus': info[3],
> + 'cpu_info': cpu_info,
> + 'maxvcpus': maxvcpus,
> 'screenshot': screenshot,
> 'icon': icon,
> # (type, listen, port, passwd, passwdValidTo)
> diff --git a/vmtemplate.py b/vmtemplate.py
> index 2f524a7..e1571b0 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -324,12 +324,9 @@ class VMTemplate(object):
>
> def _get_cpu_xml(self):
> # Include CPU topology, if provided
> - cpu_info = self.info.get('cpu_info')
> - if cpu_info is not None:
> - cpu_topo = cpu_info.get('topology')
> - return get_cpu_xml(0,
> - self.info.get('memory') << 10,
> - cpu_topo)
> + cpu_topo = self.info.get('cpu_info', {}).get('topology', None)
> +
> + return get_cpu_xml(0, self.info.get('memory') << 10, cpu_topo)
>
> def _get_max_memory(self, guest_memory):
> # Setting maxMemory of the VM, which will be lesser value between:
> @@ -371,8 +368,6 @@ class VMTemplate(object):
> libvirt_stream_protocols = kwargs.get('libvirt_stream_protocols', [])
> cdrom_xml = self._get_cdrom_xml(libvirt_stream_protocols)
>
> - max_vcpus = kwargs.get('max_vcpus', 1)
> -
> if not urlparse.urlparse(self.info.get('cdrom', "")).scheme in \
> libvirt_stream_protocols and \
> params.get('iso_stream', False):
> @@ -402,22 +397,11 @@ class VMTemplate(object):
> # set a hard limit using max_memory + 1GiB
> params['hard_limit'] = params['max_memory'] + (1024 << 10)
>
> - cpu_topo = self.info.get('cpu_info').get('topology')
> - if (cpu_topo is not None):
> - sockets = int(max_vcpus / (cpu_topo['cores'] *
> - cpu_topo['threads']))
> - self.info['cpu_info']['topology']['sockets'] = sockets
> + # vcpu element
> + params['vcpus'] = "<vcpu current='%d'>%d</vcpu>" % \
> + (params['cpus'], params['maxvcpus'])
>
> - # Reduce maxvcpu to fit number of sockets if necessary
> - total_max_vcpu = sockets * cpu_topo['cores'] * cpu_topo['threads']
> - if total_max_vcpu != max_vcpus:
> - max_vcpus = total_max_vcpu
> -
> - params['vcpus'] = "<vcpu current='%s'>%d</vcpu>" % \
> - (params['cpus'], max_vcpus)
> - else:
> - params['vcpus'] = "<vcpu current='%s'>%d</vcpu>" % \
> - (params['cpus'], max_vcpus)
> + # cpu_info element
> params['cpu_info'] = self._get_cpu_xml()
>
> xml = """
>
More information about the Kimchi-devel
mailing list