[Kimchi-devel] [PATCH] [Kimchi 2/3] Add maxvcpus attribute to guests
Paulo Vital
pvital at linux.vnet.ibm.com
Fri Jan 29 17:09:33 UTC 2016
Reviewed-by: Paulo Vital <pvital at linux.vnet.ibm.com>
On 01/29/2016 02:58 PM, Paulo Ricardo Paz Vital wrote:
>
>
> On 01/27/2016 07:14 PM, Lucio Correia wrote:
>> - Enable update of maxvcpus, vcpus and CPU topology
>> - Enable vCPU cold plug when topology is defined
>> - Add cpu_info validation to guest update
>> - Simplify _get_cpu_xml() code
>> - Remove change_numa verification
>> - Update docs and APIs with maxvcpus attribute
>>
>> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
>> ---
>> API.json | 7 ++++
>> docs/API.md | 17 +++++++-
>> i18n.py | 1 -
>> model/vms.py | 128 +++++++++++++++++++++++++++++++++++++---------------------
>> vmtemplate.py | 30 ++++----------
>> 5 files changed, 110 insertions(+), 73 deletions(-)
>>
>> diff --git a/API.json b/API.json
>> index 4876cc0..a3d99f8 100644
>> --- a/API.json
>> +++ b/API.json
>> @@ -299,12 +299,19 @@
>> }
>> }
>> },
>> + "cpu_info": { "$ref": "#/kimchitype/cpu_info" },
>> "cpus": {
>> "description": "The new number of virtual CPUs for the VM",
>> "type": "integer",
>> "minimum": 1,
>> "error": "KCHTMPL0012E"
>> },
>> + "maxvcpus": {
>> + "description": "The maximum number of virtual CPUs that can be assigned to the VM",
>> + "type": "integer",
>> + "minimum": 1,
>> + "error": "KCHTMPL0012E"
>> + },
>> "memory": {
>> "description": "The new amount (MB) of memory for the VM",
>> "type": "integer",
>> diff --git a/docs/API.md b/docs/API.md
>> index 4790d08..8dbc27c 100644
>> --- a/docs/API.md
>> +++ b/docs/API.md
>> @@ -118,6 +118,7 @@ server.
>> * uuid: UUID of the VM.
>> * memory: The amount of memory assigned to the VM (in MB)
>> * cpus: The number of CPUs assigned to the VM
>> + * maxvcpus: The maximum number of CPUs that can be assigned to the VM
>
> Same as I asked on the first patch.
>
> In API.json you said that maxvcpu is "The maximum number of virtual CPUs
> that can be assigned to the VMs..." and here looks like you missed the
> *virtual* word. Is it correct?
>
>> * screenshot: A link to a recent capture of the screen in PNG format
>> * icon: A link to an icon that represents the VM
>> * graphics: A dict to show detail of VM graphics.
>> @@ -138,12 +139,13 @@ server.
>> * groups: A list of system groups whose users have permission to access
>> the VM. Default is: empty (i.e. no groups given access).
>> * **DELETE**: Remove the Virtual Machine
>> -* **PUT**: update the parameters of existed VM
>> +* **PUT**: update the parameters of existing VM
>> * name: New name for this VM (only applied for shutoff VM)
>> * users: New list of system users.
>> * groups: New list of system groups.
>> - * cpus: New number of virtual cpus for this VM (if VM is running, new value
>> + * cpus: New number of CPUs for this VM (if VM is running, new value
>> will take effect in next reboot)
>> + * maxvcpus: The maximum number of CPUs that can be assigned to the VM
>
> Same here.
>
>> * memory: New amount of memory (MB) for this VM (if VM is running, new
>> value will take effect in next reboot)
>> * graphics: A dict to show detail of VM graphics.
>> @@ -152,6 +154,17 @@ server.
>> * passwdValidTo *(optional)*: lifetime for the console password. When
>> omitted the password will be valid just
>> for 30 seconds.
>> + * cpu_info: 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 maximum number of sockets to use.
>> + * cores - The number of cores per socket.
>> + * threads - The number of threads per core.
>> + If CPU topology is specified, *cpus* and *maxvcpus* also must be
>> + specified. Make sure *maxvcpus* is equal to the product of sockets,
>> + cores and threads, and *cpus* is a multiple of a product of cores
>> + and threads.
>>
>> * **POST**: *See Virtual Machine Actions*
>>
>> diff --git a/i18n.py b/i18n.py
>> index bb6f3d1..59870e8 100644
>> --- a/i18n.py
>> +++ b/i18n.py
>> @@ -130,7 +130,6 @@ messages = {
>> "KCHVM0071E": _("Memory value %(mem)s 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": _("Cannot change VCPU value because '%(vm)s' has a topology defined - sockets: %(sockets)s, cores: %(cores)s, threads: %(threads)s."),
>>
>> "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."),
>> "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."),
>> diff --git a/model/vms.py b/model/vms.py
>> index 9aaf530..fa3bcbb 100644
>> --- a/model/vms.py
>> +++ b/model/vms.py
>> @@ -41,7 +41,7 @@ from wok.model.tasks import TaskModel
>> from wok.rollbackcontext import RollbackContext
>> from wok.utils import add_task, convert_data_size, get_next_clone_name
>> from wok.utils import import_class, run_setfacl_set_attr, run_command, wok_log
>> -from wok.xmlutils.utils import xpath_get_text, xml_item_update
>> +from wok.xmlutils.utils import xpath_get_text, xml_item_remove, xml_item_update
>> from wok.xmlutils.utils import dictize
>>
>> from wok.plugins.kimchi import model
>> @@ -72,15 +72,12 @@ DOM_STATE_MAP = {0: 'nostate',
>> 6: 'crashed',
>> 7: 'pmsuspended'}
>>
>> -VM_STATIC_UPDATE_PARAMS = {'name': './name', 'cpus': './vcpu'}
>> -
>> -VM_LIVE_UPDATE_PARAMS = {}
>> -
>> # update parameters which are updatable when the VM is online
>> VM_ONLINE_UPDATE_PARAMS = ['graphics', 'groups', 'memory', 'users']
>> +
>> # update parameters which are updatable when the VM is offline
>> -VM_OFFLINE_UPDATE_PARAMS = ['cpus', 'graphics', 'groups', 'memory', 'name',
>> - 'users']
>> +VM_OFFLINE_UPDATE_PARAMS = ['cpus', 'cpu_info', 'graphics', 'groups',
>> + 'maxvcpus', 'memory', 'name', 'users']
>>
>> XPATH_DOMAIN_DISK = "/domain/devices/disk[@device='disk']/source/@file"
>> XPATH_DOMAIN_DISK_BY_FILE = "./devices/disk[@device='disk']/source[@file='%s']"
>> @@ -93,8 +90,10 @@ XPATH_DOMAIN_MEMORY_UNIT = '/domain/memory/@unit'
>> XPATH_DOMAIN_UUID = '/domain/uuid'
>> XPATH_DOMAIN_DEV_CPU_ID = '/domain/devices/spapr-cpu-socket/@id'
>>
>> +XPATH_NAME = './name'
>> XPATH_NUMA_CELL = './cpu/numa/cell'
>> XPATH_TOPOLOGY = './cpu/topology'
>> +XPATH_VCPU = './vcpu'
>>
>> # key: VM name; value: lock object
>> vm_locks = {}
>> @@ -285,17 +284,6 @@ class VMModel(object):
>> raise InvalidParameter('KCHVM0074E',
>> {'params': ', '.join(ext_params)})
>>
>> - if 'cpus' in params and DOM_STATE_MAP[dom.info()[0]] == 'shutoff':
>> - # user cannot change vcpu if topology is defined.
>> - curr_vcpu = dom.vcpusFlags(libvirt.VIR_DOMAIN_AFFECT_CURRENT)
>> - if self.has_topology(dom) and curr_vcpu != params['cpus']:
>> - raise InvalidOperation(
>> - 'KCHVM0075E',
>> - {'vm': dom.name(),
>> - 'sockets': self.get_vm_sockets(dom),
>> - 'cores': self.get_vm_cores(dom),
>> - 'threads': self.get_vm_threads(dom)})
>> -
>> self._live_vm_update(dom, params)
>> vm_name, dom = self._static_vm_update(name, dom, params)
>> return vm_name
>> @@ -751,6 +739,8 @@ class VMModel(object):
>> old_xml = new_xml = dom.XMLDesc(0)
>>
>> params = copy.deepcopy(params)
>> +
>> + # Update name
>> name = params.get('name')
>> nonascii_name = None
>> if name is not None:
>> @@ -760,37 +750,42 @@ class VMModel(object):
>> raise InvalidParameter("KCHVM0003E", msg_args)
>>
>> params['name'], nonascii_name = get_ascii_nonascii_name(name)
>> + new_xml = xml_item_update(new_xml, XPATH_NAME, nonascii_name, None)
>> +
>> + # Update vcpus
>> + cpus = params.get('cpus', None)
>> + if cpus:
>> + attrib = 'current'
>> + new_xml = xml_item_update(new_xml, XPATH_VCPU, str(cpus), attrib)
>> +
>> + # Update maxvcpus
>> + maxvcpus = params.get('maxvcpus', None)
>> + if maxvcpus:
>> + new_xml = xml_item_update(new_xml, XPATH_VCPU, str(maxvcpus), None)
>> +
>> + # Update topology
>> + if 'cpu_info' in params:
>> + topology = params.get('cpu_info', {}).get('topology', None)
>> +
>> + if topology:
>> + sockets = str(topology['sockets'])
>> + cores = str(topology['cores'])
>> + threads = str(topology['threads'])
>> +
>> + xpath = XPATH_TOPOLOGY
>> + new_xml = xml_item_update(new_xml, xpath, sockets, 'sockets')
>> + new_xml = xml_item_update(new_xml, xpath, cores, 'cores')
>> + new_xml = xml_item_update(new_xml, xpath, threads, 'threads')
>> + else:
>> + # topology may have been removed
>> + new_xml = xml_item_remove(new_xml, xpath)
>>
>> - for key, val in params.items():
>> - change_numa = True
>> - if key in VM_STATIC_UPDATE_PARAMS:
>> - if type(val) == int:
>> - val = str(val)
>> - xpath = VM_STATIC_UPDATE_PARAMS[key]
>> - attrib = None
>> - if key == 'cpus':
>> - if self.has_topology(dom) or dom.isActive():
>> - change_numa = False
>> - continue
>> - # Update maxvcpu firstly
>> - new_xml = xml_item_update(new_xml, xpath,
>> - str(self._get_host_maxcpu()),
>> - attrib)
>> - # Update current vcpu
>> - attrib = 'current'
>> - new_xml = xml_item_update(new_xml, xpath, val, attrib)
>> + # Revalidate cpu info - may raise CPUInfo exceptions
>> + self._validate_cpu_info(new_xml, dom)
>>
>> # Updating memory and NUMA if necessary, if vm is offline
>> - if not dom.isActive():
>> - if 'memory' in params:
>> - new_xml = self._update_memory_config(new_xml, params)
>> - elif 'cpus' in params and change_numa and \
>> - (xpath_get_text(new_xml, XPATH_NUMA_CELL + '/@memory') != []):
>> - new_xml = xml_item_update(
>> - new_xml,
>> - XPATH_NUMA_CELL,
>> - value='0',
>> - attr='cpus')
>> + if 'memory' in params and not dom.isActive():
>> + new_xml = self._update_memory_config(new_xml, params)
>>
>> if 'graphics' in params:
>> new_xml = self._update_graphics(dom, new_xml, params)
>> @@ -928,6 +923,27 @@ class VMModel(object):
>>
>> return ET.tostring(root, encoding="utf-8")
>>
>> + def _validate_cpu_info(self, new_xml, dom):
>> + topology = None
>> + if self.has_topology(dom):
>> + sockets = xpath_get_text(new_xml, XPATH_TOPOLOGY + '/@sockets')[0]
>> + cores = xpath_get_text(new_xml, XPATH_TOPOLOGY + '/@cores')[0]
>> + threads = xpath_get_text(new_xml, XPATH_TOPOLOGY + '/@threads')[0]
>> + topology = {
>> + 'sockets': int(sockets),
>> + 'cores': int(cores),
>> + 'threads': int(threads),
>> + }
>> +
>> + params = {
>> + 'maxvcpus': int(xpath_get_text(new_xml, 'vcpu')[0]),
>> + 'cpus': int(xpath_get_text(new_xml, './vcpu/@current')[0]),
>> + 'cpu_info': {'topology': topology}
>> + }
>> +
>> + cpu_model = CPUInfoModel(conn=self.conn)
>> + cpu_model.check_cpu_info(params)
>> +
>> def _live_vm_update(self, dom, params):
>> self._vm_update_access_metadata(dom, params)
>> if 'memory' in params and dom.isActive():
>> @@ -1140,8 +1156,24 @@ class VMModel(object):
>> res['io_throughput_peak'] = vm_stats.get('max_disk_io', 100)
>> users, groups = self._get_access_info(dom)
>>
>> + xml = dom.XMLDesc(0)
>> + maxvcpus = int(xpath_get_text(xml, XPATH_VCPU)[0])
>> +
>> + cpu_info = None
>> + if self.has_topology(dom):
>> + sockets = int(xpath_get_text(xml, XPATH_TOPOLOGY + '/@sockets')[0])
>> + cores = int(xpath_get_text(xml, XPATH_TOPOLOGY + '/@cores')[0])
>> + threads = int(xpath_get_text(xml, XPATH_TOPOLOGY + '/@threads')[0])
>> +
>> + cpu_info = {
>> + 'topology': {
>> + 'sockets': sockets,
>> + 'cores': cores,
>> + 'threads': threads,
>> + }
>> + }
>> +
>> if state == 'shutoff':
>> - xml = dom.XMLDesc(0)
>> val = xpath_get_text(xml, XPATH_DOMAIN_MEMORY)[0]
>> unit_list = xpath_get_text(xml, XPATH_DOMAIN_MEMORY_UNIT)
>> if len(unit_list) > 0:
>> @@ -1158,6 +1190,8 @@ class VMModel(object):
>> 'uuid': dom.UUIDString(),
>> 'memory': memory,
>> 'cpus': info[3],
>> + 'cpu_info': cpu_info,
>> + 'maxvcpus': maxvcpus,
>> 'screenshot': screenshot,
>> 'icon': icon,
>> # (type, listen, port, passwd, passwdValidTo)
>> diff --git a/vmtemplate.py b/vmtemplate.py
>> index 2f524a7..e1571b0 100644
>> --- a/vmtemplate.py
>> +++ b/vmtemplate.py
>> @@ -324,12 +324,9 @@ class VMTemplate(object):
>>
>> def _get_cpu_xml(self):
>> # Include CPU topology, if provided
>> - cpu_info = self.info.get('cpu_info')
>> - if cpu_info is not None:
>> - cpu_topo = cpu_info.get('topology')
>> - return get_cpu_xml(0,
>> - self.info.get('memory') << 10,
>> - cpu_topo)
>> + cpu_topo = self.info.get('cpu_info', {}).get('topology', None)
>> +
>> + 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:
>> @@ -371,8 +368,6 @@ class VMTemplate(object):
>> libvirt_stream_protocols = kwargs.get('libvirt_stream_protocols', [])
>> cdrom_xml = self._get_cdrom_xml(libvirt_stream_protocols)
>>
>> - max_vcpus = kwargs.get('max_vcpus', 1)
>> -
>> if not urlparse.urlparse(self.info.get('cdrom', "")).scheme in \
>> libvirt_stream_protocols and \
>> params.get('iso_stream', False):
>> @@ -402,22 +397,11 @@ class VMTemplate(object):
>> # set a hard limit using max_memory + 1GiB
>> params['hard_limit'] = params['max_memory'] + (1024 << 10)
>>
>> - cpu_topo = self.info.get('cpu_info').get('topology')
>> - if (cpu_topo is not None):
>> - sockets = int(max_vcpus / (cpu_topo['cores'] *
>> - cpu_topo['threads']))
>> - self.info['cpu_info']['topology']['sockets'] = sockets
>> + # vcpu element
>> + params['vcpus'] = "<vcpu current='%d'>%d</vcpu>" % \
>> + (params['cpus'], params['maxvcpus'])
>>
>> - # Reduce maxvcpu to fit number of sockets if necessary
>> - total_max_vcpu = sockets * cpu_topo['cores'] * cpu_topo['threads']
>> - if total_max_vcpu != max_vcpus:
>> - max_vcpus = total_max_vcpu
>> -
>> - params['vcpus'] = "<vcpu current='%s'>%d</vcpu>" % \
>> - (params['cpus'], max_vcpus)
>> - else:
>> - params['vcpus'] = "<vcpu current='%s'>%d</vcpu>" % \
>> - (params['cpus'], max_vcpus)
>> + # cpu_info element
>> params['cpu_info'] = self._get_cpu_xml()
>>
>> xml = """
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
More information about the Kimchi-devel
mailing list