Re: [Kimchi-devel] [PATCH 1/3] Backend support for templates with sockets, cores, and threads

In order to allow a guest to use SMT/hyperthreading, we should enable passing in of the sockets, cores, and threads values when creating a template. All three values must be specified, as per the topology descr at http://libvirt.org/formatdomain.html#elementsCPU Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com&gt; --- docs/API.md | 13 ++++++++++++- src/kimchi/API.json | 36 ++++++++++++++++++++++++++++++++++-- src/kimchi/control/templates.py | 31 +++++++++++++++++-------------- src/kimchi/i18n.py | 2 ++ src/kimchi/model/templates.py | 32 ++++++++++++++++++++++++++++++++ src/kimchi/osinfo.py | 5 ++--- src/kimchi/vmtemplate.py | 16 ++++++++++++++++ 7 files changed, 115 insertions(+), 20 deletions(-) diff --git a/docs/API.md b/docs/API.md index 92fbbd5..6984649 100644 --- a/docs/API.md +++ b/docs/API.md @@ -194,7 +194,9 @@ Represents a snapshot of the Virtual Machine's primary monitor. * name: The name of the Template. Used to identify the Template in this API * os_distro *(optional)*: The operating system distribution * os_version *(optional)*: The version of the operating system distribution - * cpus *(optional)*: The number of CPUs assigned to the VM. Default is 1. + * cpus *(optional)*: The number of CPUs assigned to the VM. + Default is 1, unlees specifying a cpu topology. In that case, cpus + will default to a product of the topology values (see cpu_info). * memory *(optional)*: The amount of memory assigned to the VM. Default is 1024M. * cdrom *(optional)*: A volume name or URI to an ISO image. @@ -216,6 +218,15 @@ Represents a snapshot of the Virtual Machine's primary monitor. Independent Computing Environments * null: Graphics is disabled or type not supported * listen: The network which the vnc/spice server listens on. + * cpu_info *(optional)*: 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 number of sockets to use. + * cores - The number of cores per socket. + * threads - The number of threads per core. + If specifying both cpus and CPU topology, make sure cpus is + equal to the product of sockets, cores, and threads. ### Sub-Collection: Virtual Machine Network Interfaces diff --git a/src/kimchi/API.json b/src/kimchi/API.json index d9e13f0..a1156d5 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -26,6 +26,36 @@ ] } } + }, + "cpu_info": { + "description": "Configure CPU specifics for a VM.", + "type": "object", + "properties": { + "topology": { + "description": "Configure the guest CPU topology.", + "type": "object", + "properties": { + "sockets": { + "type": "integer", + "required": true, + "minimum": 1, + "error": "KCHTMPL0026E" + }, + "cores": { + "type": "integer", + "required": true, + "minimum": 1, + "error": "KCHTMPL0026E" + }, + "threads": { + "type": "integer", + "required": true, + "minimum": 1, + "error": "KCHTMPL0026E" + } + } + } + } } }, "properties": { @@ -448,7 +478,8 @@ "type": "array", "items": { "type": "string" } }, - "graphics": { "$ref": "#/kimchitype/graphics" } + "graphics": { "$ref": "#/kimchitype/graphics" }, + "cpu_info": { "$ref": "#/kimchitype/cpu_info" } }, "additionalProperties": false, "error": "KCHAPI0001E" @@ -612,7 +643,8 @@ "type": "array", "items": { "type": "string" } }, - "graphics": { "$ref": "#/kimchitype/graphics" } + "graphics": { "$ref": "#/kimchitype/graphics" }, + "cpu_info": { "$ref": "#/kimchitype/cpu_info" } }, "additionalProperties": false, "error": "KCHAPI0001E" diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py index e17fa54..70c9457 100644 --- a/src/kimchi/control/templates.py +++ b/src/kimchi/control/templates.py @@ -38,22 +38,25 @@ def __init__(self, model, ident): self.update_params = ["name", "folder", "icon", "os_distro", "storagepool", "os_version", "cpus", "memory", "cdrom", "disks", "networks", - "graphics"] + "graphics", "cpu_info"] self.uri_fmt = "/templates/%s" self.clone = self.generate_action_handler('clone') @property def data(self): - return {'name': self.ident, - 'icon': self.info['icon'], - 'invalid': self.info['invalid'], - 'os_distro': self.info['os_distro'], - 'os_version': self.info['os_version'], - 'cpus': self.info['cpus'], - 'memory': self.info['memory'], - 'cdrom': self.info.get('cdrom', None), - 'disks': self.info['disks'], - 'storagepool': self.info['storagepool'], - 'networks': self.info['networks'], - 'folder': self.info.get('folder', []), - 'graphics': self.info['graphics']} + return { + 'name': self.ident, + 'icon': self.info['icon'], + 'invalid': self.info['invalid'], + 'os_distro': self.info['os_distro'], + 'os_version': self.info['os_version'], + 'cpus': self.info['cpus'], + 'memory': self.info['memory'], + 'cdrom': self.info.get('cdrom', None), + 'disks': self.info['disks'], + 'storagepool': self.info['storagepool'], + 'networks': self.info['networks'], + 'folder': self.info.get('folder', []), + 'graphics': self.info['graphics'], + 'cpu_info': self.info.get('cpu_info') + } diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 75fb076..74ea98e 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -143,6 +143,8 @@ "KCHTMPL0022E": _("Disk size must be an integer greater than 1GB."), "KCHTMPL0023E": _("Template base image must be a valid local image file"), "KCHTMPL0024E": _("Cannot identify base image %(path)s format"), + "KCHTMPL0025E": _("When specifying CPU topology, VCPUs must be a product of sockets, cores, and threads."), + "KCHTMPL0026E": _("When specifying CPU topology, each element must be an integer greater than zero."), "KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index bb5bd3a..ff1070d 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -48,6 +48,23 @@ def create(self, params): {'filename': iso, 'user': user, 'err': excp}) + cpu_info = params.get('cpu_info') + if cpu_info: + topology = cpu_info.get('topology') + # Check, even though currently only topology + # is supported. + if topology: + sockets = topology['sockets'] + cores = topology['cores'] + threads = topology['threads'] + vcpus = params.get('cpus') + if vcpus is None: + params['cpus'] = sockets * cores * threads + elif vcpus != sockets * cores * threads: + raise InvalidParameter("KCHTMPL0025E") + else: + params['cpu_info'] = dict() + conn = self.conn.get() pool_uri = params.get(u'storagepool', '') if pool_uri: @@ -156,6 +173,10 @@ def update(self, name, params): old_t = self.lookup(name) new_t = copy.copy(old_t) new_t.update(params) + + if not self._validate_updated_cpu_params(new_t): + raise InvalidParameter('KCHTMPL0025E') + ident = name conn = self.conn.get() @@ -187,6 +208,17 @@ def update(self, name, params): raise return ident + def _validate_updated_cpu_params(self, info): + # Note: cpu_info is the parent of topology. cpus is vcpus + vcpus = info['cpus'] + cpu_info = info.get('cpu_info') + # cpu_info will always be at least an empty dict + topology = cpu_info.get('topology') + if topology is None: + return True + return vcpus == topology['sockets'] * topology['cores'] * \ + topology['threads'] + class LibvirtVMTemplate(VMTemplate): def __init__(self, args, scan=False, conn=None): diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index 6ee5e48..0e16b50 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -32,9 +32,8 @@ 'power': ('ppc', 'ppc64')} -common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', - 'cdrom_index': 2, 'mouse_bus': 'ps2'} +common_spec = {'cpus': 1, 'memory': 1024, 'disks': [{'index': 0, 'size': 10}], + 'cdrom_bus': 'ide', 'cdrom_index': 2, 'mouse_bus': 'ps2'} modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio') diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index 5f22db9..18d802b 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -358,6 +358,20 @@ def _get_input_output_xml(self): input_output += sound % self.info return input_output + def _get_cpu_xml(self): + + cpu_info = self.info.get('cpu_info') + if cpu_info is None: + return "" + cpu_topo = cpu_info.get('topology') + if cpu_topo is None: + return "" + return etree.tostring(E.cpu(E.topology( + sockets=str(cpu_topo['sockets']), + cores=str(cpu_topo['cores']), + threads=str(cpu_topo['threads'])))) + + def to_vm_xml(self, vm_name, vm_uuid, **kwargs): params = dict(self.info) params['name'] = vm_name @@ -369,6 +383,7 @@ def to_vm_xml(self, vm_name, vm_uuid, **kwargs): params['qemu-stream-cmdline'] = '' graphics = kwargs.get('graphics') params['graphics'] = self._get_graphics_xml(graphics) + params['cpu_info'] = self._get_cpu_xml() # Current implementation just allows to create disk in one single # storage pool, so we cannot mix the types (scsi volumes vs img file) @@ -400,6 +415,7 @@ def to_vm_xml(self, vm_name, vm_uuid, **kwargs): <uuid>%(uuid)s</uuid> <memory unit='MiB'>%(memory)s</memory> <vcpu>%(cpus)s</vcpu> + %(cpu_info)s <os> <type arch='%(arch)s'>hvm</type> <boot dev='hd'/>