[PATCH] [Kimchi 0/4] CPU Hot plug/unplug feature

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch set implements CPU Hot plug/unplug capabilities in Kimchi. To test it, simply add/remove CPUs in a running guest via the 'Edit' menu. Note that all restrictions on the current CPU value (can't exceed max number of CPUs, must be a number that makes sense in the topology if one is set) still applies. For Power systems, the hot unplug requires additional software running in the guest to work: powerpc-utils, ppc64-diag and librtas. The service 'rtas_err' must be running too. Further enhancements and bug fixes in the CPU handling when editing a turned off VM (such as #1042) will be send in a separated patch. Daniel Henrique Barboza (4): CPU Hot plug/unplug: i18n changes CPU Hot plug/unplug: model changes CPU Hot plug/unplug: test changes CPU Hot plug/unplug: ui changes i18n.py | 5 +++ model/vms.py | 50 ++++++++++++++++++++++- tests/test_model.py | 80 +++++++++++++++++++++++++++++++++++++ ui/js/src/kimchi.guest_edit_main.js | 12 +++--- ui/pages/guest-edit.html.tmpl | 1 + ui/pages/help/en_US/guests.dita | 8 ++++ 6 files changed, 148 insertions(+), 8 deletions(-) -- 2.7.4

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch adds new i18n messages to be used by the new CPU Hotplug backend. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- i18n.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/i18n.py b/i18n.py index 03929e5..205e1af 100644 --- a/i18n.py +++ b/i18n.py @@ -363,6 +363,11 @@ messages = { "KCHCPUINF0008E": _("Parameter 'cpu_info' expects an object with fields among: 'vcpus', 'maxvcpus', 'topology'."), "KCHCPUINF0009E": _("Parameter 'topology' expects an object with fields among: 'sockets', 'cores', 'threads'."), + "KCHCPUHOTP0001E": _("Unable to update Max vCPU or CPU topology when guest is running."), + "KCHCPUHOTP0002E": _("VM '%(vm)s' cannot have more than %(cpus)s CPUs. Please update the CPU value when the VM is not running or increase Max CPU value."), + "KCHCPUHOTP0003E": _("Topology set for vm is sockets='%(sockets)s', cores='%(cores)s' and threads='%(threads)s'. New CPU value must be a multiple of the threads value (%(threads)s)."), + "KCHCPUHOTP0004E": _("Unable to hot plug/unplug CPUs. Details: %(err)s"), + "KCHLVMS0001E": _("Invalid volume group name parameter: %(name)s."), "KCHCONN0001E": _("Unable to establish connection with libvirt. Please check your libvirt URI which is often defined in /etc/libvirt/libvirt.conf"), -- 2.7.4

On 08/11/2016 09:49, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch adds new i18n messages to be used by the new CPU Hotplug backend.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- i18n.py | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/i18n.py b/i18n.py index 03929e5..205e1af 100644 --- a/i18n.py +++ b/i18n.py @@ -363,6 +363,11 @@ messages = { "KCHCPUINF0008E": _("Parameter 'cpu_info' expects an object with fields among: 'vcpus', 'maxvcpus', 'topology'."), "KCHCPUINF0009E": _("Parameter 'topology' expects an object with fields among: 'sockets', 'cores', 'threads'."),
+ "KCHCPUHOTP0001E": _("Unable to update Max vCPU or CPU topology when guest is running."), + "KCHCPUHOTP0002E": _("VM '%(vm)s' cannot have more than %(cpus)s CPUs. Please update the CPU value when the VM is not running or increase Max CPU value."),
It would be better to keep the same term (either Max vCPU or Max CPU).
+ "KCHCPUHOTP0003E": _("Topology set for vm is sockets='%(sockets)s', cores='%(cores)s' and threads='%(threads)s'. New CPU value must be a multiple of the threads value (%(threads)s)."), + "KCHCPUHOTP0004E": _("Unable to hot plug/unplug CPUs. Details: %(err)s"), + "KCHLVMS0001E": _("Invalid volume group name parameter: %(name)s."),
"KCHCONN0001E": _("Unable to establish connection with libvirt. Please check your libvirt URI which is often defined in /etc/libvirt/libvirt.conf"),
-- Lucio Correia Software Engineer IBM LTC Brazil

On 11/08/2016 10:33 AM, Lucio Correia wrote:
On 08/11/2016 09:49, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch adds new i18n messages to be used by the new CPU Hotplug backend.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- i18n.py | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/i18n.py b/i18n.py index 03929e5..205e1af 100644 --- a/i18n.py +++ b/i18n.py @@ -363,6 +363,11 @@ messages = { "KCHCPUINF0008E": _("Parameter 'cpu_info' expects an object with fields among: 'vcpus', 'maxvcpus', 'topology'."), "KCHCPUINF0009E": _("Parameter 'topology' expects an object with fields among: 'sockets', 'cores', 'threads'."),
+ "KCHCPUHOTP0001E": _("Unable to update Max vCPU or CPU topology when guest is running."), + "KCHCPUHOTP0002E": _("VM '%(vm)s' cannot have more than %(cpus)s CPUs. Please update the CPU value when the VM is not running or increase Max CPU value."),
It would be better to keep the same term (either Max vCPU or Max CPU).
You're right. I've checked the UI and it is using 'CPU' there. I've changed these messages to 'CPU' in the v2. Thanks!
+ "KCHCPUHOTP0003E": _("Topology set for vm is sockets='%(sockets)s', cores='%(cores)s' and threads='%(threads)s'. New CPU value must be a multiple of the threads value (%(threads)s)."), + "KCHCPUHOTP0004E": _("Unable to hot plug/unplug CPUs. Details: %(err)s"), + "KCHLVMS0001E": _("Invalid volume group name parameter: %(name)s."),
"KCHCONN0001E": _("Unable to establish connection with libvirt. Please check your libvirt URI which is often defined in /etc/libvirt/libvirt.conf"),

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch makes changes on model/vms.py to support the hotplug of new CPUs in a running VM. For CPU Hot unplug in Power hypervisors there is a need for guest software to be installed: * guest must have packages: powerpc-utils, ppc64-diag, librtas * guest must have service rtas_err.service running Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- model/vms.py | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/model/vms.py b/model/vms.py index bff7ed2..cc9cd4c 100644 --- a/model/vms.py +++ b/model/vms.py @@ -82,7 +82,8 @@ DOM_STATE_MAP = {0: 'nostate', 7: 'pmsuspended'} # update parameters which are updatable when the VM is online -VM_ONLINE_UPDATE_PARAMS = ['graphics', 'groups', 'memory', 'users'] +VM_ONLINE_UPDATE_PARAMS = ['cpu_info', 'graphics', 'groups', + 'memory', 'users'] # update parameters which are updatable when the VM is offline VM_OFFLINE_UPDATE_PARAMS = ['cpu_info', 'graphics', 'groups', 'memory', @@ -1028,6 +1029,15 @@ class VMModel(object): unit='Kib')) return ET.tostring(root, encoding="utf-8") + def get_vm_cpu_cores(self, vm_xml): + return xpath_get_text(vm_xml, XPATH_TOPOLOGY + '/@cores')[0] + + def get_vm_cpu_sockets(self, vm_xml): + return xpath_get_text(vm_xml, XPATH_TOPOLOGY + '/@sockets')[0] + + def get_vm_cpu_threads(self, vm_xml): + return xpath_get_text(vm_xml, XPATH_TOPOLOGY + '/@threads')[0] + def _update_cpu_info(self, new_xml, dom, new_info): topology = {} if self.has_topology(dom): @@ -1064,6 +1074,44 @@ class VMModel(object): if (('memory' in params) and ('current' in params['memory'])): self._update_memory_live(dom, params) + if 'vcpus' in params.get('cpu_info', {}): + self.cpu_hotplug_precheck(dom, params) + vcpus = params['cpu_info'].get('vcpus') + self.update_cpu_live(dom, vcpus) + + def cpu_hotplug_precheck(self, dom, params): + + if (('maxvcpus' in params['cpu_info']) or + ('topology' in params['cpu_info'])): + raise InvalidParameter('KCHCPUHOTP0001E') + + vcpus = params['cpu_info'].get('vcpus') + if vcpus > dom.maxVcpus(): + raise InvalidParameter( + 'KCHCPUHOTP0002E', + {'vm': dom.name(), 'cpus': dom.maxVcpus()} + ) + + if self.has_topology(dom): + sockets = int(self.get_vm_cpu_sockets(dom.XMLDesc(0))) + cores = int(self.get_vm_cpu_cores(dom.XMLDesc(0))) + threads = int(self.get_vm_cpu_threads(dom.XMLDesc(0))) + + if vcpus % threads != 0: + raise InvalidParameter( + 'KCHCPUHOTP0003E', + {'sockets': str(sockets), 'cores': str(cores), + 'threads': str(threads)} + ) + + def update_cpu_live(self, dom, vcpus): + flags = libvirt.VIR_DOMAIN_AFFECT_LIVE | \ + libvirt.VIR_DOMAIN_AFFECT_CONFIG + try: + dom.setVcpusFlags(vcpus, flags) + except libvirt.libvirtError as e: + raise OperationFailed('KCHCPUHOTP0004E', {'err': e.message}) + def _get_mem_dev_total_size(self, xml): root = ET.fromstring(xml) totMemDevs = 0 -- 2.7.4

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch adds new tests in test_model.py to verify this new feature. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- tests/test_model.py | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index 05d7415..f3f6d49 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -22,6 +22,7 @@ import __builtin__ as builtins import base64 import grp +import libvirt import lxml.etree as ET import os import platform @@ -1397,6 +1398,85 @@ class ModelTests(unittest.TestCase): inst.vm_update(u'пeω-∨м', {"bootmenu": False}) self.assertEquals("no", inst.vm_lookup(u'пeω-∨м')['bootmenu']) + def test_get_vm_cpu_cores(self): + xml = """<domain type='kvm'>\ +<cpu><topology sockets='3' cores='2' threads='8'/></cpu>\ +</domain>""" + inst = model.Model(None, objstore_loc=self.tmp_store) + self.assertEqual('2', inst.vm_get_vm_cpu_cores(xml)) + + def test_get_vm_cpu_sockets(self): + xml = """<domain type='kvm'>\ +<cpu><topology sockets='3' cores='2' threads='8'/></cpu>\ +</domain>""" + inst = model.Model(None, objstore_loc=self.tmp_store) + self.assertEqual('3', inst.vm_get_vm_cpu_sockets(xml)) + + def test_get_vm_cpu_threads(self): + xml = """<domain type='kvm'>\ +<cpu><topology sockets='3' cores='2' threads='8'/></cpu>\ +</domain>""" + inst = model.Model(None, objstore_loc=self.tmp_store) + self.assertEqual('8', inst.vm_get_vm_cpu_threads(xml)) + + def test_vm_cpu_hotplug_invalidparam_fail(self): + inst = model.Model(None, objstore_loc=self.tmp_store) + + with self.assertRaisesRegexp(InvalidParameter, 'KCHCPUHOTP0001E'): + params = {"cpu_info": {"vcpus": 1, 'maxvcpus': 4}} + inst.vm_cpu_hotplug_precheck('', params) + + def test_vm_cpu_hotplug_abovemax_fail(self): + class FakeDom(): + def maxVcpus(self): + return 4 + + def name(self): + return 'fakedom' + + inst = model.Model(None, objstore_loc=self.tmp_store) + + with self.assertRaisesRegexp(InvalidParameter, 'KCHCPUHOTP0002E'): + params = {"cpu_info": {"vcpus": 8}} + inst.vm_cpu_hotplug_precheck(FakeDom(), params) + + @mock.patch('wok.plugins.kimchi.model.vms.VMModel.has_topology') + @mock.patch('wok.plugins.kimchi.model.vms.VMModel.get_vm_cpu_sockets') + @mock.patch('wok.plugins.kimchi.model.vms.VMModel.get_vm_cpu_cores') + @mock.patch('wok.plugins.kimchi.model.vms.VMModel.get_vm_cpu_threads') + def test_vm_cpu_hotplug_topology_mismatch_fail(self, mock_threads, + mock_cores, mock_sockets, + mock_has_topology): + class FakeDom(): + def maxVcpus(self): + return 48 + + def name(self): + return 'fakedom' + + def XMLDesc(self, int): + return '' + + mock_has_topology.return_value = True + mock_sockets.return_value = '3' + mock_cores.return_value = '2' + mock_threads.return_value = '8' + + inst = model.Model(None, objstore_loc=self.tmp_store) + + with self.assertRaisesRegexp(InvalidParameter, 'KCHCPUHOTP0003E'): + params = {"cpu_info": {"vcpus": 10}} + inst.vm_cpu_hotplug_precheck(FakeDom(), params) + + def test_vm_cpu_hotplug_error(self): + class FakeDom(): + def setVcpusFlags(self, vcpu, flags): + raise libvirt.libvirtError('') + + inst = model.Model(None, objstore_loc=self.tmp_store) + with self.assertRaisesRegexp(OperationFailed, 'KCHCPUHOTP0004E'): + inst.vm_update_cpu_live(FakeDom(), '') + def test_get_interfaces(self): inst = model.Model('test:///default', objstore_loc=self.tmp_store) -- 2.7.4

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch enables the CPU field for hotplug/unplug operations and adds information text below CPU field about guest cpu hotplug/unplug support if guest is running. Changes in guests.dita were made to add information about the guest support for this feature. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_edit_main.js | 12 +++++------- ui/pages/guest-edit.html.tmpl | 1 + ui/pages/help/en_US/guests.dita | 8 ++++++++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js index 9669d08..558325a 100644 --- a/ui/js/src/kimchi.guest_edit_main.js +++ b/ui/js/src/kimchi.guest_edit_main.js @@ -981,16 +981,14 @@ kimchi.guest_edit_main = function() { wok.window.open('plugins/kimchi/guest-storage-add.html', 'extendCreateStorage'); }); if ((kimchi.thisVMState === "running") || (kimchi.thisVMState === "paused")) { + $("#form-guest-edit-general input").not("#guest-edit-cores-textbox").prop("disabled", true); + $("#guest-edit-cores-hotplug-unsupported").removeClass('hidden'); if (kimchi.capabilities.mem_hotplug_support) { - $("#form-guest-edit-general input").not("#guest-edit-memory-textbox").prop("disabled", true); + $("#guest-edit-memory-textbox").prop("disabled", false); } else { - $("#form-guest-edit-general input").prop("disabled", true); + $("#guest-edit-memory-hotplug-unsupported").removeClass('hidden'); } } - if (! kimchi.capabilities.mem_hotplug_support) { - $("#guest-edit-max-memory-textbox").prop("disabled", true); - $("#guest-edit-memory-hotplug-unsupported").removeClass('hidden'); - } $('#guest-show-max-memory').on('click', function(e) { e.preventDefault; @@ -1071,7 +1069,7 @@ kimchi.guest_edit_main = function() { // since it is disabled; for cpu_info, when guest is running, just skip it since no processing is required if (kimchi.thisVMState === 'running' || kimchi.thisVMState === 'paused') { if (key === 'cpu_info') { - continue; + data['cpu_info']['maxvcpus'] = org.cpu_info.maxvcpus; } if (key === 'memory') { // Replace valueFromUI of max mem with one from original as otherwise the value is undefined diff --git a/ui/pages/guest-edit.html.tmpl b/ui/pages/guest-edit.html.tmpl index 7e0e3d3..f88dbf6 100644 --- a/ui/pages/guest-edit.html.tmpl +++ b/ui/pages/guest-edit.html.tmpl @@ -49,6 +49,7 @@ <div id="guest-processor"> <input id="guest-edit-cores-textbox" class="form-control" name="vcpus" type="number" min="1" /> <button id="guest-show-max-processor" class="btn btn-primary" type="button"><i class="fa fa-plus-circle"></i> <span class="cputext">$_("More")</span></button> + <p id="guest-edit-cores-hotplug-unsupported" class="help-block hidden"><i class="fa fa-info-circle"></i> $_("Guest must support CPU hotplug. Check Help page for details.")</p> </div> </div> <div id="guest-max-processor-panel" class="form-group"> diff --git a/ui/pages/help/en_US/guests.dita b/ui/pages/help/en_US/guests.dita index 45cf7da..5c71694 100644 --- a/ui/pages/help/en_US/guests.dita +++ b/ui/pages/help/en_US/guests.dita @@ -77,6 +77,14 @@ create a template.</li> can be edited only while guest is stopped. Others will take effect in next boot.</shortdesc> <csbody> +<ol><li><uicontrol>If guest is off: </uicontrol>CPU and Memory new values +takes effect after next boot.</li> +<li><uicontrol>If guest is running: </uicontrol>Guest must support +hotplug/hotunplug of CPU and Memory devices, otherwise changes in these fields will not take effect. For Power systems, the guest must have:<ul> +<li>Latest versions of packages <uicontrol>powerpc-utils, ppc64-diag</uicontrol> and <uicontrol>librtas</uicontrol> +must be installed.</li> +<li>Service <uicontrol>rtas_err</uicontrol> must be running.</li></ul> +</li></ol> <dl><dlentry> <dt>General</dt> <dd>Displays information about your guest, including name, CPUs, memory, -- 2.7.4

Looks good. Only a comment in first patch. Reviewed-By: Lucio Correia <luciojhc@linux.vnet.ibm.com> On 08/11/2016 09:49, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch set implements CPU Hot plug/unplug capabilities in Kimchi.
To test it, simply add/remove CPUs in a running guest via the 'Edit' menu. Note that all restrictions on the current CPU value (can't exceed max number of CPUs, must be a number that makes sense in the topology if one is set) still applies.
For Power systems, the hot unplug requires additional software running in the guest to work: powerpc-utils, ppc64-diag and librtas. The service 'rtas_err' must be running too.
Further enhancements and bug fixes in the CPU handling when editing a turned off VM (such as #1042) will be send in a separated patch.
Daniel Henrique Barboza (4): CPU Hot plug/unplug: i18n changes CPU Hot plug/unplug: model changes CPU Hot plug/unplug: test changes CPU Hot plug/unplug: ui changes
i18n.py | 5 +++ model/vms.py | 50 ++++++++++++++++++++++- tests/test_model.py | 80 +++++++++++++++++++++++++++++++++++++ ui/js/src/kimchi.guest_edit_main.js | 12 +++--- ui/pages/guest-edit.html.tmpl | 1 + ui/pages/help/en_US/guests.dita | 8 ++++ 6 files changed, 148 insertions(+), 8 deletions(-)
-- Lucio Correia Software Engineer IBM LTC Brazil
participants (3)
-
Daniel Henrique Barboza
-
dhbarboza82@gmail.com
-
Lucio Correia