From 0335c9ce62d7c9d716756e933d0ab9275a589616 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Sat, 25 Jan 2020 15:11:02 -0500 Subject: [PATCH] virtManager: Remove maximum VCPUS API and VCPU hotplug This was proposed here: https://www.redhat.com/archives/virt-tools-list/2019-June/msg00117.html """ * UI maxmem and maxcpu notions, and related memballoon and cpu hotplug operations. These have been in the UI forever but I'm not sure people actually use them. cpu hotplug has always been a mess, and unless the user plans ahead by setting a high maxmem value ballooning is only good for reducing memory. These all sound like advanced usage to me that just confuses the typical usecase of adding more mem or vcpus to an offline VM. And the hotplug operations with virsh are simple to invoke. So I'd like to drop this from the UI """ The remaining UI field now sets both maximum and current VCPU allocation. Signed-off-by: Cole Robinson --- tests/uitests/details.py | 12 ++-- ui/details.ui | 128 +++++++++++++-------------------- virtManager/details/details.py | 56 ++++----------- virtManager/object/domain.py | 17 +---- 4 files changed, 70 insertions(+), 143 deletions(-) diff --git a/tests/uitests/details.py b/tests/uitests/details.py index a7c218f08..97655460c 100644 --- a/tests/uitests/details.py +++ b/tests/uitests/details.py @@ -93,7 +93,7 @@ class Details(uiutils.UITestCase): Test overview, memory, cpu pages """ self.app.uri = tests.utils.URIs.kvm - win = self._open_details_window(vmname="test-many-devices") + win = self._open_details_window(vmname="test") appl = win.find("config-apply", "push button") # Overview description @@ -112,15 +112,16 @@ class Details(uiutils.UITestCase): appl.click() uiutils.check_in_loop(lambda: not appl.sensitive) + # There's no hotplug operations after this point + self._stop_vm(win) # CPU hotplug tab = self._select_hw(win, "CPUs", "cpu-tab") - tab.find("Current allocation:", "spin button").text = "2" + tab.find("VCPU allocation:", "spin button").text = "4" appl.click() uiutils.check_in_loop(lambda: not appl.sensitive) # Static CPU config - self._stop_vm(win) # more cpu config: host-passthrough, copy, clear CPU, manual tab.find("cpu-model").click_combo_entry() tab.find_fuzzy("Clear CPU", "menu item").click() @@ -141,13 +142,14 @@ class Details(uiutils.UITestCase): uiutils.check_in_loop(lambda: not appl.sensitive) # CPU topology + tab.find_fuzzy("Topology", "toggle button").click_expander() tab.find_fuzzy("Manually set", "check").click() tab.find("Sockets:", "spin button").typeText("8") tab.find("Cores:", "spin button").typeText("2") tab.find("Threads:", "spin button").typeText("2") appl.click() uiutils.check_in_loop(lambda: not appl.sensitive) - self.assertTrue(tab.find_fuzzy("Maximum", "spin").text == "32") + self.assertTrue(tab.find_fuzzy("VCPU allocation", "spin").text == "32") def testDetailsEditDomain2(self): @@ -500,7 +502,7 @@ class Details(uiutils.UITestCase): finish.click() win.find("Details", "page tab").click() self.assertEqual( - tab.find("Current allocation:", "spin button").text, "8") + tab.find("VCPU allocation:", "spin button").text, "8") # Make some disk edits tab = self._select_hw(win, "IDE Disk 1", "disk-tab") diff --git a/ui/details.ui b/ui/details.ui index 9b9689445..c58d09f16 100644 --- a/ui/details.ui +++ b/ui/details.ui @@ -61,13 +61,6 @@ 1 10 - - 1 - 1024 - 1 - 1 - 2 - 1 1024 @@ -1451,28 +1444,13 @@ 0 - - - True - False - start - center - Ma_ximum allocation: - True - cpu-maxvcpus - - - 0 - 2 - - True False start center - Current a_llocation: + VCPU a_llocation: True cpu-vcpus @@ -1481,26 +1459,13 @@ 1 - - - True - True - ● - 1 - adjustment6 - 1 - - - - 1 - 2 - - True True + start center + True ● 2 adjustment7 @@ -1520,6 +1485,45 @@ 1 + + + True + False + start + 6 + + + True + False + start + gtk-dialog-warning + + + False + False + 0 + + + + + True + False + <small>Overcommitting vCPUs can hurt performance</small> + True + + + True + True + 1 + + + + + 0 + 2 + 2 + + False @@ -1527,48 +1531,6 @@ 0 - - - True - False - 6 - - - True - False - start - gtk-dialog-warning - - - False - False - 0 - - - - - 300 - True - False - start - <small>Overcommitting vCPUs can hurt performance</small> - True - True - 32 - - - True - True - 1 - - - - - True - True - 1 - - @@ -3187,6 +3149,12 @@ 3 + + + + + + diff --git a/virtManager/details/details.py b/virtManager/details/details.py index 5b9d99d1a..bcb8d4bd7 100644 --- a/virtManager/details/details.py +++ b/virtManager/details/details.py @@ -42,7 +42,6 @@ from ..xmleditor import vmmXMLEditor EDIT_OS_NAME, EDIT_VCPUS, - EDIT_MAXVCPUS, EDIT_CPU, EDIT_TOPOLOGY, @@ -96,7 +95,7 @@ from ..xmleditor import vmmXMLEditor EDIT_FS, - EDIT_HOSTDEV_ROMBAR) = range(1, 51) + EDIT_HOSTDEV_ROMBAR) = range(1, 50) # Columns in hw list model @@ -453,8 +452,7 @@ class vmmDetails(vmmGObjectUI): "on_details_inspection_refresh_clicked": self.inspection_refresh, - "on_cpu_vcpus_changed": self.config_vcpus_changed, - "on_cpu_maxvcpus_changed": self.config_maxvcpus_changed, + "on_cpu_vcpus_changed": self._config_vcpus_changed_cb, "on_cpu_model_changed": lambda *x: self.config_cpu_model_changed(x), "on_cpu_copy_host_clicked": self.on_cpu_copy_host_clicked, "on_cpu_secure_toggled": self.on_cpu_secure_toggled, @@ -1227,10 +1225,8 @@ class vmmDetails(vmmGObjectUI): # VCPUS def config_get_vcpus(self): return uiutil.spin_get_helper(self.widget("cpu-vcpus")) - def config_get_maxvcpus(self): - return uiutil.spin_get_helper(self.widget("cpu-maxvcpus")) - def config_vcpus_changed(self, src): + def _config_vcpus_changed_cb(self, src): self.enable_apply(EDIT_VCPUS) conn = self.vm.conn @@ -1241,26 +1237,6 @@ class vmmDetails(vmmGObjectUI): warn = bool(cur > host_active_count) self.widget("cpu-vcpus-warn-box").set_visible(warn) - maxadj = self.widget("cpu-maxvcpus") - maxval = self.config_get_maxvcpus() - if maxval < cur: - if maxadj.get_sensitive(): - maxadj.set_value(cur) - else: - src.set_value(maxval) - cur = maxval - ignore, upper = maxadj.get_range() - maxadj.set_range(cur, upper) - - def config_maxvcpus_changed(self, ignore): - if self.widget("cpu-maxvcpus").get_sensitive(): - self.config_cpu_topology_changed() - - # As this callback can be triggered by other events, set EDIT_MAXVCPUS - # only when the value is changed. - if self.config_get_maxvcpus() != self.vm.vcpu_max_count(): - self.enable_apply(EDIT_MAXVCPUS) - def on_cpu_copy_host_clicked(self, src): uiutil.set_grid_row_visible( self.widget("cpu-model"), not src.get_active()) @@ -1282,7 +1258,7 @@ class vmmDetails(vmmGObjectUI): def config_cpu_topology_changed(self, ignore=None): manual_top = self.widget("cpu-topology-table").is_sensitive() - self.widget("cpu-maxvcpus").set_sensitive(not manual_top) + self.widget("cpu-vcpus").set_sensitive(not manual_top) if manual_top: cores = uiutil.spin_get_helper(self.widget("cpu-cores")) or 1 @@ -1291,7 +1267,7 @@ class vmmDetails(vmmGObjectUI): total = cores * sockets * threads if uiutil.spin_get_helper(self.widget("cpu-vcpus")) > total: self.widget("cpu-vcpus").set_value(total) - self.widget("cpu-maxvcpus").set_value(total) + self.widget("cpu-vcpus").set_value(total) # Warn about hyper-threading setting cpu_model = self.get_config_cpu_model() @@ -1299,8 +1275,8 @@ class vmmDetails(vmmGObjectUI): self.widget("cpu-topology-warn-box").set_visible(warn_ht) else: - maxvcpus = uiutil.spin_get_helper(self.widget("cpu-maxvcpus")) - self.widget("cpu-sockets").set_value(maxvcpus or 1) + vcpus = uiutil.spin_get_helper(self.widget("cpu-vcpus")) + self.widget("cpu-sockets").set_value(vcpus or 1) self.widget("cpu-cores").set_value(1) self.widget("cpu-threads").set_value(1) @@ -1569,14 +1545,9 @@ class vmmDetails(vmmGObjectUI): def config_vcpus_apply(self): kwargs = {} - hotplug_args = {} if self.edited(EDIT_VCPUS): kwargs["vcpus"] = self.config_get_vcpus() - hotplug_args["vcpus"] = kwargs["vcpus"] - - if self.edited(EDIT_MAXVCPUS): - kwargs["maxvcpus"] = self.config_get_maxvcpus() if self.edited(EDIT_CPU): kwargs["model"] = self.get_config_cpu_model() @@ -1593,8 +1564,7 @@ class vmmDetails(vmmGObjectUI): kwargs["threads"] = None return vmmAddHardware.change_config_helper(self.vm.define_cpu, - kwargs, self.vm, self.err, - hotplug_args=hotplug_args) + kwargs, self.vm, self.err) def config_memory_apply(self): kwargs = {} @@ -2120,7 +2090,7 @@ class vmmDetails(vmmGObjectUI): return self.vm.get_xmlobj().os.is_x86() and len(features) > 0 def refresh_config_cpu(self): - # Set topology first, because it impacts maxvcpus values + # Set topology first, because it impacts vcpus values cpu = self.vm.get_cpu_config() show_top = bool(cpu.sockets or cpu.cores or cpu.threads) self.widget("cpu-topology-enable").set_active(show_top) @@ -2136,14 +2106,12 @@ class vmmDetails(vmmGObjectUI): self.widget("cpu-topology-expander").set_expanded(True) host_active_count = self.vm.conn.host_active_processor_count() - maxvcpus = self.vm.vcpu_max_count() - curvcpus = self.vm.vcpu_count() + vcpus = self.vm.xmlobj.vcpus - self.widget("cpu-vcpus").set_value(int(curvcpus)) - self.widget("cpu-maxvcpus").set_value(int(maxvcpus)) + self.widget("cpu-vcpus").set_value(int(vcpus)) self.widget("state-host-cpus").set_text(str(host_active_count)) - # Trigger this again to make sure maxvcpus is correct + # Trigger this again to make sure vcpus is correct self.config_cpu_topology_changed() # Warn about overcommit diff --git a/virtManager/object/domain.py b/virtManager/object/domain.py index b85391a10..7145f9462 100644 --- a/virtManager/object/domain.py +++ b/virtManager/object/domain.py @@ -540,15 +540,14 @@ class vmmDomain(vmmLibvirtObject): self._redefine_xmlobj(xmlobj) return editdev, newdev - def define_cpu(self, vcpus=_SENTINEL, maxvcpus=_SENTINEL, + def define_cpu(self, vcpus=_SENTINEL, model=_SENTINEL, secure=_SENTINEL, sockets=_SENTINEL, cores=_SENTINEL, threads=_SENTINEL): guest = self._make_xmlobj_to_define() if vcpus != _SENTINEL: + guest.vcpus = int(vcpus) guest.vcpu_current = int(vcpus) - if maxvcpus != _SENTINEL: - guest.vcpus = int(maxvcpus) if sockets != _SENTINEL: guest.cpu.sockets = sockets @@ -1002,7 +1001,7 @@ class vmmDomain(vmmLibvirtObject): log.debug("update_device with xml=\n%s", xml) self._backend.updateDeviceFlags(xml, flags) - def hotplug(self, vcpus=_SENTINEL, memory=_SENTINEL, maxmem=_SENTINEL, + def hotplug(self, memory=_SENTINEL, maxmem=_SENTINEL, description=_SENTINEL, title=_SENTINEL, device=_SENTINEL): if not self.is_active(): return @@ -1019,11 +1018,6 @@ class vmmDomain(vmmLibvirtObject): libvirt.VIR_DOMAIN_AFFECT_CONFIG) self._backend.setMetadata(mtype, val, None, None, flags) - if vcpus != _SENTINEL: - vcpus = int(vcpus) - if vcpus != self.vcpu_count(): - self._backend.setVcpus(vcpus) - if memory != _SENTINEL: log.debug("Hotplugging curmem=%s maxmem=%s for VM '%s'", memory, maxmem, self.get_name()) @@ -1316,11 +1310,6 @@ class vmmDomain(vmmLibvirtObject): def maximum_memory(self): return int(self.get_xmlobj().memory) - def vcpu_count(self): - return int(self.get_xmlobj().vcpu_current or self.get_xmlobj().vcpus) - def vcpu_max_count(self): - return int(self.get_xmlobj().vcpus) - def get_cpu_config(self): return self.get_xmlobj().cpu