[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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()



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]