<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 10/13/2014 11:40 AM, Christy Perez
wrote:<br>
</div>
<blockquote cite="mid:543BE45E.50107@linux.vnet.ibm.com" type="cite">
<pre wrap="">
On 10/13/2014 04:57 AM, Royce Lv wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 2014年10月10日 05:20, Christy Perez wrote:
</pre>
<blockquote type="cite">
<pre wrap="">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
<a class="moz-txt-link-freetext" href="http://libvirt.org/formatdomain.html#elementsCPU">http://libvirt.org/formatdomain.html#elementsCPU</a>
v2->v3:
- Set vcpus based on topology, if specified.
- Move the update cpu+topology validation out to a function
for redability
- Add a minimum value of 1 for topology values
- Leave new English error msg as empty string
- Update the API documentation on cpu defaults
v1->v2:
- Added a check to make sure that vcpus = sockets*cores*threads
- Set individual topoology params to required in API.json
- Change the topology object types from string to integer
- Always return cpu_info from templates lookup()
- Removed check for cpu_info in to_vm_xml
- Build cpu_info xml using lxml.builder instead of string
- CPU and topology verification on template update
Signed-off-by: Christy Perez <a class="moz-txt-link-rfc2396E" href="mailto:christy@linux.vnet.ibm.com"><christy@linux.vnet.ibm.com></a>
---
docs/API.md | 13 ++++++++++-
po/en_US.po | 3 +++
po/pt_BR.po | 3 +++
po/zh_CN.po | 3 +++
src/kimchi/API.json | 33 ++++++++++++++++++++++++--
src/kimchi/control/templates.py | 13 ++++++++---
src/kimchi/i18n.py | 1 +
src/kimchi/model/templates.py | 52
+++++++++++++++++++++++++++++++++++++++++
src/kimchi/osinfo.py | 2 +-
src/kimchi/vmtemplate.py | 15 ++++++++++++
10 files changed, 131 insertions(+), 7 deletions(-)
diff --git a/docs/API.md b/docs/API.md
index cc438cc..7f1a8c2 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -179,7 +179,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
</pre>
</blockquote>
<pre wrap="">unlees-->unless? :)
</pre>
<blockquote type="cite">
<pre wrap="">+ 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.
@@ -201,6 +203,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/po/en_US.po b/po/en_US.po
index eb571ca..a88675c 100644
--- a/po/en_US.po
+++ b/po/en_US.po
@@ -371,6 +371,9 @@ msgstr ""
msgid "Cannot identify base image %(path)s format"
msgstr ""
+msgid "When specifying CPU topology, VCPUs must be a product of
sockets, cores, and threads."
+msgstr ""
+
#, python-format
msgid "Storage pool %(name)s already exists"
msgstr ""
diff --git a/po/pt_BR.po b/po/pt_BR.po
index f2fba64..1bebe30 100644
--- a/po/pt_BR.po
+++ b/po/pt_BR.po
@@ -424,6 +424,9 @@ msgstr "Imagem base do modelo deve ser um arquivo
de imagem local válido"
msgid "Cannot identify base image %(path)s format"
msgstr "Não foi possível identificar o formato da imagem base %(path)s"
+msgid "When specifying CPU topology, VCPUs must be a product of
sockets, cores, and threads."
+msgstr ""
+
#, python-format
msgid "Storage pool %(name)s already exists"
msgstr "Storage pool %(name)s já existe"
diff --git a/po/zh_CN.po b/po/zh_CN.po
index f77e405..f944b8d 100644
--- a/po/zh_CN.po
+++ b/po/zh_CN.po
@@ -392,6 +392,9 @@ msgstr "模板基础镜像必须为一个有效的本地镜像文件"
msgid "Cannot identify base image %(path)s format"
msgstr "未能识别基础镜像%(path)s格式"
+msgid "When specifying CPU topology, VCPUs must be a product of
sockets, cores, and threads."
+msgstr ""
+
#, python-format
msgid "Storage pool %(name)s already exists"
msgstr "存储池%(name)s已经存在"
diff --git a/src/kimchi/API.json b/src/kimchi/API.json
index 5b752dc..fe3df72 100644
--- a/src/kimchi/API.json
+++ b/src/kimchi/API.json
@@ -26,6 +26,33 @@
]
}
}
+ },
+ "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
+ },
+ "cores": {
+ "type": "integer",
+ "required": true,
+ "minimum": 1
+ },
+ "threads": {
+ "type": "integer",
+ "required": true,
+ "minimum": 1
+ }
+ }
+ }
+ }
}
},
"properties": {
@@ -448,7 +475,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 +640,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..54caa92 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,10 @@ 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']
+ else:
+ return_data['cpu_info'] = ''
+ return return_data
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index 1b543ce..9ce8e86 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -131,6 +131,7 @@
"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."),
"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 9278cdc..04d2af9 100644
--- a/src/kimchi/model/templates.py
+++ b/src/kimchi/model/templates.py
@@ -48,6 +48,22 @@ def create(self, params):
{'filename': iso, 'user': user,
'err': excp})
+ cpu_info = params.get('cpu_info')
+ if cpu_info:
+ cpu_info = dict(params['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 sockets * cores * threads != vcpus:
+ raise InvalidParameter("KCHTMPL0025E")
+
conn = self.conn.get()
pool_uri = params.get(u'storagepool', '')
if pool_uri:
@@ -154,6 +170,9 @@ def delete(self, name):
def update(self, name, params):
old_t = self.lookup(name)
+ if not self._verify_updated_cpu_params(params, old_t):
+ raise InvalidParameter('KCHTMPL0025E')
+
new_t = copy.copy(old_t)
new_t.update(params)
ident = name
@@ -187,6 +206,39 @@ def update(self, name, params):
raise
return ident
+ def _verify_updated_cpu_params(self, params, old_t):
+ # Note: cpu_info is the parent of topology. cpus is vcpus
+ # Keep two check_ variables for what we'll verify at the end to
+ # keep from duplicating code.
+ check_cpu = None
+ check_topology = None
+ # First see what parameters were passed in.
+ new_vcpus = params.get('cpus')
+ new_cpu_info = params.get('cpu_info')
+ new_topology = ''
+ if new_cpu_info:
+ new_topology = new_cpu_info.get('topology')
+ # Now figure out what needs to be verified.
+ if new_vcpus and new_topology:
+ check_cpu = new_vcpus
+ check_topology = new_topology
+ elif new_vcpus:
+ old_cpu_info = old_t.get('cpu_info')
+ if old_cpu_info:
+ old_topology = old_cpu_info.get('topology')
+ if old_topology:
+ check_cpu = new_vcpus
+ check_topology = old_topology
+ elif new_topology:
+ check_cpu = old_t.get('cpus')
+ check_topology = new_topology
+ # Now verify the cpu and topoloy parameters
+ if check_cpu and (check_cpu != check_topology['sockets'] * \
+ check_topology['cores'] * check_topology['threads']):
+ return False
+
+ return True
+
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..10d0ab0 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,
</pre>
</blockquote>
<pre wrap="">As we use common_spec to create template, we need to put it in dict
"cpu_info", I suppose put it here won't get expected default value, right?
In osinfo.py:
def lookup(distro, version):
....
if distro in modern_version_bases[arch]:
if LooseVersion(version) >= LooseVersion(
modern_version_bases[arch][distro]):
params.update(template_specs[arch]['modern'])
else:
params.update(template_specs[arch]['old'])
else:
params['os_distro'] = params['os_version'] = "unknown"
params.update(template_specs[arch]['old'])
....
return params
In vmtemplate.py:
...
entry = osinfo.lookup(os_distro, os_version)
self.info.update(entry)
And in def _get_cpu_xml(self), you used
cpu_info = self.info.get('cpu_info')
So we need to define default value in 'cpu_info' dict instead.
</pre>
</blockquote>
<pre wrap="">
I think the default value for this should be to leave it out from the
backend standpoint.
When we implement the UI, I think the default value should be to have
SMT/threads checked, and make the user uncheck it. But, that's another
discussion.
I'm not sure why the cpu_info information was put into the dict like it
was. Maybe we should just take those elements out?</pre>
</blockquote>
<br>
No, those values must be there.<br>
<br>
Let me try to clarify the relation between osinfo.py ->
VMTemplate (vmtemplate.py) -> Template (model/templates.py)<br>
<br>
First, keep in mind that you only need a ISO or Image file to create
a Template.<br>
<br>
When creating a Template, Kimchi will scan the ISO or Image file to
get its OS and OS version.<br>
Based on OS information we use the osinfo.py to get the default
configuration,<br>
<br>
<b>O</b><b>n vmtemplate.py</b> (__init__)<br>
<br>
self.info = {}<br>
entry = osinfo.lookup(os_distro, os_version)<br>
self.info.update(entry)<br>
<br>
(...)<br>
<br>
# update the dict with user inputs<br>
self.info.update(args) <br>
<br>
You can think VMTemplate like a dict of data.<br>
<br>
if you add to common_specs:<br>
<br>
cpu_info: {topology: {sockets: 1, cores: 1, threads: 1}}<br>
<br>
All VMTemplate instances will have this configuration.<br>
And it will be only override by user inputs (if he/she wants to)<br>
<br>
Template is the Kimchi resource. Its data is just a copy of
VMTemplate.info data.<br>
<br>
Does that make sense?<br>
<br>
In a next step, we will have a new default dict based on host
architecture.<br>
For example, for Power 8, we should use SMT8<br>
<br>
Something like:<br>
<br>
power_specs = {8: {cpu_info: {topology: {sockets: 1, cores: 2,
threads: 4}}}},<br>
....<br>
}<br>
<br>
That way, user does not care about the topology values. Kimchi will
automatically handle it according to host architecture and guest OS.<br>
<br>
<blockquote cite="mid:543BE45E.50107@linux.vnet.ibm.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap=""> '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 5f22db9..34720de 100644
--- a/src/kimchi/vmtemplate.py
+++ b/src/kimchi/vmtemplate.py
@@ -358,6 +358,19 @@ 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 etree.tostring(E.cpu(E.topology(
+ sockets=str(cpu_topo['sockets']),
+ cores=str(cpu_topo['cores']),
+ threads=str(cpu_topo['threads']))))
+
+ return ""
+
def to_vm_xml(self, vm_name, vm_uuid, **kwargs):
params = dict(self.info)
params['name'] = vm_name
@@ -369,6 +382,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 +414,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'/>
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
_______________________________________________
Kimchi-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Kimchi-devel@ovirt.org">Kimchi-devel@ovirt.org</a>
<a class="moz-txt-link-freetext" href="http://lists.ovirt.org/mailman/listinfo/kimchi-devel">http://lists.ovirt.org/mailman/listinfo/kimchi-devel</a>
</pre>
</blockquote>
<br>
</body>
</html>