[virt-tools-list] [PATCH 3/3] details: Add auto USB redirection support in console viewer
Leonardo Augusto Guimarães Garcia
lagarcia at linux.vnet.ibm.com
Wed Jun 26 18:51:51 UTC 2013
On 06/26/2013 11:13 AM, Guannan Ren wrote:
> Add "Select USB devices to redirect" option in console viewer
> Initialize and embed UsbDeviceWidget object from SpiceClientGtk into
> a dialog to let user choose available USB devices for redirection.
> Throw an error message if USB connection failed.
>
> Auto-redirection is enable by default.
> ---
> ui/vmm-details.ui | 9 ++++++
> virtManager/console.py | 85 ++++++++++++++++++++++++++++++++++++++++++++++++--
> virtManager/details.py | 18 ++++++++++-
> 3 files changed, 109 insertions(+), 3 deletions(-)
>
> diff --git a/ui/vmm-details.ui b/ui/vmm-details.ui
> index ea4b53e..e7a8dd5 100644
> --- a/ui/vmm-details.ui
> +++ b/ui/vmm-details.ui
> @@ -337,6 +337,15 @@
> <signal name="activate" handler="on_details_menu_screenshot_activate" swapped="no"/>
> </object>
> </child>
> + <child>
> + <object class="GtkMenuItem" id="details-menu-usb-redirection">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="label" translatable="yes">_Redirect USB device</property>
> + <property name="use_underline">True</property>
> + <signal name="activate" handler="on_details_menu_usb_redirection" swapped="no"/>
> + </object>
> + </child>
> </object>
> </child>
> </object>
> diff --git a/virtManager/console.py b/virtManager/console.py
> index 001318e..11709fa 100644
> --- a/virtManager/console.py
> +++ b/virtManager/console.py
> @@ -280,6 +280,9 @@ class Viewer(vmmGObject):
> def get_desktop_resolution(self):
> raise NotImplementedError()
>
> + def has_usb_redirection(self):
> + return False
> +
>
> class VNCViewer(Viewer):
> def __init__(self, console):
> @@ -446,6 +449,8 @@ class SpiceViewer(Viewer):
> self.display = None
> self.audio = None
> self.display_channel = None
> + self.usbdev_manager = None
> + self.usbwidget = None
>
> def _init_widget(self):
> self.set_grab_keys()
> @@ -497,6 +502,8 @@ class SpiceViewer(Viewer):
> self.display.destroy()
> self.display = None
> self.display_channel = None
> + self.usbdev_manager = None
> + self.usbwidget = None
>
> def is_open(self):
> return self.spice_session is not None
> @@ -552,6 +559,29 @@ class SpiceViewer(Viewer):
> return None
> return self.display_channel.get_properties("width", "height")
>
> + def _get_usbdev_manager(self):
> + if self.usbdev_manager:
> + return self.usbdev_manager
> +
> + if self.spice_session:
> + self.usbdev_manager = SpiceClientGLib.UsbDeviceManager.get(self.spice_session)
> + if self.usbdev_manager:
> + self.usbdev_manager.connect("auto-connect-failed", self._usbdev_redirect_error)
> + self.usbdev_manager.connect("device-error", self._usbdev_redirect_error)
> +
> + return self.usbdev_manager
> +
> + def _get_spice_session(self):
> + self.spice_session = SpiceClientGLib.Session()
> + gtk_session = SpiceClientGtk.GtkSession.get(self.spice_session)
> + gtk_session.set_property("auto-clipboard", True)
> +
> + self._get_usbdev_manager()
> +
> + autoredir = self.config.get_auto_redirection()
> + if autoredir:
> + gtk_session.set_property("auto-usbredir", True)
> +
I would change the name of this function. A get function, IMHO,
implicitly means that something is being returned, which is not the case
here.
> def open_host(self, ginfo, password=None):
> host, port = ginfo.get_conn_host()
>
> @@ -559,7 +589,7 @@ class SpiceViewer(Viewer):
> uri += str(host) + "?port=" + str(port)
> logging.debug("spice uri: %s", uri)
>
> - self.spice_session = SpiceClientGLib.Session()
> + self._get_spice_session()
> self.spice_session.set_property("uri", uri)
> if password:
> self.spice_session.set_property("password", password)
> @@ -568,7 +598,7 @@ class SpiceViewer(Viewer):
> self.spice_session.connect()
>
> def open_fd(self, fd, password=None):
> - self.spice_session = SpiceClientGLib.Session()
> + self._get_spice_session()
> if password:
> self.spice_session.set_property("password", password)
> GObject.GObject.connect(self.spice_session, "channel-new",
> @@ -594,6 +624,46 @@ class SpiceViewer(Viewer):
> return
> self.display.set_property("scaling", scaling)
>
> + def _usbdev_redirect_error(self,
> + spice_usbdev_widget, spice_usb_device,
> + errstr):
> + ignore_widget = spice_usbdev_widget
> + ignore_device = spice_usb_device
> +
> + error = self.console.err
> + error.show_err(_("USB redirection error"),
> + text2=str(errstr),
> + async=False)
> +
> + def get_usb_widget(self):
> +
> + # The @format positional parameters are the following:
> + # 1 '%s' manufacturer
> + # 2 '%s' product
> + # 3 '%s' descriptor (a [vendor_id:product_id] string)
> + # 4 '%d' bus
> + # 5 '%d' address
> +
> + usb_device_description_fmt = _("%s %s %s at %d-%d")
> +
> + if self.spice_session:
> + self.usbwidget = SpiceClientGtk.UsbDeviceWidget.new(self.spice_session,
> + usb_device_description_fmt)
> + self.usbwidget.connect("connect-failed", self._usbdev_redirect_error)
> + return self.usbwidget
> +
> + return
> +
> + def has_usb_redirection(self):
> + usbredir_channel_type = SpiceClientGLib.Channel.string_to_type('usbredir')
> +
> + if self.spice_session:
> + if self._get_usbdev_manager() and \
> + self.spice_session.has_channel_type(usbredir_channel_type):
> + return True
> +
> + return False
> +
>
> class vmmConsolePages(vmmGObjectUI):
> def __init__(self, vm, builder, topwin):
> @@ -955,11 +1025,13 @@ class vmmConsolePages(vmmGObjectUI):
> self.close_viewer()
> self.widget("console-pages").set_current_page(PAGE_UNAVAILABLE)
> self.widget("details-menu-vm-screenshot").set_sensitive(False)
> + self.widget("details-menu-usb-redirection").set_sensitive(False)
> self.widget("console-unavailable").set_label("<b>" + msg + "</b>")
>
> def activate_auth_page(self, withPassword=True, withUsername=False):
> (pw, username) = self.config.get_console_password(self.vm)
> self.widget("details-menu-vm-screenshot").set_sensitive(False)
> + self.widget("details-menu-usb-redirection").set_sensitive(False)
>
> if withPassword:
> self.widget("console-auth-password").show()
> @@ -999,6 +1071,15 @@ class vmmConsolePages(vmmGObjectUI):
> if self.viewer and self.viewer.display:
> self.viewer.display.grab_focus()
>
> + if not self.viewer.has_usb_redirection():
> + return
> +
> + devs = self.vm.get_redirdev_devices()
> + for dev in devs:
> + if dev.type == "spicevmc":
> + self.widget("details-menu-usb-redirection").set_sensitive(True)
> + return
> +
I'd rather join the two blocks above in an if then else statement and
remove the returns than keep the return after the if clause. Today, we
know that these two blocks are linked, but if in the future someone else
wants to activate any other element, the return in the middle of the
function can be confusing. But this would not block me from accepting
this patch.
However, after testing this peace of code, this is not working for me.
The USB redirection menu option is always inactive, even though my VM
has USB redirection devices. I tested the VM with virt-viewer and the
menu was active there. So we might have something wrong going on here.
Also, and I don't know if this is related to this change,
auto-redirection still didn't work for me.
> def page_changed(self, ignore1=None, ignore2=None, ignore3=None):
> self.set_allow_fullscreen()
>
> diff --git a/virtManager/details.py b/virtManager/details.py
> index b2d496e..f2d0318 100644
> --- a/virtManager/details.py
> +++ b/virtManager/details.py
> @@ -415,6 +415,7 @@ class vmmDetails(vmmGObjectUI):
> "on_details_menu_migrate_activate": self.control_vm_migrate,
> "on_details_menu_delete_activate": self.control_vm_delete,
> "on_details_menu_screenshot_activate": self.control_vm_screenshot,
> + "on_details_menu_usb_redirection": self.control_vm_usb_redirection,
> "on_details_menu_view_toolbar_activate": self.toggle_toolbar,
> "on_details_menu_view_manager_activate": self.view_manager,
> "on_details_menu_view_details_toggled": self.details_console_changed,
> @@ -621,7 +622,9 @@ class vmmDetails(vmmGObjectUI):
> ##########################
>
> def init_menus(self):
> - # Shutdown button menu
> + # Virtual Machine menu
> + self.widget("details-menu-usb-redirection").set_tooltip_text(
> + _("Redirect USB device attached on host to virtual machine with SPICE graphics. USB Redirection device is required for Virtual Machine to support this functionality. Auto-redirection is enabled by default"))
You should set the tooltip in the UI XML file.
Best regards,
Leonardo Garcia
> uihelpers.build_shutdown_button_menu(self.widget("control-shutdown"),
> self.control_vm_shutdown,
> self.control_vm_reboot,
> @@ -1593,6 +1596,19 @@ class vmmDetails(vmmGObjectUI):
> except Exception, e:
> self.err.show_err(_("Error taking screenshot: %s") % str(e))
>
> + def control_vm_usb_redirection(self, src):
> + ignore = src
> + spice_usbdev_dialog = self.err
> +
> + spice_usbdev_widget = self.console.viewer.get_usb_widget()
> + if not spice_usbdev_widget:
> + self.err.show_err(_("Error initializing spice USB device widget"))
> + return
> +
> + spice_usbdev_widget.show()
> + spice_usbdev_dialog.show_info(_("Select USB devices for redirection"),
> + widget=spice_usbdev_widget)
> +
> def _take_screenshot(self):
> image = self.console.viewer.get_pixbuf()
>
More information about the virt-tools-list
mailing list