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

[Cole Robinson]

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