[Kimchi-devel] [PATCH] Backend support for templates with sockets, cores, and threads
Christy Perez
christy at linux.vnet.ibm.com
Tue Sep 2 22:56:35 UTC 2014
Aline,
A question for you (inline)...
On 08/25/2014 02:08 PM, Christy Perez wrote:
>
>
> On 08/21/2014 01:22 PM, Aline Manera wrote:
>>
>> 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
>> },
>>
> I've changed it to that.
>>
>>
>>> + }
>>> + }
>>> }
>>> },
>>> "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.
How would you suggest we handle this then? If it's not set, we can't
return 0's or 1's.
All 0's won't work:
# virsh edit domainname
error: XML error: Invalid CPU topology
Failed. Try again? [y,n,f,?]:
All 1's as a default but a change in the vcpus would lead to an error:
# virsh edit domainname
error: Maximum CPUs greater than topology limit
Failed. Try again? [y,n,f,?]:
>>
>>
>>> + 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:...}}
> The structure was like that already. I just put in the new value for
> sockets. I didn't look too deeply into how the rest of the code uses the
> osifno spec information, but I left it as-is so as not to break any
> current functionality.
>
>>
>> And even with this cpu info (with cores, threads, sockets) we will
>> continue to have the "cpus" data?
> Yes. With all 1's as the default, the cpus can be left as-is.
>
> I'm adding a check in the vmtemplates code to make sure that this value
> is always a product of the three values.
>
> When we add in some intelligence, we can set the vcpu to be a multiple.
>
>>
>>> '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?
> Sure! I was wondering about that. I just stuck with how it was being
> done everywhere else, but I like that method.
>>
>>> + """ % 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.
> ACK. Thanks.
>
>>
>>> # 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'/>
>>
Regards,
- Christy
More information about the Kimchi-devel
mailing list