[libvirt] [PATCH] check "fc_host" and "vport_ops" capabilitis in SCSI host nodedevs
virNodeListDevices called by listDevices indirectly uses the virNodeDeviceHasCap() which is buggy. Earlier, virNodeListDevices was used in "virsh nodedev-list" as well. Though, the code was rewritten to use vshNodeDeviceListCollect instead. The patch fixes listDevices output for the below python script. ------------- import libvirt conn = libvirt.openReadOnly('qemu:///system') fc = conn.listDevices('fc_host', 0) print(fc) ------------ --- Shivaprasad G Bhat (1): check "fc_host" and "vport_ops" capabilities in SCSI host nodedevs src/conf/node_device_conf.c | 8 ++++++++ 1 file changed, 8 insertions(+) -- Signature
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@linux.vnet.ibm.com> --- src/conf/node_device_conf.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 03b88a2..ab27829 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -78,6 +78,14 @@ int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const char *cap) while (caps) { if (STREQ(cap, virNodeDevCapTypeToString(caps->type))) return 1; + else if (caps->type == VIR_NODE_DEV_CAP_SCSI_HOST) + if ((STREQ(cap, "fc_host") && + (caps->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) || + (STREQ(cap, "vport_ops") && + (caps->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS))) + return 1; caps = caps->next; } return 0;
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@linux.vnet.ibm.com> --- 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
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 03b88a2..ab27829 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -78,6 +78,14 @@ int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const char *cap) while (caps) { if (STREQ(cap, virNodeDevCapTypeToString(caps->type))) return 1; + else if (caps->type == VIR_NODE_DEV_CAP_SCSI_HOST) + if ((STREQ(cap, "fc_host") && + (caps->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) || + (STREQ(cap, "vport_ops") && + (caps->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS))) + return 1; caps = caps->next; } return 0;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@linux.vnet.ibm.com> --- 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. 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
On Wed, Feb 4, 2015 at 7:25 PM, John Ferlan <jferlan@redhat.com> 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@linux.vnet.ibm.com> --- 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
On Wed, Feb 4, 2015 at 8:39 PM, Shivaprasad bhat <shivaprasadbhat@gmail.com> wrote:
On Wed, Feb 4, 2015 at 7:25 PM, John Ferlan <jferlan@redhat.com> 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@linux.vnet.ibm.com> --- 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
<...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
On Thu, Feb 5, 2015 at 6:13 PM, John Ferlan <jferlan@redhat.com> wrote:
<...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...
Yes it is.. :) # python testCaps.py The capabilities are ['scsi_host'] The num of Caps : 1
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...
Thanks for offering to edit my patch. Regards, Shiva
John
On 02/03/2015 06:52 AM, Shivaprasad G Bhat wrote:
virNodeListDevices called by listDevices indirectly uses the virNodeDeviceHasCap() which is buggy.
Earlier, virNodeListDevices was used in "virsh nodedev-list" as well. Though, the code was rewritten to use vshNodeDeviceListCollect instead.
The patch fixes listDevices output for the below python script. ------------- import libvirt conn = libvirt.openReadOnly('qemu:///system') fc = conn.listDevices('fc_host', 0) print(fc) ------------
---
Shivaprasad G Bhat (1): check "fc_host" and "vport_ops" capabilities in SCSI host nodedevs
src/conf/node_device_conf.c | 8 ++++++++ 1 file changed, 8 insertions(+)
These are now pushed... Thanks again! John
participants (3)
-
John Ferlan -
Shivaprasad bhat -
Shivaprasad G Bhat