[virt-tools-list] [PATCH] Virt-manager host device information bits
Michal Novotny
minovotn at redhat.com
Wed Nov 4 10:39:35 UTC 2009
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vmm-show-hostdev-information-bits.patch
Type: text/x-patch
Size: 11417 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20091104/e19d8eab/attachment.bin>
More information about the virt-tools-list
mailing list