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

+        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

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

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

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

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

- Cole
Right, this is new version of my patch... It has also "Source Device" value aligned to the left (there was a bad alignment to right) and also helper functions attrVal() which checks whether we have attr and if so it returns it contents, otherwise it returns None and also intify() function with arguments of val and hex... It converts val from decimal string to int (if hex is False) or hex string to int (if hex is True). The function is used for both the cases in this refresh_hostdev_info() method so please review...

In the glade file, the bad deletion of delete_event has been neutralized by adding it back manually not to cause problems...

Thanks,
Michal
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	Wed Nov 04 11:34:35 2009 +0100
@@ -634,6 +634,12 @@
 
     def refresh_resources(self, ignore):
         details = self.window.get_widget("details-pages")
+
+        # If details are NoneType return not to cause errors like
+        # you can see in the log file
+        if not details:
+            return
+
         page = details.get_current_page()
 
         # If the dialog is visible, we want to make sure the XML is always
@@ -908,17 +914,78 @@
         self.window.get_widget("char-target-port").set_text(charinfo[3] or "")
         self.window.get_widget("char-source-path").set_text(charinfo[5] or "-")
 
+    # Function to intify val. If hex is set to True, the val is considered
+    # to be a hex string so it tries to convert from hex or returns 0
+    def intify(self, val, hex=False):
+        try:
+            if hex:
+                return int(val or '0x00', 16)
+            else:
+                return int(val)
+        except:
+            return -1
+
+    # Function to get attribute value (if exists) from node, otherwise
+    # returns None
+    def attrVal(self, node, attr):
+        if not hasattr(node, attr):
+            return None
+        return getattr(node, attr)
+
     def refresh_hostdev_page(self):
         hostdevinfo = self.get_hw_selection(HW_LIST_COL_DEVICE)
         if not hostdevinfo:
             return
 
+        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
+            device = 0
+            bus = 0
+            domain = 0
+            func = 0
+            slot = 0
+        elif devinfo.get("address"):
+            vendor_id = -1
+            product_id = -1
+            device = self.intify(devinfo["address"].get("device"), True)
+            bus = self.intify(devinfo["address"].get("bus"), True)
+            domain = self.intify(devinfo["address"].get("domain"), True)
+            func = self.intify(devinfo["address"].get("function"), True)
+            slot = self.intify(devinfo["address"].get("slot"), True)
+
+        typ = devinfo.get("type")
+        # For USB we want a device, not a bus
+        if typ == 'usb':
+            typ = 'usb_device'
+        dev_pretty_name = None
+        devs = self.vm.get_connection().get_devices( typ, None )
+
+        # Get device pretty name
+        for dev in devs:
+            # Try to get info from {product|vendor}_id
+            if (self.attrVal(dev, "product_id") == product_id
+              and self.attrVal(dev, "vendor_id") == vendor_id):
+                dev_pretty_name = dev.pretty_name()
+                break
+            else:
+                # Try to get info from bus/addr
+                dev_id = self.intify(self.attrVal(dev, "device"))
+                bus_id = self.intify(self.attrVal(dev, "bus"))
+                dom_id = self.intify(self.attrVal(dev, "domain"))
+                func_id = self.intify(self.attrVal(dev, "function"))
+                slot_id = self.intify(self.attrVal(dev, "slot"))
+
+                if ((dev_id == device and bus_id == bus)
+                  or (dom_id == domain and func_id == func and slot_id == slot)):
+                    dev_pretty_name = dev.pretty_name()
+                    break
+
         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	Wed Nov 04 11:34:35 2009 +0100
@@ -3430,6 +3430,17 @@
                                         <property name="column_spacing">8</property>
                                         <property name="row_spacing">4</property>
                                         <child>
+                                          <widget class="GtkLabel" id="label8">
+                                            <property name="visible">True</property>
+                                            <property name="xalign">0</property>
+                                            <property name="label" translatable="yes">Source Device:</property>
+                                          </widget>
+                                          <packing>
+                                            <property name="x_options">GTK_FILL</property>
+                                            <property name="y_options"></property>
+                                          </packing>
+                                        </child>
+                                        <child>
                                           <widget class="GtkLabel" id="hostdev-source">
                                             <property name="visible">True</property>
                                             <property name="xalign">0</property>
@@ -3438,73 +3449,24 @@
                                           <packing>
                                             <property name="left_attach">1</property>
                                             <property name="right_attach">2</property>
-                                            <property name="top_attach">2</property>
-                                            <property name="bottom_attach">3</property>
-                                            <property name="y_options"></property>
-                                          </packing>
-                                        </child>
-                                        <child>
-                                          <widget class="GtkLabel" id="hostdev-mode">
-                                            <property name="visible">True</property>
-                                            <property name="xalign">0</property>
-                                            <property name="label">label</property>
-                                          </widget>
-                                          <packing>
-                                            <property name="left_attach">1</property>
-                                            <property name="right_attach">2</property>
-                                            <property name="top_attach">1</property>
-                                            <property name="bottom_attach">2</property>
-                                            <property name="y_options"></property>
-                                          </packing>
-                                        </child>
-                                        <child>
-                                          <widget class="GtkLabel" id="hostdev-type">
-                                            <property name="visible">True</property>
-                                            <property name="xalign">0</property>
-                                            <property name="label">label</property>
-                                          </widget>
-                                          <packing>
-                                            <property name="left_attach">1</property>
-                                            <property name="right_attach">2</property>
-                                            <property name="y_options"></property>
-                                          </packing>
-                                        </child>
-                                        <child>
-                                          <widget class="GtkLabel" id="label9">
-                                            <property name="visible">True</property>
-                                            <property name="xalign">1</property>
-                                            <property name="label" translatable="yes">Device Mode:</property>
-                                          </widget>
-                                          <packing>
-                                            <property name="top_attach">1</property>
-                                            <property name="bottom_attach">2</property>
                                             <property name="x_options">GTK_FILL</property>
                                             <property name="y_options"></property>
                                           </packing>
                                         </child>
                                         <child>
-                                          <widget class="GtkLabel" id="label8">
-                                            <property name="visible">True</property>
-                                            <property name="xalign">1</property>
-                                            <property name="label" translatable="yes">Device Type:</property>
-                                          </widget>
-                                          <packing>
-                                            <property name="x_options">GTK_FILL</property>
-                                            <property name="y_options"></property>
-                                          </packing>
+                                          <placeholder/>
                                         </child>
                                         <child>
-                                          <widget class="GtkLabel" id="label3">
-                                            <property name="visible">True</property>
-                                            <property name="xalign">1</property>
-                                            <property name="label" translatable="yes">Source Device:</property>
-                                          </widget>
-                                          <packing>
-                                            <property name="top_attach">2</property>
-                                            <property name="bottom_attach">3</property>
-                                            <property name="x_options">GTK_FILL</property>
-                                            <property name="y_options">GTK_FILL</property>
-                                          </packing>
+                                          <placeholder/>
+                                        </child>
+                                        <child>
+                                          <placeholder/>
+                                        </child>
+                                        <child>
+                                          <placeholder/>
+                                        </child>
+                                        <child>
+                                          <placeholder/>
                                         </child>
                                       </widget>
                                     </child>

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