[PATCH v3] Backend support for templates with sockets, cores, and threads

9 Oct 2014

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

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 <christy@linux.vnet.ibm.com>
---
 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
+          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,
                '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'/>
-- 
1.9.3

    

Christy Perez

Royce Lv

Christy Perez

Aline Manera

Aline Manera

Christy Perez

tags

participants (3)