On 02/03/2015 06:55 AM, Shivaprasad G Bhat wrote: > fc_host & vport_ops devices are SCSI devices with additional capabilities. > Mere string comparison of basic types is not sufficient in this case. This > patch introduces additional capability checks for SCSI devices if the user > is looking to list 'fc_host' or 'vport_ops' devices. > > Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; > --- > src/conf/node_device_conf.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > While this does work and more or less follows what was done for virNodeDeviceCapMatch, I'm wondering if the 'fc_host' and 'vports_ops' capabilities need to also be returned in a "list" of capabilities for a node device That is I see that virNodeDeviceListCaps() seems to be only returning 1 capability for every device. However, for the scsi_host, it has those additional fc_host and vport_ops capabilities which if returned in the list would then be "found" by the python listDevices code which for some devices (like the scsi_host here) there may be more than one way to "get at" the information. I'm investigating whether modifying nodeDeviceListCaps() in src/node_device/node_device_driver.c to add "fc_host" and "vport_ops" to the return caps_list will resolve the issue... This also means virNodeDeviceNumOfCaps() (and it's driver API nodeDeviceNumOfCaps) will need some tweaking too. Another option would be to fix the libvirt-python code to use ListAllDevices with the flags argument like virsh does. John

On Wed, Feb 4, 2015 at 7:25 PM, John Ferlan <jferlan(a)redhat.com&gt; wrote: > > > On 02/03/2015 03:59 PM, John Ferlan wrote: >> >> >> On 02/03/2015 06:55 AM, Shivaprasad G Bhat wrote: >>> fc_host & vport_ops devices are SCSI devices with additional capabilities. >>> Mere string comparison of basic types is not sufficient in this case. This >>> patch introduces additional capability checks for SCSI devices if the user >>> is looking to list 'fc_host' or 'vport_ops' devices. >>> >>> Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; >>> --- >>> src/conf/node_device_conf.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >> >> While this does work and more or less follows what was done for >> virNodeDeviceCapMatch, I'm wondering if the 'fc_host' and 'vports_ops' >> capabilities need to also be returned in a "list" of capabilities for a >> node device >> >> That is I see that virNodeDeviceListCaps() seems to be only returning 1 >> capability for every device. However, for the scsi_host, it has those >> additional fc_host and vport_ops capabilities which if returned in the >> list would then be "found" by the python listDevices code which for some >> devices (like the scsi_host here) there may be more than one way to "get >> at" the information. >> >> I'm investigating whether modifying nodeDeviceListCaps() in >> src/node_device/node_device_driver.c to add "fc_host" and "vport_ops" to >> the return caps_list will resolve the issue... This also means >> virNodeDeviceNumOfCaps() (and it's driver API nodeDeviceNumOfCaps) will >> need some tweaking too. >> >> Another option would be to fix the libvirt-python code to use >> ListAllDevices with the flags argument like virsh does. >> >> John > <...snip...> > > As it turns out your patch will be required in order to cover the > virNodeListDevices case as well as the attached patch in order to cover > the virNodeDeviceNumOfCaps and virNodeDeviceListCaps cases. > You are right, The listDevices doesn't use either of these APIs. > Could you take a look at the attached code and try it out on your system > to make sure it does what I expect (the system I use to test these types > of checks is unavailable at the moment). What should happen is for > 'scsi_host' devices with 'fc_host' and/or 'vports' capability types - > the cap_list returned will be larger than 1. The code currently only has > ever returned 1 element. The patch also lists the sample python which > should work. > > With your successful test, I'll push the patches (although I'm tweaking > your commit message a bit - it'll be very similar to the attached patch > commit message). > Thanks John. I agree with your changes for virNodeDeviceListCaps and virNodeDeviceNumOfCaps. I too don't have systems for verifying the patch for now. Let me see if I can catch hold of a system tomorrow.

> Thanks - > > John > >

<...snip...> > > Had a chance to try your patch. > > # cat testCaps.py > import libvirt > conn = libvirt.openReadOnly('qemu:///system') > fc = conn.nodeDeviceLookupByName('scsi_host1') > caps = fc.listCaps(); > print "The capabilities are", caps > nCaps = fc.numOfCaps(); > print "The number of Caps : ", nCaps > > # ./run python testCaps.py > The capabilities are ['scsi_host', 'fc_host', 'vports'] > The number of Caps : 3 > Excellent - thanks... I "assume" prior to my patch the result would just be 'scsi_host' and 1 for number of caps...

> Given the above output(also virsh behaviour), I am thinking my patch should > actually be checking the string "vports" instead of "vport_ops". > > I am sending you the v2 with that change. Feel free to change the commit > message as appropriate. > I can edit your existing patch in my git branch to change "vport_ops" to "vports" - that's not a problem...

John