From fd181cecc3b2019c2eb8ef42fde1f5b0f7170d57 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Thu, 17 Feb 2022 15:33:09 -0500 Subject: [PATCH] virtManager: Split out tpmdetails.py and tweak it Split out tpmdetails.py, following the pattern of fsdetails.py. This adds more UI editing fields for an already attached TPM. Move the model and version under an 'Advanced options' expander, since we should be getting this correct by default. Signed-off-by: Cole Robinson --- tests/uitests/test_addhardware.py | 7 +- tests/uitests/test_details.py | 5 +- ui/addhardware.ui | 108 +----------------- ui/details.ui | 121 +------------------- ui/tpmdetails.ui | 179 +++++++++++++++++++++++++++++ virtManager/addhardware.py | 104 ++--------------- virtManager/details/details.py | 37 ++---- virtManager/device/tpmdetails.py | 183 ++++++++++++++++++++++++++++++ virtManager/object/domain.py | 9 +- 9 files changed, 399 insertions(+), 354 deletions(-) create mode 100644 ui/tpmdetails.ui create mode 100644 virtManager/device/tpmdetails.py diff --git a/tests/uitests/test_addhardware.py b/tests/uitests/test_addhardware.py index 2814f4367..3e3136eed 100644 --- a/tests/uitests/test_addhardware.py +++ b/tests/uitests/test_addhardware.py @@ -610,7 +610,7 @@ def testAddHWMisc1(app): tab.combo_select("Mode:", "Passthrough") _finish(addhw, check=details) - # Add TPM emulated + # Add TPM default _open_addhw(app, details) tab = _select_hw(addhw, "TPM", "tpm-tab") _finish(addhw, check=details) @@ -645,9 +645,10 @@ def testAddHWMisc2(app): # Add TPM passthrough _open_addhw(app, details) tab = _select_hw(addhw, "TPM", "tpm-tab") - tab.combo_select("Model:", "TIS") - tab.combo_select("Backend:", "Passthrough") + tab.combo_select("Type:", "Passthrough") tab.find("Device Path:", "text").set_text("/tmp/foo") + tab.find("Advanced options", "toggle button").click_expander() + tab.combo_select("Model:", "TIS") _finish(addhw, check=details) # Add RNG diff --git a/tests/uitests/test_details.py b/tests/uitests/test_details.py index 0ecc438a8..0110b2243 100644 --- a/tests/uitests/test_details.py +++ b/tests/uitests/test_details.py @@ -632,7 +632,10 @@ def testDetailsEditDevices2(app): # TPM tweaks tab = _select_hw(app, win, "TPM", "tpm-tab") - tab.combo_select("tpm-model", "CRB") + tab.combo_select("Type:", "Emulated") + tab.find("Advanced options", "toggle button").click_expander() + tab.combo_select("Model:", "CRB") + tab.combo_select("Version:", "2.0") appl.click() lib.utils.check(lambda: not appl.sensitive) diff --git a/ui/addhardware.ui b/ui/addhardware.ui index 30cbb6c0c..3bd1637c0 100644 --- a/ui/addhardware.ui +++ b/ui/addhardware.ui @@ -1197,115 +1197,11 @@ - - + True False - 6 - 6 - - True - False - start - Device _Path: - True - tpm-device-path - - - 0 - 2 - - - - - True - True - ● - 12 - - - 1 - 2 - - - - - True - False - end - _Backend: - True - tpm-type - - - 0 - 1 - - - - - True - False - - - - 1 - 1 - - - - - True - False - end - _Model: - True - tpm-model - - - 0 - 0 - - - - - True - False - - - 1 - 0 - - - - - True - False - end - _Version: - True - tpm-version - - - 0 - 3 - - - - - True - False - - - 1 - 3 - - - - - tpm-tab - + diff --git a/ui/details.ui b/ui/details.ui index 39ce508b2..359f922e2 100644 --- a/ui/details.ui +++ b/ui/details.ui @@ -4418,130 +4418,13 @@ 0 none - + True False 3 12 - - - True - False - 3 - 4 - 8 - - - True - False - start - tpm-dev-type - - - 1 - 0 - - - - - True - False - start - tpm-device-path - - - 1 - 1 - - - - - True - False - end - Type: - - - 0 - 0 - - - - - True - False - end - Path: - - - 0 - 1 - - - - - True - False - end - Version: - - - 0 - 2 - - - - - True - False - start - tpm-version - - - 1 - 2 - - - - - True - False - end - Device mode_l: - True - tpm-model-text - - - 0 - 3 - - - - - True - False - start - False - True - - - - True - - - - - tpm-model - - - - - 1 - 3 - - - + diff --git a/ui/tpmdetails.ui b/ui/tpmdetails.ui new file mode 100644 index 000000000..c34c11208 --- /dev/null +++ b/ui/tpmdetails.ui @@ -0,0 +1,179 @@ + + + + + + True + False + vertical + 12 + + + + True + False + 6 + 6 + + + True + False + start + Device _Path: + True + tpm-device-path + + + 0 + 1 + + + + + True + True + ● + 12 + + + + 1 + 1 + + + + + True + False + end + _Type: + True + tpm-type + + + 0 + 0 + + + + + True + False + True + + + + False + + + + + 1 + 0 + + + + + False + True + 0 + + + + + True + True + + + + True + False + 6 + 6 + 6 + + + True + False + end + _Model: + True + tpm-model + + + 0 + 0 + + + + + True + False + True + + + + False + + + + + 1 + 0 + + + + + True + False + end + _Version: + True + tpm-version + + + 0 + 1 + + + + + True + False + True + + + + False + + + + + 1 + 1 + + + + + + + True + False + Adva_nced options + True + + + + + False + True + 1 + + + + + tpm-tab + + + + diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py index 1f257db45..bcf44bd4e 100644 --- a/virtManager/addhardware.py +++ b/virtManager/addhardware.py @@ -12,7 +12,7 @@ from virtinst import (DeviceChannel, DeviceConsole, DeviceController, DeviceDisk, DeviceHostdev, DeviceInput, DeviceInterface, DevicePanic, DeviceParallel, DeviceRedirdev, DeviceRng, DeviceSerial, DeviceSmartcard, - DeviceSound, DeviceTpm, DeviceVideo, DeviceVsock, DeviceWatchdog) + DeviceSound, DeviceVideo, DeviceVsock, DeviceWatchdog) from virtinst import log from .lib import uiutil @@ -22,6 +22,7 @@ from .device.addstorage import vmmAddStorage from .device.fsdetails import vmmFSDetails from .device.gfxdetails import vmmGraphicsDetails from .device.netlist import vmmNetworkList +from .device.tpmdetails import vmmTPMDetails from .device.vsockdetails import vmmVsockDetails from .storagebrowse import vmmStorageBrowser from .xmleditor import vmmXMLEditor @@ -78,6 +79,9 @@ class vmmAddHardware(vmmGObjectUI): self._vsockdetails = vmmVsockDetails(self.vm, self.builder, self.topwin) self.widget("vsock-align").add(self._vsockdetails.top_box) + self._tpmdetails = vmmTPMDetails(self.vm, self.builder, self.topwin) + self.widget("tpm-align").add(self._tpmdetails.top_box) + self._xmleditor = vmmXMLEditor(self.builder, self.topwin, self.widget("create-pages-align"), self.widget("create-pages")) @@ -99,8 +103,6 @@ class vmmAddHardware(vmmGObjectUI): "on_char_target_name_changed": self._change_char_target_name, "on_char_auto_socket_toggled": self._change_char_auto_socket, - "on_tpm_device_type_changed": self._change_tpm_device_type, - "on_usbredir_type_changed": self._change_usbredir_type, "on_controller_type_changed": self._change_controller_type, @@ -145,6 +147,8 @@ class vmmAddHardware(vmmGObjectUI): self.addstorage = None self._vsockdetails.cleanup() self._vsockdetails = None + self._tpmdetails.cleanup() + self._tpmdetails = None self._xmleditor.cleanup() self._xmleditor = None @@ -188,7 +192,6 @@ class vmmAddHardware(vmmGObjectUI): self.build_watchdogaction_combo(self.vm, self.widget("watchdog-action")) self.build_smartcard_mode_combo(self.vm, self.widget("smartcard-mode")) self._build_redir_type_combo() - self._build_tpm_type_combo(self.vm) self._build_panic_model_combo() uiutil.build_simple_combo(self.widget("controller-model"), []) self._build_controller_type_combo() @@ -316,9 +319,9 @@ class vmmAddHardware(vmmGObjectUI): # Remaining devices self._fsdetails.reset_state() - self.widget("tpm-device-path").set_text("/dev/tpm0") self._gfxdetails.reset_state() self._vsockdetails.reset_state() + self._tpmdetails.reset_state() @staticmethod def change_config_helper(define_func, define_args, vm, err, @@ -506,23 +509,6 @@ class vmmAddHardware(vmmGObjectUI): } return bus_mappings.get(bus, bus) - @staticmethod - def tpm_pretty_type(val): - labels = { - DeviceTpm.TYPE_PASSTHROUGH: _("Passthrough device"), - DeviceTpm.TYPE_EMULATOR: _("Emulated device"), - } - return labels.get(val, val) - - @staticmethod - def tpm_pretty_model(val): - labels = { - DeviceTpm.MODEL_TIS: _("TIS"), - DeviceTpm.MODEL_CRB: _("CRB"), - DeviceTpm.MODEL_SPAPR: _("SPAPR"), - } - return labels.get(val, val) - @staticmethod def panic_pretty_model(val): labels = { @@ -881,50 +867,6 @@ class vmmAddHardware(vmmGObjectUI): uiutil.build_simple_combo(self.widget("usbredir-list"), values) - def _build_tpm_type_combo(self, vm): - values = [] - for t in DeviceTpm.TYPES: - values.append([t, vmmAddHardware.tpm_pretty_type(t)]) - uiutil.build_simple_combo(self.widget("tpm-type"), values) - values = [] - for t in vmmAddHardware._get_tpm_model_list(vm, None): - values.append([t, vmmAddHardware.tpm_pretty_model(t)]) - uiutil.build_simple_combo(self.widget("tpm-model"), values) - values = [] - for t in DeviceTpm.VERSIONS: - values.append([t, t]) - uiutil.build_simple_combo(self.widget("tpm-version"), values, - default_value=DeviceTpm.VERSION_2_0) - - @staticmethod - def _get_tpm_model_list(vm, tpmversion): - mod_list = [] - if vm.is_hvm(): - if vm.xmlobj.os.is_pseries(): - mod_list.append("tpm-spapr") - else: - mod_list.append("tpm-tis") - if tpmversion != '1.2': - mod_list.append("tpm-crb") - mod_list.sort() - return mod_list - - @staticmethod - def populate_tpm_model_combo(vm, combo, tpmversion): - model = combo.get_model() - model.clear() - - mod_list = vmmAddHardware._get_tpm_model_list(vm, tpmversion) - for m in mod_list: - model.append([m, vmmAddHardware.tpm_pretty_model(m)]) - combo.set_active(0) - - @staticmethod - def build_tpm_model_combo(vm, combo, tpmversion): - uiutil.build_simple_combo(combo, []) - vmmAddHardware.populate_tpm_model_combo(vm, combo, tpmversion) - - def _build_panic_model_combo(self): values = [] for m in DevicePanic.get_models(self.vm.get_xmlobj()): @@ -1131,19 +1073,6 @@ class vmmAddHardware(vmmGObjectUI): else: self.widget("create-mac-address").set_sensitive(False) - def _change_tpm_device_type(self, src): - devtype = uiutil.get_list_selection(src) - if devtype is None: - return # pragma: no cover - - dev = DeviceTpm(self.conn.get_backend()) - dev.type = devtype - - uiutil.set_grid_row_visible(self.widget("tpm-device-path-label"), - devtype == dev.TYPE_PASSTHROUGH) - uiutil.set_grid_row_visible(self.widget("tpm-version-label"), - devtype == dev.TYPE_EMULATOR) - def _change_char_auto_socket(self, src): if not src.get_visible(): return @@ -1591,22 +1520,7 @@ class vmmAddHardware(vmmGObjectUI): return dev def _build_tpm(self): - typ = uiutil.get_list_selection(self.widget("tpm-type")) - model = uiutil.get_list_selection(self.widget("tpm-model")) - device_path = self.widget("tpm-device-path").get_text() - version = uiutil.get_list_selection(self.widget("tpm-version")) - - if not self.widget("tpm-device-path").get_visible(): - device_path = None - if not self.widget("tpm-version").get_visible(): - version = None - - dev = DeviceTpm(self.conn.get_backend()) - dev.type = typ - dev.model = model - dev.device_path = device_path - dev.version = version - return dev + return self._tpmdetails.build_device() def _build_panic(self): model = uiutil.get_list_selection(self.widget("panic-model")) diff --git a/virtManager/details/details.py b/virtManager/details/details.py index 5bc0f51bf..2b483409e 100644 --- a/virtManager/details/details.py +++ b/virtManager/details/details.py @@ -21,6 +21,7 @@ from ..device.fsdetails import vmmFSDetails from ..device.gfxdetails import vmmGraphicsDetails from ..device.mediacombo import vmmMediaCombo from ..device.netlist import vmmNetworkList +from ..device.tpmdetails import vmmTPMDetails from ..device.vsockdetails import vmmVsockDetails from ..lib.graphwidgets import Sparkline from ..oslist import vmmOSList @@ -76,15 +77,14 @@ from ..delete import vmmDeleteStorage EDIT_CONTROLLER_MODEL, - EDIT_TPM_TYPE, - EDIT_TPM_MODEL, + EDIT_TPM, EDIT_VSOCK_AUTO, EDIT_VSOCK_CID, EDIT_FS, - EDIT_HOSTDEV_ROMBAR) = range(1, 39) + EDIT_HOSTDEV_ROMBAR) = range(1, 38) # Columns in hw list model @@ -377,6 +377,10 @@ class vmmDetails(vmmGObjectUI): self.widget("network-source-ui-align").add(self.netlist.top_box) self.netlist.connect("changed", _e(EDIT_NET_SOURCE)) + self.tpmdetails = vmmTPMDetails(self.vm, self.builder, self.topwin) + self.widget("tpm-align").add(self.tpmdetails.top_box) + self.tpmdetails.connect("changed", _e(EDIT_TPM)) + self.vsockdetails = vmmVsockDetails(self.vm, self.builder, self.topwin) self.widget("vsock-align").add(self.vsockdetails.top_box) self.vsockdetails.connect("changed-auto-cid", _e(EDIT_VSOCK_AUTO)) @@ -471,7 +475,6 @@ class vmmDetails(vmmGObjectUI): "on_hostdev_rombar_toggled": _e(EDIT_HOSTDEV_ROMBAR), "on_controller_model_combo_changed": _e(EDIT_CONTROLLER_MODEL), - "on_tpm_model_combo_changed": _e(EDIT_TPM_MODEL), "on_config_apply_clicked": self._config_apply_clicked_cb, "on_config_cancel_clicked": self._config_cancel_clicked_cb, @@ -841,10 +844,6 @@ class vmmDetails(vmmGObjectUI): sc_mode = self.widget("smartcard-mode") vmmAddHardware.build_smartcard_mode_combo(self.vm, sc_mode) - # TPM model - tpm_model = self.widget("tpm-model") - vmmAddHardware.build_tpm_model_combo(self.vm, tpm_model, None) - # Controller model combo = self.widget("controller-model") model = Gtk.ListStore(str, str) @@ -1662,9 +1661,8 @@ class vmmDetails(vmmGObjectUI): def _apply_tpm(self, devobj): kwargs = {} - if self._edited(EDIT_TPM_MODEL): - model = uiutil.get_list_selection(self.widget("tpm-model")) - kwargs["model"] = model + if self._edited(EDIT_TPM): + kwargs["newdev"] = self.tpmdetails.update_device(devobj) return self._change_config( self.vm.define_tpm, kwargs, devobj=devobj) @@ -2082,22 +2080,7 @@ class vmmDetails(vmmGObjectUI): self.widget("redir-address"), bool(address)) def _refresh_tpm_page(self, tpmdev): - def show_ui(widgetname, val): - doshow = bool(val) - uiutil.set_grid_row_visible(self.widget(widgetname), doshow) - self.widget(widgetname).set_text(val or "-") - - dev_type = tpmdev.type - self.widget("tpm-dev-type").set_text( - vmmAddHardware.tpm_pretty_type(dev_type)) - - vmmAddHardware.populate_tpm_model_combo( - self.vm, self.widget("tpm-model"), tpmdev.version) - uiutil.set_list_selection(self.widget("tpm-model"), tpmdev.model) - - # Device type specific properties, only show if apply to the cur dev - show_ui("tpm-device-path", tpmdev.device_path) - show_ui("tpm-version", tpmdev.version) + self.tpmdetails.set_dev(tpmdev) def _refresh_panic_page(self, dev): model = dev.model or "isa" diff --git a/virtManager/device/tpmdetails.py b/virtManager/device/tpmdetails.py new file mode 100644 index 000000000..34f1fadb0 --- /dev/null +++ b/virtManager/device/tpmdetails.py @@ -0,0 +1,183 @@ +# This work is licensed under the GNU GPLv2 or later. +# See the COPYING file in the top-level directory. + +from virtinst import DeviceTpm + +from ..lib import uiutil +from ..baseclass import vmmGObjectUI + + +def _tpm_pretty_model(val): + labels = { + DeviceTpm.MODEL_TIS: _("TIS"), + DeviceTpm.MODEL_CRB: _("CRB"), + DeviceTpm.MODEL_SPAPR: _("SPAPR"), + } + return labels.get(val, val) + + +_EDIT_TPM_ENUM = range(1, 5) +( + _EDIT_TPM_TYPE, + _EDIT_TPM_DEVICE_PATH, + _EDIT_TPM_MODEL, + _EDIT_TPM_VERSION, +) = _EDIT_TPM_ENUM + + +class vmmTPMDetails(vmmGObjectUI): + __gsignals__ = { + "changed": (vmmGObjectUI.RUN_FIRST, None, []), + } + + def __init__(self, vm, builder, topwin): + super().__init__("tpmdetails.ui", None, + builder=builder, topwin=topwin) + self.vm = vm + self.conn = vm.conn + + self._active_edits = [] + + def _e(edittype): + def signal_cb(*args): + self._change_cb(edittype) + return signal_cb + + self.builder.connect_signals({ + "on_tpm_type_changed": _e(_EDIT_TPM_TYPE), + "on_tpm_device_path_changed": _e(_EDIT_TPM_DEVICE_PATH), + "on_tpm_model_changed": _e(_EDIT_TPM_MODEL), + "on_tpm_version_changed": _e(_EDIT_TPM_VERSION), + }) + + self._init_ui() + self.top_box = self.widget("top-box") + + def _cleanup(self): + self.vm = None + self.conn = None + + + ############## + # UI helpers # + ############## + + def _init_ui(self): + domcaps = self.vm.get_domain_capabilities() + + # We could check domcaps for this, but emulated is really the + # preferred default here, so just let it fail + rows = [ + [DeviceTpm.TYPE_EMULATOR, _("Emulated")], + [DeviceTpm.TYPE_PASSTHROUGH, _("Passthrough")], + ] + uiutil.build_simple_combo(self.widget("tpm-type"), rows, sort=False) + + rows = [[None, _("Hypervisor default")]] + if domcaps.devices.tpm.present: + values = domcaps.devices.tpm.get_enum("model").get_values() + else: + values = [DeviceTpm.MODEL_CRB, DeviceTpm.MODEL_TIS] + for v in values: + rows.append([v, _tpm_pretty_model(v)]) + + uiutil.build_simple_combo(self.widget("tpm-model"), rows, sort=False) + + rows = [ + [None, _("Hypervisor default")], + [DeviceTpm.VERSION_2_0, DeviceTpm.VERSION_2_0], + [DeviceTpm.VERSION_1_2, DeviceTpm.VERSION_1_2], + ] + uiutil.build_simple_combo(self.widget("tpm-version"), rows, sort=False) + + + def _sync_ui(self): + devtype = uiutil.get_list_selection(self.widget("tpm-type")) + + uiutil.set_grid_row_visible(self.widget("tpm-device-path"), + devtype == DeviceTpm.TYPE_PASSTHROUGH) + uiutil.set_grid_row_visible(self.widget("tpm-version"), + devtype == DeviceTpm.TYPE_EMULATOR) + + + ################## + # Public UI APIs # + ################## + + def reset_state(self): + self.widget("tpm-device-path").set_text("/dev/tpm0") + uiutil.set_list_selection( + self.widget("tpm-type"), DeviceTpm.TYPE_EMULATOR) + + default_model = DeviceTpm.default_model(self.vm.xmlobj) + uiutil.set_list_selection( + self.widget("tpm-model"), default_model) + uiutil.set_list_selection( + self.widget("tpm-version"), None) + + + def set_dev(self, dev): + self.reset_state() + + uiutil.set_list_selection( + self.widget("tpm-type"), dev.type) + uiutil.set_list_selection( + self.widget("tpm-model"), dev.model) + uiutil.set_list_selection( + self.widget("tpm-version"), dev.version) + self.widget("tpm-device-path").set_text(dev.device_path or "") + + self._active_edits = [] + + + ######################## + # Device building APIs # + ######################## + + def _set_values(self, dev): + typ = uiutil.get_list_selection(self.widget("tpm-type")) + model = uiutil.get_list_selection(self.widget("tpm-model")) + device_path = self.widget("tpm-device-path").get_text() + version = uiutil.get_list_selection(self.widget("tpm-version")) + + if not self.widget("tpm-device-path").get_visible(): + device_path = None + if not self.widget("tpm-version").get_visible(): + version = None + + if _EDIT_TPM_TYPE in self._active_edits: + dev.type = typ + if _EDIT_TPM_MODEL in self._active_edits: + dev.model = model + if _EDIT_TPM_DEVICE_PATH in self._active_edits: + dev.device_path = device_path + if _EDIT_TPM_VERSION in self._active_edits: + dev.version = version + + return dev + + def build_device(self): + self._active_edits = _EDIT_TPM_ENUM[:] + + conn = self.conn.get_backend() + dev = DeviceTpm(conn) + self._set_values(dev) + + dev.validate() + return dev + + def update_device(self, dev): + newdev = DeviceTpm(dev.conn, parsexml=dev.get_xml()) + self._set_values(newdev) + return newdev + + + ############# + # Listeners # + ############# + + def _change_cb(self, edittype): + self._sync_ui() + if edittype not in self._active_edits: + self._active_edits.append(edittype) + self.emit("changed") diff --git a/virtManager/object/domain.py b/virtManager/object/domain.py index 655e74634..c1669c1ff 100644 --- a/virtManager/object/domain.py +++ b/virtManager/object/domain.py @@ -999,14 +999,17 @@ class vmmDomain(vmmLibvirtObject): self._process_device_define(editdev, xmlobj, do_hotplug) - def define_tpm(self, devobj, do_hotplug, model=_SENTINEL): + def define_tpm(self, devobj, do_hotplug, newdev=_SENTINEL): xmlobj = self._make_xmlobj_to_define() editdev = self._lookup_device_to_define(xmlobj, devobj, do_hotplug) if not editdev: return # pragma: no cover - if model != _SENTINEL: - editdev.model = model + if newdev != _SENTINEL: + editdev.model = newdev.model + editdev.type = newdev.type + editdev.version = newdev.version + editdev.device_path = newdev.device_path self._process_device_define(editdev, xmlobj, do_hotplug)