[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

On 06/25/2013 12:14 AM, Leonardo Augusto GuimarĂ£es Garcia wrote:
Comments below.

      def _init_widget(self):
@@ -594,6 +596,22 @@ class SpiceViewer(Viewer):
          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.

Yeah, I tried it before, the python binding API doesn't accept None for this argument.
       widget = SpiceClientGtk.UsbDeviceWidget.new(self.session, None)
       Error like: TypeError: Argument 1 does not allow None as a value

+ 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.

the variable just help to make code more readable no other meanings.

+        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.

Actually, we need this, dialog.show_all() will take this effect, but in error.py we use dialog.show() or nothing if sync is true.

        Thanks for this review.


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