[virt-tools-list] [PATCH 2/2] details: Add USB redirection support in console viewer

Leonardo Augusto Guimarães Garcia lagarcia at linux.vnet.ibm.com
Mon Jun 24 16:14:37 UTC 2013


Comments below.

On 06/24/2013 07:11 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 use choose available USB devices for redirection.
> Throw an error message if USB connection failed.
> ---
>   ui/vmm-details.ui      | 15 +++++++++++++++
>   virtManager/console.py | 23 +++++++++++++++++++++++
>   virtManager/details.py | 26 ++++++++++++++++++++++++++
>   3 files changed, 64 insertions(+)
>
> diff --git a/ui/vmm-details.ui b/ui/vmm-details.ui
> index ea4b53e..8da5b32 100644
> --- a/ui/vmm-details.ui
> +++ b/ui/vmm-details.ui
> @@ -337,6 +337,21 @@
>                           <signal name="activate" handler="on_details_menu_screenshot_activate" swapped="no"/>
>                         </object>
>                       </child>
> +                    <child>
> +                      <object class="GtkSeparatorMenuItem" id="separator13">
> +                        <property name="visible">True</property>
> +                        <property name="can_focus">False</property>
> +                      </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">_Select USB devices to redirect</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..a2fc4f8 100644
> --- a/virtManager/console.py
> +++ b/virtManager/console.py
> @@ -286,6 +286,7 @@ class VNCViewer(Viewer):
>           Viewer.__init__(self, console)
>           self.display = GtkVnc.Display.new()
>           self.sockfd = None
> +        self.type = "vnc"
>
>           # Last noticed desktop resolution
>           self.desktop_resolution = None
> @@ -446,6 +447,7 @@ class SpiceViewer(Viewer):
>           self.display = None
>           self.audio = None
>           self.display_channel = None
> +        self.type = "spice"
As self.type will be defined on both VNCViewer and SpiceViewer, I would 
add it to Viewer class as well:

         self.type = None
>
>       def _init_widget(self):
>           self.set_grab_keys()
> @@ -594,6 +596,22 @@ class SpiceViewer(Viewer):
>               return
>           self.display.set_property("scaling", scaling)
>
> +    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")
This is the default string. We can pass None instead of redefining it here.
> +
> +        if self.spice_session:
> +            return SpiceClientGtk.UsbDeviceWidget.new(self.spice_session,
> +                                                      usb_device_description_fmt)
> +        return
> +
>
>   class vmmConsolePages(vmmGObjectUI):
>       def __init__(self, vm, builder, topwin):
> @@ -955,11 +973,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 +1019,9 @@ class vmmConsolePages(vmmGObjectUI):
>           if self.viewer and self.viewer.display:
>               self.viewer.display.grab_focus()
>
> +            if self.viewer.type == "spice":
> +                self.widget("details-menu-usb-redirection").set_sensitive(True)
> +
In fact, I think it would be cleaner if we have a dummy 
has_usb_redirection() method in Viewer that always return False and each 
subclass can define whether it will override the method or not.
>       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..9a68819 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,
> @@ -1593,6 +1594,31 @@ class vmmDetails(vmmGObjectUI):
>           except Exception, e:
>               self.err.show_err(_("Error taking screenshot: %s") % str(e))
>
> +    def spice_usbdev_rediret_error(self,
spice_usbdev_redirect_error
> +                                   spice_usbdev_widget, spice_usb_device,
> +                                   errstr):
> +        ignore_widget = spice_usbdev_widget
> +        ignore_device = spice_usb_device
> +
> +        self.err.show_err(_("USB redirection error"),
> +                         text2=str(errstr),
> +                         async=False)
> +
> +    def control_vm_usb_redirection(self, src):
> +        ignore = src
> +        spice_usbdev_dialog = self.err
I don't think we need this variable here as you are referencing self.err 
directly below. You should either use self.err or spice_usbdev_dialog in 
bellow references.
> +
> +        spice_usbdev_widget = self.console.viewer.get_usb_widget()
> +        if not spice_usbdev_widget:
> +            self.err.show_err(_("Error initialize spice USB device widget"))
Error initializing spice USB device widget
> +            return
> +
> +        spice_usbdev_widget.connect("connect-failed",
> +                                    self.spice_usbdev_rediret_error)
> +        spice_usbdev_widget.show()
I think this is not necessary. When you call show on the info dialog, 
all the widgets within it will be shown.

Best regards,

Leonardo Garcia
> +        spice_usbdev_dialog.show_info_with_widget(_("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