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

Re: [virt-tools-list] [PATCH] Virt-manager host device information bits



On 11/03/2009 06:53 PM, Cole Robinson wrote:
On 11/03/2009 12:24 PM, Michal Novotny wrote:
Hi,
this is new patch for host device information to just find&  show pretty
name of device for USB and PCI devices, tested with test driver so
please review...

Thanks,
Michal

Comments inline

diff -r a5b9807ead04 src/virtManager/details.py
--- a/src/virtManager/details.py	Sun Nov 01 21:56:21 2009 -0500
+++ b/src/virtManager/details.py	Tue Nov 03 18:19:29 2009 +0100
@@ -913,12 +913,76 @@
          if not hostdevinfo:
              return

+        try:
+            typ = hostdevinfo[1]['type']
+            if typ == 'usb':
+                typ = 'usb_device'
+            vendor_id = hostdevinfo[1]['vendor']['id']
+            product_id = hostdevinfo[1]['product']['id']
+            device = 0
+            bus = 0
+            domain = 0
+            func = 0
+            slot = 0
+        except Exception, e:
+            try:
+                device = int(hostdevinfo[1]['address']['device'] or '0x00', 16)
+                bus = int(hostdevinfo[1]['address']['bus'] or '0x00', 16)
+                domain = int(hostdevinfo[1]['address']['domain'] or '0x00', 16)
+                func = int(hostdevinfo[1]['address']['function'] or '0x00', 16)
+                slot = int(hostdevinfo[1]['address']['slot'] or '0x00', 16)
+                vendor_id = -1
+                product_id = -1
+            except Exception, e:
+                device = 0
+                bus = 0
+                domain = 0
+                func = 0
+                slot = 0
+                vendor_id = -1
+                product_id = -1
+
Rather than do all this inside try...except blocks, you can just use
dict.get():

devinfo = hostdevinfo[1]
if devinfo.get("vendor") and devinfo.get("product"):
     vendor_id = devinfo["vendor"].get("id") or -1
     product_id = devinfo["product"].get("id") or -1
elif devinfo.get("address"):
     ...

etc.

Well, thanks for the tip. I'll rewrite it this way tomorrow...

+        dev_pretty_name = None
+        devs = self.vm.get_connection().get_devices( typ, None )
+
+        # Try to get info from {product|vendor}_id
+        for dev in devs:
+            if dev.product_id == product_id and dev.vendor_id == vendor_id:
+                dev_pretty_name = dev.pretty_name()
+
Probably want to 'break' after a successful lookup


It may speed up the code a little, that's right...

+        if not dev_pretty_name:
+            # Try to get info from bus/addr
+            for dev in devs:
+                try:
+                    dev_id = int(dev.device)
Rather than compare by 'int', just keep the
hostdevinfo[1]["address"]["device"] in string form and compare like that.


Well, I was running into some issues ... when having the required variables, it was clearly int() type... ie. `device` is a required variable... but when accessing dev.device, I am getting eg. 0x0000 which is hexadecimal so the comparison is not available because one is int() and second is hex()...

+                except Exception, e:
+                    dev_id = -1
+                try:
+                    bus_id = int(dev.bus)
+                except Exception, e:
+                    bus_id = -1
+                try:
+                    domain_id = int(dev.domain)
+                except Exception, e:
+                    domain_id = -1
+                try:
+                    func_id = int(dev.function)
+                except Exception, e:
+                    func_id = -1
+                try:
+                    slot_id = int(dev.slot)
+                except Exception, e:
+                    slot_id = -1
+
+                if ((dev_id == device and bus_id == bus)
+                  or (domain_id == domain and func_id == func and slot_id == slot)):
+                    dev_pretty_name = dev.pretty_name()
+
This should all be in the same 'for dev in devs' loop. If the
product/vendor lookup fails, just do

else if dev_id == device and bus_id == bus ...


Ok, I can rewrite this as well...

+
          devlabel = "<b>Physical %s Device</b>" % hostdevinfo[4].upper()

          self.window.get_widget("hostdev-title").set_markup(devlabel)
-        self.window.get_widget("hostdev-type").set_text(hostdevinfo[4])
-        self.window.get_widget("hostdev-mode").set_text(hostdevinfo[3])
-        self.window.get_widget("hostdev-source").set_text(hostdevinfo[5])
+        self.window.get_widget("hostdev-source").set_text(dev_pretty_name or "-")

      def refresh_video_page(self):
          vidinfo = self.get_hw_selection(HW_LIST_COL_DEVICE)
diff -r a5b9807ead04 src/vmm-details.glade
--- a/src/vmm-details.glade	Sun Nov 01 21:56:21 2009 -0500
+++ b/src/vmm-details.glade	Tue Nov 03 18:19:29 2009 +0100
@@ -6,7 +6,6 @@
      <property name="title" translatable="yes">Virtual Machine</property>
      <property name="default_width">800</property>
      <property name="default_height">600</property>
-<signal name="delete_event" handler="on_vmm_details_delete_event"/>
      <child>
        <widget class="GtkVBox" id="vbox2">
          <property name="visible">True</property>
Urgh, broken glade-3 strikes again. Can't remember what version it is,
but glade likes to drop the delete signal handler for windows, which
means hitting the 'X' in the top corner destroys the window rather than
just hiding it. Next time you go to launch the same window, virt-manager
will error.


This is the first time I've been trying to use Glade-3 designer because I've always tried to edit it manually in the XML/Glade file...

I'll make sure not to commit this piece but it is something to watch out
for if you start hitting errors locally.

Right, well, it's the interface designer issue I guess. Better to edit it manually like I did before...
- Cole
Thanks,
- Michal


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