[Kimchi-devel] [PATCH v4] Backend support for templates with sockets, cores, and threads

Royce Lv lvroyce at linux.vnet.ibm.com
Tue Oct 14 07:06:31 UTC 2014


Reviewed-by: Royce Lv<lvroyce at linux.vnet.ibm.com>

On 2014年10月14日 06:54, 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
>
> v3->v4:
> - Remove the unused cpu_ elements from common_spec
> - Pass new_t into validate function to reduce complexity
> - Rearrange code to decrese indents in _get_cpu_xml
>
> 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 at 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   | 32 ++++++++++++++++++++++++++++++++
>   src/kimchi/osinfo.py            |  5 ++---
>   src/kimchi/vmtemplate.py        | 16 ++++++++++++++++
>   10 files changed, 113 insertions(+), 9 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/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 d9e13f0..d3edf27 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 75fb076..611316c 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -143,6 +143,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..2954148 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 vcpus !=  sockets * cores * threads:
> +                    raise InvalidParameter("KCHTMPL0025E")
> +
>           conn = self.conn.get()
>           pool_uri = params.get(u'storagepool', '')
>           if pool_uri:
> @@ -156,6 +172,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 +207,18 @@ 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')
> +	if cpu_info is None:
> +	    return True
> +	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..cfb7704 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'/>




More information about the Kimchi-devel mailing list