On 2/1/22 8:43 AM, Shalini Chellathurai Saroja wrote:
> With virt-manager application, it is possible to add inactive node
> devices(eg: mediated device) in host system to guest system. But it is
> impossible to start a guest system with inactive node devices. Also,
> it is not yet possible to start a node device with virt-manager
> application. So, the user cannot use the inactive node devices.
>
> This patch disables the "finish" button and provides a tip, when
> inactive node devices are selected. So, it is not possible to add
> inactive node devices to the guest system.
>
> Signed-off-by: Shalini Chellathurai Saroja <shalini@xxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
> ---
> virtManager/addhardware.py | 42 +++++++++++++++++++++++++++++++++++---
> 1 file changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
> index 132ba4e0..60b58741 100644
> --- a/virtManager/addhardware.py
> +++ b/virtManager/addhardware.py
> @@ -754,7 +754,7 @@ class vmmAddHardware(vmmGObjectUI):
> def _build_hostdev_treeview(self):
> host_dev = self.widget("host-device")
> # [ xmlobj, label]
> - host_dev_model = Gtk.ListStore(object, str)
> + host_dev_model = Gtk.ListStore(object, str, bool)
> host_dev.set_model(host_dev_model)
> host_col = Gtk.TreeViewColumn()
> text = Gtk.CellRendererText()
Add this here to make the text visibly insensitive:
diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
index 60b58741..5e52c1ea 100644
--- a/virtManager/addhardware.py
+++ b/virtManager/addhardware.py
@@ -760,6 +760,7 @@ class vmmAddHardware(vmmGObjectUI):
text = Gtk.CellRendererText()
host_col.pack_start(text, True)
host_col.add_attribute(text, 'text', 1)
+ host_col.add_attribute(text, 'sensitive', 2)
host_dev_model.set_sort_column_id(1, Gtk.SortType.ASCENDING)
host_dev.append_column(host_col)
> @@ -763,6 +763,28 @@ class vmmAddHardware(vmmGObjectUI):
> host_dev_model.set_sort_column_id(1, Gtk.SortType.ASCENDING)
> host_dev.append_column(host_col)
>
> +
> + def disable_finish_if_inactive(self, selection):
Please name this _hostdev_row_selected_cb or something describing the
context it is called from.
> + model, row = selection.get_selected()
> +
the 'row' here is a 'treeiter'
> + if row is None:
> + return
> +
> + hostdev = model[row]
calling this variable 'hostdev' is confusing. This should be called
'row' IMO
> + if hostdev[1] is None:
> + self.widget("create-finish").set_sensitive(False)
> + self.widget("create-finish").set_tooltip_text()
> + elif hostdev[2]:
> + self.widget("create-finish").set_sensitive(True)
> + self.widget("create-finish").set_tooltip_text()
> + else:
> + tooltip = (_("%s is not active in the host system.\n"
> + "Please start the mdev in the host system before adding it to the guest.")
> + % hostdev[1])
> + self.widget("create-finish").set_sensitive(False)
> + self.widget("create-finish").set_tooltip_text(tooltip)
> +
I prefer if there's only a single set_sensitive and set_tooltip_text
call. The state is basically
tooltip = None
sensitive = row[2]
if row[1] and sensitive is False:
tooltip = ...
set_tooltip_text(tooltip)
set_sensitive(sensitive)
> +
> def _populate_hostdev_model(self, devtype):
> devlist = self.widget("host-device")
> model = devlist.get_model()
> @@ -790,12 +812,26 @@ class vmmAddHardware(vmmGObjectUI):
> prettyname = "%s %s" % (
> parentdev.pretty_name(), prettyname)
>
> - model.append([dev.xmlobj, prettyname])
> + model.append([dev.xmlobj, prettyname, dev.is_active()])
>
> if len(model) == 0:
> - model.append([None, _("No Devices Available")])
> + model.append([None, _("No Devices Available"), False])
> +
> uiutil.set_list_selection_by_number(devlist, 0)
>
> + # enable/disable finish button for default selection
> + if model[0][2]:
> + self.widget("create-finish").set_sensitive(True)
> + self.widget("create-finish").set_tooltip_text()
> + else:
> + tooltip = (_("%s is not active in the host system.\n"
> + "Please start the mdev in the host system before adding it to the guest.")
> + % model[0][1])
> + self.widget("create-finish").set_sensitive(False)
> + self.widget("create-finish").set_tooltip_text(tooltip)
> +
> + devlist.get_selection().connect("changed", self.disable_finish_if_inactive)
> +
Please break this long line here.
Duplication like that is a sign that something is wrong :)
You can trigger the callback by doing:
devlist.get_selection().emit("changed")
Thanks,
Cole
