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 <crobinso@redhat.com>
This commit is contained in:
Cole Robinson 2020-01-25 15:11:02 -05:00
parent b4b497e28f
commit 0335c9ce62
4 changed files with 70 additions and 143 deletions

View File

@ -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")

View File

@ -61,13 +61,6 @@
<property name="step_increment">1</property>
<property name="page_increment">10</property>
</object>
<object class="GtkAdjustment" id="adjustment6">
<property name="lower">1</property>
<property name="upper">1024</property>
<property name="value">1</property>
<property name="step_increment">1</property>
<property name="page_increment">2</property>
</object>
<object class="GtkAdjustment" id="adjustment7">
<property name="lower">1</property>
<property name="upper">1024</property>
@ -1451,28 +1444,13 @@
<property name="top_attach">0</property>
</packing>
</child>
<child>
<object class="GtkLabel" id="label335">
<property name="visible">True</property>
<property name="can_focus">False</property>
<property name="halign">start</property>
<property name="valign">center</property>
<property name="label" translatable="yes">Ma_ximum allocation:</property>
<property name="use_underline">True</property>
<property name="mnemonic_widget">cpu-maxvcpus</property>
</object>
<packing>
<property name="left_attach">0</property>
<property name="top_attach">2</property>
</packing>
</child>
<child>
<object class="GtkLabel" id="label333">
<property name="visible">True</property>
<property name="can_focus">False</property>
<property name="halign">start</property>
<property name="valign">center</property>
<property name="label" translatable="yes">Current a_llocation:</property>
<property name="label" translatable="yes">VCPU a_llocation:</property>
<property name="use_underline">True</property>
<property name="mnemonic_widget">cpu-vcpus</property>
</object>
@ -1481,26 +1459,13 @@
<property name="top_attach">1</property>
</packing>
</child>
<child>
<object class="GtkSpinButton" id="cpu-maxvcpus">
<property name="visible">True</property>
<property name="can_focus">True</property>
<property name="invisible_char">●</property>
<property name="text" translatable="yes">1</property>
<property name="adjustment">adjustment6</property>
<property name="value">1</property>
<signal name="changed" handler="on_cpu_maxvcpus_changed" swapped="no"/>
</object>
<packing>
<property name="left_attach">1</property>
<property name="top_attach">2</property>
</packing>
</child>
<child>
<object class="GtkSpinButton" id="cpu-vcpus">
<property name="visible">True</property>
<property name="can_focus">True</property>
<property name="halign">start</property>
<property name="valign">center</property>
<property name="hexpand">True</property>
<property name="invisible_char">●</property>
<property name="text" translatable="yes">2</property>
<property name="adjustment">adjustment7</property>
@ -1520,6 +1485,45 @@
<property name="top_attach">1</property>
</packing>
</child>
<child>
<object class="GtkBox" id="cpu-vcpus-warn-box">
<property name="visible">True</property>
<property name="can_focus">False</property>
<property name="halign">start</property>
<property name="spacing">6</property>
<child>
<object class="GtkImage" id="image9">
<property name="visible">True</property>
<property name="can_focus">False</property>
<property name="halign">start</property>
<property name="stock">gtk-dialog-warning</property>
</object>
<packing>
<property name="expand">False</property>
<property name="fill">False</property>
<property name="position">0</property>
</packing>
</child>
<child>
<object class="GtkLabel">
<property name="visible">True</property>
<property name="can_focus">False</property>
<property name="label" translatable="yes">&lt;small&gt;Overcommitting vCPUs can hurt performance&lt;/small&gt;</property>
<property name="use_markup">True</property>
</object>
<packing>
<property name="expand">True</property>
<property name="fill">True</property>
<property name="position">1</property>
</packing>
</child>
</object>
<packing>
<property name="left_attach">0</property>
<property name="top_attach">2</property>
<property name="width">2</property>
</packing>
</child>
</object>
<packing>
<property name="expand">False</property>
@ -1527,48 +1531,6 @@
<property name="position">0</property>
</packing>
</child>
<child>
<object class="GtkBox" id="cpu-vcpus-warn-box">
<property name="visible">True</property>
<property name="can_focus">False</property>
<property name="spacing">6</property>
<child>
<object class="GtkImage" id="image9">
<property name="visible">True</property>
<property name="can_focus">False</property>
<property name="halign">start</property>
<property name="stock">gtk-dialog-warning</property>
</object>
<packing>
<property name="expand">False</property>
<property name="fill">False</property>
<property name="position">0</property>
</packing>
</child>
<child>
<object class="GtkLabel" id="label28">
<property name="width_request">300</property>
<property name="visible">True</property>
<property name="can_focus">False</property>
<property name="halign">start</property>
<property name="label" translatable="yes">&lt;small&gt;Overcommitting vCPUs can hurt performance&lt;/small&gt;</property>
<property name="use_markup">True</property>
<property name="wrap">True</property>
<property name="max_width_chars">32</property>
</object>
<packing>
<property name="expand">True</property>
<property name="fill">True</property>
<property name="position">1</property>
</packing>
</child>
</object>
<packing>
<property name="expand">True</property>
<property name="fill">True</property>
<property name="position">1</property>
</packing>
</child>
</object>
</child>
</object>
@ -3187,6 +3149,12 @@
<property name="top_attach">3</property>
</packing>
</child>
<child>
<placeholder/>
</child>
<child>
<placeholder/>
</child>
</object>
</child>
<child type="label">

View File

@ -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

View File

@ -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