On Wed, Feb 4, 2015 at 8:39 PM, Shivaprasad bhat
<shivaprasadbhat@xxxxxxxxx> wrote:
> On Wed, Feb 4, 2015 at 7:25 PM, John Ferlan <jferlan@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
>>>> ---
>>>> 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.
>
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
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.
>> Thanks -
>>
>> John
>>
>>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list