[Kimchi-devel] [PATCH][Kimchi 1/3] Add support to edit max memory in Templates

Paulo Ricardo Paz Vital pvital at linux.vnet.ibm.com
Tue Feb 16 12:43:05 UTC 2016



On 02/16/2016 12:32 AM, Rodrigo Trujillo wrote:
> This patch changes the 'memory' parameter in API of Templates to:
> memory: {current: XXX, maxmemory: YYY}
> Other changes include:
> * enable maxmemory edition
> * remove max memory tests and limits
> * keeps max memory limit to 1TiB
> * changes templates.conf to suport max memory
> * set default memory and maxmemory to 1024
> 
> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
> ---
>  API.json           | 44 ++++++++++++++++++++++++++------
>  docs/API.md        | 18 ++++++++++---
>  i18n.py            | 11 ++++++--
>  model/templates.py | 58 +++++++++++++++++++++++++++++++++++++++---
>  model/vms.py       | 10 ++++++--
>  osinfo.py          | 16 ++++++++----
>  template.conf      | 10 +++++---
>  vmtemplate.py      | 74 ++++++++++++++++++++++--------------------------------
>  8 files changed, 169 insertions(+), 72 deletions(-)
> 
> diff --git a/API.json b/API.json
> index 294be64..fcde123 100644
> --- a/API.json
> +++ b/API.json
> @@ -453,10 +453,24 @@
>                      "error": "KCHTMPL0011E"
>                  },
>                  "memory": {
> -                    "description": "Memory (MB) for the template",
> -                    "type": "integer",
> -                    "minimum": 512,
> -                    "error": "KCHTMPL0013E"
> +                    "description": "Current memory and maximum memory values",
> +                    "type": "object",
> +                    "properties": {
> +                        "current": {
> +                            "description": "Memory (MB) for the template",
> +                            "type": "integer",
> +                            "minimum": 512,
> +                            "error": "KCHTMPL0013E"
> +                        },
> +                        "maxmemory": {
> +                            "description": "Maximum memory (MB) for the template",
> +                            "type": "integer",
> +                            "minimum": 512,
> +                            "error": "KCHTMPL0013E"
> +                        }
> +                    },
> +		    "additionalProperties": false,
> +                    "error": "KCHTMPL0030E"
>                  },
>                  "cdrom": {
>                      "description": "Path for cdrom",
> @@ -630,10 +644,24 @@
>                      "error": "KCHTMPL0011E"
>                  },
>                  "memory": {
> -                    "description": "Memory (MB) for the template",
> -                    "type": "integer",
> -                    "minimum": 512,
> -                    "error": "KCHTMPL0013E"
> +                    "description": "Current memory and maximum memory values",
> +                    "type": "object",
> +                    "properties": {
> +                        "current": {
> +                            "description": "Memory (MB) for the template",
> +                            "type": "integer",
> +                            "minimum": 512,
> +                            "error": "KCHTMPL0013E"
> +                        },
> +                        "maxmemory": {
> +                            "description": "Maximum memory (MB) for the template",
> +                            "type": "integer",
> +                            "minimum": 512,
> +                            "error": "KCHTMPL0013E"
> +                        }
> +                    },
> +		    "additionalProperties": false,
> +                    "error": "KCHTMPL0030E"
>                  },
>                  "cdrom": {
>                      "description": "Path for cdrom",
> diff --git a/docs/API.md b/docs/API.md
> index a46e80e..33d51ac 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -292,8 +292,11 @@ 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
> -    * memory *(optional)*: The amount of memory assigned to the VM.
> -      Default is 1024M.
> +    * memory *(optional)*: The memory parameters of the template, specify one
> +      or both. Default values are 1024MiB:
> +        * current: The amount of memory that will be assigned to the VM.
> +        * maxmemory: The maximum total of memory that the VM can have. Amount
> +          over current will be used exclusively for memory hotplug
>      * cdrom *(optional)*: A volume name or URI to an ISO image.
>      * networks *(optional)*: list of networks will be assigned to the new VM.
>        Default is '[default]'
> @@ -396,7 +399,11 @@ A interface represents available network interface on VM.
>      * icon: A URI to a PNG image representing this template
>      * os_distro: The operating system distribution
>      * os_version: The version of the operating system distribution
> -    * memory: The amount of memory assigned to the VM in the unit of MB
> +    * memory: The memory parameters of the template, that will be assigned to
> +      the VM in the unit of MiB.
> +        * current: The amount of memory that will be assigned to the VM.
> +        * maxmemory: The maximum total of memory that the VM can have. Amount
> +          over current will be used exclusively for memory hotplug
>      * cdrom: A volume name or URI to an ISO image
>      * storagepool: URI of the storagepool where template allocates vm storage.
>      * networks *(optional)*: list of networks will be assigned to the new VM.
> @@ -439,7 +446,10 @@ A interface represents available network interface on VM.
>      * icon: A URI to a PNG image representing this template
>      * os_distro: The operating system distribution
>      * os_version: The version of the operating system distribution
> -    * memory: The amount of memory assigned to the VM
> +    * memory: The memory parameters of the template, specify one or both of:
> +        * current: The amount of memory that will be assigned to the VM.
> +        * maxmemory: The maximum total of memory that the VM can have. Amount
> +          over current will be used exclusively for memory hotplug
>      * cdrom: A volume name or URI to an ISO image
>      * networks *(optional)*: list of networks will be assigned to the new VM.
>      * disks: An array of requested disks with the following optional fields
> diff --git a/i18n.py b/i18n.py
> index 59e75f7..5395879 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -127,9 +127,14 @@ messages = {
>      "KCHVM0068E": _("Unable to setup password-less login at remote host %(host)s using user %(user)s. Error: %(error)s"),
>      "KCHVM0069E": _("Password field must be a string."),
>      "KCHVM0070E": _("Error creating local host ssh rsa key of user 'root'."),
> -    "KCHVM0071E": _("Memory value %(mem)s must be aligned to %(alignment)sMiB."),
> +    "KCHVM0071E": _("%(param)s value (%(mem)sMiB) must be aligned to %(alignment)sMiB."),
>      "KCHVM0073E": _("Unable to update the following parameters while the VM is offline: %(params)s"),
>      "KCHVM0074E": _("Unable to update the following parameters while the VM is online: %(params)s"),
> +    "KCHVM0075E": _("Maximum memory requested is higher than amount supported by the host: %(memHost)sMiB."),
> +    "KCHVM0076E": _("Maximum memory requested is higher than maximum amount recommended: 1TiB"),
> +    "KCHVM0077E": _("Cannot update maximum memory when guest is running."),

KCHVM0076E and KCHVM0077E already exist and were not modified. See below

> +    "KCHVM0078E": _("Maximum memory requested is higher than amount supported by the host: %(memHost)sMiB."),
> +    "KCHVM0079E": _("Memory requested is higher than maximum amount recommended: 1TiB"),
> 
>      "KCHVM0076E": _("VM %(name)s must have serial and console defined to open a web serial console"),
>      "KCHVM0077E": _("Impossible to get the serial console of %(name)s"),

They are here ^^^

> @@ -165,7 +170,7 @@ messages = {
>      "KCHTMPL0010E": _("Template distribution must be a string"),
>      "KCHTMPL0011E": _("Template distribution version must be a string"),
>      "KCHTMPL0012E": _("The number of CPUs must be an integer greater than 0"),
> -    "KCHTMPL0013E": _("Amount of memory (MB) must be an integer greater than 512"),
> +    "KCHTMPL0013E": _("Amount of memory and maximum memory (MB) must be an integer greater than 512"),
>      "KCHTMPL0014E": _("Template CDROM must be a local or remote ISO file"),
>      "KCHTMPL0015E": _("Invalid storage pool URI %(value)s specified for template"),
>      "KCHTMPL0016E": _("Specify an ISO image as CDROM or a base image to create a template"),
> @@ -181,6 +186,8 @@ messages = {
>      "KCHTMPL0027E": _("Invalid disk image format. Valid formats: bochs, cloop, cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc."),
>      "KCHTMPL0028E": _("When setting template disks, following parameters are required: 'index', 'pool name', 'format', 'size' or 'volume' (for scsi/iscsi pools)"),
>      "KCHTMPL0029E": _("Disk format must be 'raw', for logical, iscsi, and scsi pools."),
> +    "KCHTMPL0030E": _("Memory expects an object with one or both parameters: 'current' and 'maxmemory'"),
> +    "KCHTMPL0031E": _("Memory value (%(mem)sMiB) must be equal or lesser than maximum memory value (%(maxmem)sMiB)"),
> 
>      "KCHPOOL0001E": _("Storage pool %(name)s already exists"),
>      "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
> diff --git a/model/templates.py b/model/templates.py
> index 8a29e02..6138592 100644
> --- a/model/templates.py
> +++ b/model/templates.py
> @@ -20,6 +20,8 @@
>  import copy
>  import libvirt
>  import os
> +import platform
> +import psutil
>  import stat
> 
>  from wok.exception import InvalidOperation, InvalidParameter
> @@ -34,6 +36,12 @@ from wok.plugins.kimchi.utils import pool_name_from_uri
>  from wok.plugins.kimchi.vmtemplate import VMTemplate
> 
> 
> +# In PowerPC, memories must be aligned to 256 MiB
> +PPC_MEM_ALIGN = 256
> +# Max memory 1TB, in KiB
> +MAX_MEM_LIM = 1073741824
> +
> +
>  class TemplatesModel(object):
>      def __init__(self, **kargs):
>          self.objstore = kargs['objstore']
> @@ -69,10 +77,8 @@ class TemplatesModel(object):
>          # Validate cpu info
>          t.cpuinfo_validate()
> 
> -        # Validate max memory
> -        maxMem = (t._get_max_memory(t.info.get('memory')) >> 10)
> -        if t.info.get('memory') > maxMem:
> -            raise OperationFailed("KCHVM0041E", {'maxmem': str(maxMem)})
> +        # Validate memory
> +        self._validate_memory(t.info.get('memory'))
> 
>          # Validate volumes
>          for disk in t.info.get('disks'):
> @@ -115,6 +121,44 @@ class TemplatesModel(object):
>                  raise InvalidParameter("KCHTMPL0019E", {'pool': pool_name,
>                                                          'volume': volume})
> 
> +    def _validate_memory(self, memory):
> +        current = memory.get('current')
> +        maxmem = memory.get('maxmemory')
> +
> +        # Memories must be lesser than 1TB and the Host memory limit
> +        if maxmem > (MAX_MEM_LIM >> 10):
> +            raise InvalidParameter("KCHVM0076E")
> +        if current > (MAX_MEM_LIM >> 10):
> +            raise InvalidParameter("KCHVM0079E")
> +
> +        if hasattr(psutil, 'virtual_memory'):
> +            host_memory = psutil.virtual_memory().total >> 10 >> 10
> +        else:
> +            host_memory = psutil.TOTAL_PHYMEM >> 10 >> 10
> +        if maxmem > host_memory:
> +            raise InvalidParameter("KCHVM0075E", {'memHost': host_memory})
> +        if current > host_memory:
> +            raise InvalidParameter("KCHVM0078E", {'memHost': host_memory})
> +
> +        if current > maxmem:
> +            raise InvalidParameter("KCHTMPL0031E",
> +                                   {'mem': str(current),
> +                                    'maxmem': str(maxmem)})
> +
> +        # make sure memory and Maxmemory are alingned in 256MiB in PowerPC
> +        distro, _, _ = platform.linux_distribution()
> +        if distro == "IBM_PowerKVM":
> +            if current % PPC_MEM_ALIGN != 0:
> +                raise InvalidParameter('KCHVM0071E',
> +                                       {'param': "Memory",
> +                                        'mem': str(current),
> +                                        'alignment': str(PPC_MEM_ALIGN)})
> +            elif maxmem % PPC_MEM_ALIGN != 0:
> +                raise InvalidParameter('KCHVM0071E',
> +                                       {'param': "Maximum Memory",
> +                                        'mem': str(maxmem),
> +                                        'alignment': str(PPC_MEM_ALIGN)})
> +
> 
>  class TemplateModel(object):
>      def __init__(self, **kargs):
> @@ -180,6 +224,12 @@ class TemplateModel(object):
>              cpu_info.update(new_cpu_info)
>              params['cpu_info'] = cpu_info
> 
> +        # Fix memory values, because method update does not work recursively
> +        new_mem = params.get('memory')
> +        if new_mem:
> +            params['memory'] = copy.copy(old_t.get('memory'))
> +            params['memory'].update(new_mem)
> +
>          new_t.update(params)
> 
>          for net_name in params.get(u'networks', []):
> diff --git a/model/vms.py b/model/vms.py
> index 23e0df9..400ef46 100644
> --- a/model/vms.py
> +++ b/model/vms.py
> @@ -53,13 +53,13 @@ from wok.plugins.kimchi.model.config import CapabilitiesModel
>  from wok.plugins.kimchi.model.cpuinfo import CPUInfoModel
>  from wok.plugins.kimchi.model.featuretests import FeatureTests
>  from wok.plugins.kimchi.model.templates import TemplateModel
> +from wok.plugins.kimchi.model.templates import MAX_MEM_LIM, PPC_MEM_ALIGN
>  from wok.plugins.kimchi.model.utils import get_ascii_nonascii_name, get_vm_name
>  from wok.plugins.kimchi.model.utils import get_metadata_node
>  from wok.plugins.kimchi.model.utils import remove_metadata_node
>  from wok.plugins.kimchi.model.utils import set_metadata_node
>  from wok.plugins.kimchi.screenshot import VMScreenshot
>  from wok.plugins.kimchi.utils import template_name_from_uri
> -from wok.plugins.kimchi.vmtemplate import MAX_MEM_LIM, PPC_MEM_ALIGN
>  from wok.plugins.kimchi.xmlutils.cpu import get_cpu_xml, get_numa_xml
>  from wok.plugins.kimchi.xmlutils.cpu import get_topology_xml
>  from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks
> @@ -256,7 +256,13 @@ class VMModel(object):
>              if 'memory' in params and distro == "IBM_PowerKVM":
>                  if params['memory'] % PPC_MEM_ALIGN != 0:
>                      raise InvalidParameter('KCHVM0071E',
> -                                           {'mem': str(params['memory']),
> +                                           {'param': "Memory",
> +                                            'mem': str(memory),
> +                                            'alignment': str(PPC_MEM_ALIGN)})
> +                elif maxmem % PPC_MEM_ALIGN != 0:
> +                    raise InvalidParameter('KCHVM0071E',
> +                                           {'param': "Maximum Memory",
> +                                            'mem': str(maxmem),
>                                              'alignment': str(PPC_MEM_ALIGN)})
> 
>              dom = self.get_vm(name, self.conn)
> diff --git a/osinfo.py b/osinfo.py
> index 2ec5c3e..8102548 100644
> --- a/osinfo.py
> +++ b/osinfo.py
> @@ -110,11 +110,13 @@ def _get_tmpl_defaults():
>      ConfigObj returns a dict like below when no changes were made in the
>      template configuration file (template.conf)
> 
> -    {'main': {}, 'storage': {'disk.0': {}}, 'processor': {}, 'graphics': {}}
> +    {'main': {}, 'memory': {}, 'storage': {'disk.0': {}}, 'processor': {},
> +     'graphics': {}}
> 
>      The default values should be like below:
> 
> -    {'main': {'networks': ['default'], 'memory': '1024'},
> +    {'main': {'networks': ['default']},
> +     'memory': {'current': 1024, 'maxmemory': 1024},
>       'storage': { 'disk.0': {'format': 'qcow2', 'size': '10',
>                               'pool': '/plugins/kimchi/storagepools/default'}},
>       'processor': {'vcpus': '1',  'maxvcpus': 1},
> @@ -123,7 +125,8 @@ def _get_tmpl_defaults():
>      # Create dict with default values
>      tmpl_defaults = defaultdict(dict)
>      tmpl_defaults['main']['networks'] = ['default']
> -    tmpl_defaults['main']['memory'] = _get_default_template_mem()
> +    tmpl_defaults['memory'] = {'current': _get_default_template_mem(),
> +                               'maxmemory': _get_default_template_mem()}
>      tmpl_defaults['storage']['disk.0'] = {'size': 10, 'format': 'qcow2',
>                                            'pool': 'default'}
>      tmpl_defaults['processor']['vcpus'] = 1
> @@ -145,8 +148,11 @@ def _get_tmpl_defaults():
>                  'cdrom_bus': 'ide', 'cdrom_index': 2, 'mouse_bus': 'ps2'}
> 
>      # Parse main section to get networks and memory values
> -    main_section = default_config.pop('main')
> -    defaults.update(main_section)
> +    defaults.update(default_config.pop('main'))
> +    defaults['memory'] = default_config.pop('memory')
> +
> +    defaults['memory']['current'] = int(defaults['memory']['current'])
> +    defaults['memory']['maxmemory'] = int(defaults['memory']['maxmemory'])
> 
>      # Parse storage section to get disks values
>      storage_section = default_config.pop('storage')
> diff --git a/template.conf b/template.conf
> index 3839be4..c4598f1 100644
> --- a/template.conf
> +++ b/template.conf
> @@ -3,13 +3,17 @@
>  #
> 
>  [main]
> -# Memory in MB
> -#memory = 1024
> -
>  # List of networks separated by comma
>  # Represents the virtual network interfaces to be assigned to guest
>  #networks = default,
> 
> +[memory]
> +# Memory in MB
> +# current = 1024
> +
> +# Maximum value of memory to be assigned to guest in MB
> +# maxmemory = 1024
> +
>  [storage]
> 
>  # Specify multiple [[disk.X]] sub-sections to add multiples disks to guest
> diff --git a/vmtemplate.py b/vmtemplate.py
> index ef17ff6..ab6d580 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -19,7 +19,6 @@
> 
>  import os
>  import platform
> -import psutil
>  import stat
>  import time
>  import urlparse
> @@ -42,12 +41,6 @@ from wok.plugins.kimchi.xmlutils.qemucmdline import get_qemucmdline_xml
>  from wok.plugins.kimchi.xmlutils.serial import get_serial_xml
> 
> 
> -# In PowerPC, memories must be aligned to 256 MiB
> -PPC_MEM_ALIGN = 256
> -# Max memory 1TB, in KiB
> -MAX_MEM_LIM = 1073741824
> -
> -
>  class VMTemplate(object):
>      def __init__(self, args, scan=False):
>          """
> @@ -84,6 +77,14 @@ class VMTemplate(object):
>              args['graphics'] = graphics
> 
>          default_disk = self.info['disks'][0]
> +
> +        # Complete memory args, because dict method update is not recursive
> +        if 'memory' in args:
> +            if 'current' not in args['memory']:

After apply your patch and execute a GET on templates, to list my
current templates, I got the following error:

  File
"/home/pvital/Projects/tmp/wok_test/src/wok/plugins/kimchi/vmtemplate.py",
line 83, in __init__
    if 'current' not in args['memory']:
TypeError: argument of type 'int' is not iterable


the line executed was:

$ curl -k -u test -H "Content-Type: application/json" -H "Accept:
application/json" 'https://localhost:8001/plugins/kimchi/templates' -X GET

So, what happens if I already have templates in the old model (the
current one today) after I apply your patches? I don't have the new
structure of memory.

> +                args['memory']['current'] = self.info['memory']['current']
> +            if 'maxmemory' not in args['memory']:
> +                args['memory']['maxmemory'] = self.info['memory']['maxmemory']
> +
>          # Override template values according to 'args'
>          self.info.update(args)
>          disks = self.info.get('disks')
> @@ -325,29 +326,8 @@ class VMTemplate(object):
>      def _get_cpu_xml(self):
>          # Include CPU topology, if provided
>          cpu_topo = self.info.get('cpu_info', {}).get('topology', {})
> -
> -        return get_cpu_xml(0, self.info.get('memory') << 10, cpu_topo)
> -
> -    def _get_max_memory(self, guest_memory):
> -        # Setting maxMemory of the VM, which will be lesser value between:
> -        # 1TB,  (Template Memory * 4),  Host Physical Memory.
> -        max_memory = MAX_MEM_LIM
> -        if hasattr(psutil, 'virtual_memory'):
> -            host_memory = psutil.virtual_memory().total >> 10
> -        else:
> -            host_memory = psutil.TOTAL_PHYMEM >> 10
> -        if host_memory < max_memory:
> -            max_memory = host_memory
> -        if (((guest_memory * 4) << 10) < max_memory):
> -            max_memory = (guest_memory * 4) << 10
> -
> -        # set up arch to ppc64 instead of ppc64le due to libvirt compatibility
> -        if self.info["arch"] == "ppc64":
> -            # in Power, memory must be aligned in 256MiB
> -            if (max_memory >> 10) % PPC_MEM_ALIGN != 0:
> -                alignment = max_memory % (PPC_MEM_ALIGN << 10)
> -                max_memory -= alignment
> -        return max_memory
> +        return get_cpu_xml(0, (self.info.get('memory').get('current')) << 10,
> +                           cpu_topo)
> 
>      def to_vm_xml(self, vm_name, vm_uuid, **kwargs):
>          params = dict(self.info)
> @@ -375,27 +355,33 @@ class VMTemplate(object):
>          else:
>              params['cdroms'] = cdrom_xml
> 
> -        # max memory
> -        params['max_memory'] = self._get_max_memory(params['memory'])
> -
>          # Setting maximum number of slots to avoid errors when hotplug memory
>          # Number of slots are the numbers of chunks of 1GB that fit inside
>          # the max_memory of the host minus memory assigned to the VM. It
>          # cannot have more than 32 slots in Power.
> -        params['slots'] = ((params['max_memory'] >> 10) -
> -                           params['memory']) >> 10
> -        if params['slots'] < 0:
> +        memory = self.info['memory'].get('current')
> +        maxmemory = self.info['memory'].get('maxmemory')
> +
> +        slots = (maxmemory - memory) >> 10
> +        if slots < 0:
>              raise OperationFailed("KCHVM0041E",
> -                                  {'maxmem': str(params['max_memory'] >> 10)})
> -        elif params['slots'] == 0:
> -            params['slots'] = 1
> -        elif params['slots'] > 32:
> +                                  {'maxmem': str(maxmemory)})
> +        elif slots == 0:
> +            slots = 1
> +        elif slots > 32:
>              distro, _, _ = platform.linux_distribution()
>              if distro == "IBM_PowerKVM":
> -                params['slots'] = 32
> +                slots = 32
> +
> +        # Rearrange memory parameters
> +        params['memory'] = self.info['memory'].get('current')
> +        params['max_memory'] = ""
> +        if memory != maxmemory:
> +            maxmem_xml = "<maxMemory slots='%s' unit='MiB'>%s</maxMemory>"
> +            params['max_memory'] = maxmem_xml % (slots, maxmemory)
> 
>          # set a hard limit using max_memory + 1GiB
> -        params['hard_limit'] = params['max_memory'] + (1024 << 10)
> +        params['hard_limit'] = maxmemory + 1024
> 
>          # vcpu element
>          cpus = params['cpu_info']['vcpus']
> @@ -411,9 +397,9 @@ class VMTemplate(object):
>            <name>%(name)s</name>
>            <uuid>%(uuid)s</uuid>
>            <memtune>
> -            <hard_limit unit='KiB'>%(hard_limit)s</hard_limit>
> +            <hard_limit unit='MiB'>%(hard_limit)s</hard_limit>
>            </memtune>
> -          <maxMemory slots='%(slots)s' unit='KiB'>%(max_memory)s</maxMemory>
> +          %(max_memory)s
>            <memory unit='MiB'>%(memory)s</memory>
>            %(vcpus_xml)s
>            %(cpu_info_xml)s
> 




More information about the Kimchi-devel mailing list