[Kimchi-devel] [PATCH] Backend support for templates with sockets, cores, and threads
Aline Manera
alinefm at linux.vnet.ibm.com
Thu Aug 21 18:22:27 UTC 2014
On 08/18/2014 08:04 PM, Christy Perez wrote:
> 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 at linux.vnet.ibm.com>
> ---
> docs/API.md | 7 +++++++
> src/kimchi/API.json | 28 ++++++++++++++++++++++++++--
> src/kimchi/control/templates.py | 11 ++++++++---
> src/kimchi/osinfo.py | 2 +-
> src/kimchi/vmtemplate.py | 22 ++++++++++++++++++++++
> 5 files changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/docs/API.md b/docs/API.md
> index d75c55f..cb4d541 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -200,6 +200,13 @@ 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.
>
> ### Sub-Collection: Virtual Machine Network Interfaces
>
> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
> index c3fc5e3..d510634 100644
> --- a/src/kimchi/API.json
> +++ b/src/kimchi/API.json
> @@ -26,6 +26,28 @@
> ]
> }
> }
> + },
> + "cpu_info": {
> + "description": "Configure CPU specifics for a VM.",
> + "type": "object",
> + "properties": {
> + "topology": {
> + "description": "Configure the guest CPU topology.",
> + "type": "object",
> + "properties": {
> + "sockets": {
> + "type": "number"
> + },
> + "cores": {
> + "type": "number"
> + },
> + "threads": {
> + "type": "number"
> + }
> + },
> + "required": [ "sockets", "cores", "threads" ]
Usually we set the required = true for each parameter.
"sockets": {
"type": "number",
"required": true
},
> + }
> + }
> }
> },
> "properties": {
> @@ -438,7 +460,8 @@
> "type": "array",
> "items": { "type": "string" }
> },
> - "graphics": { "$ref": "#/kimchitype/graphics" }
> + "graphics": { "$ref": "#/kimchitype/graphics" },
> + "cpu_info": { "$ref": "#/kimchitype/cpu_info" }
> },
> "additionalProperties": false,
> "error": "KCHAPI0001E"
> @@ -608,7 +631,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..5ef35ce 100644
> --- a/src/kimchi/control/templates.py
> +++ b/src/kimchi/control/templates.py
> @@ -38,13 +38,14 @@ 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,
> + return_data = {
> + 'name': self.ident,
> 'icon': self.info['icon'],
> 'invalid': self.info['invalid'],
> 'os_distro': self.info['os_distro'],
> @@ -56,4 +57,8 @@ def data(self):
> 'storagepool': self.info['storagepool'],
> 'networks': self.info['networks'],
> 'folder': self.info.get('folder', []),
> - 'graphics': self.info['graphics']}
> + 'graphics': self.info['graphics']
> + }
> + if (self.info.get('cpu_info')):
> + return_data['cpu_info'] = self.info['cpu_info']
We should always return the same data set while accessing a template
That way "cpu_info" must always have a value.
> + return return_data
> diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py
> index 1ad353c..8492bb7 100644
> --- a/src/kimchi/osinfo.py
> +++ b/src/kimchi/osinfo.py
> @@ -32,7 +32,7 @@
> 'power': ('ppc', 'ppc64')}
>
>
> -common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024,
> +common_spec = {'cpus': 1, 'cpu_sockets': '1', 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024,
Why are you using a different structure here?
I'd expect to have
{..., 'cpu_info': {cores: .., threads:..., sockets:...}}
And even with this cpu info (with cores, threads, sockets) we will
continue to have the "cpus" data?
> 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide',
> 'cdrom_index': 2, 'mouse_bus': 'ps2'}
>
> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
> index 761bac5..c64e61d 100644
> --- a/src/kimchi/vmtemplate.py
> +++ b/src/kimchi/vmtemplate.py
> @@ -359,6 +359,22 @@ 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 not None:
> + cpu_topo = cpu_info.get('topology')
> + if cpu_topo is not None:
> + return """
> + <cpu>
> + <topology sockets='%(sockets)s'
> + cores='%(cores)s'
> + threads='%(threads)s'/>
> + </cpu>
We are in a movement to use "lxml.builder.E" to build the xml instead of
string.
Could you do it in that way too?
> + """ % cpu_topo
> +
> + return ""
> +
> def to_vm_xml(self, vm_name, vm_uuid, **kwargs):
> params = dict(self.info)
> params['name'] = vm_name
> @@ -371,6 +387,11 @@ def to_vm_xml(self, vm_name, vm_uuid, **kwargs):
> graphics = kwargs.get('graphics')
> params['graphics'] = self._get_graphics_xml(graphics)
> + if params.get('cpu_info') is not None:
> + params['cpu_info'] = self._get_cpu_xml()
> + else:
> + params['cpu_info'] = ''
> +
As this "cpu_info" is set in the "common_spec" for any ISO file, this
information will always exist. Otherwise we have a bug
So you can assume that info will be there for use.
> # Current implementation just allows to create disk in one single
> # storage pool, so we cannot mix the types (scsi volumes vs img file)
> storage_type = self._get_storage_type()
> @@ -401,6 +422,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'/>
More information about the Kimchi-devel
mailing list