Re: [PATCH] check "fc_host" and "vport_ops" capabilities in SCSI host nodedevs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.

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


>From 37c59646de925e1f8eb76d1022d33032cd671bf5 Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@xxxxxxxxxx>
Date: Wed, 4 Feb 2015 08:10:52 -0500
Subject: [PATCH] nodedev: check/add for scsi_host caps for NumOfCaps and
 ListCaps

Commit id '652a2ec6' introduced two new node device capability flags
and the ability to use those flags as a way to search for a specific
subset of a 'scsi_host' device - namely a 'fc_host' and/or 'vports'.
The code modified the virNodeDeviceCapMatch whichs allows for searching
using the 'virsh nodedev-list [cap]' via virConnectListAllNodeDevices.

However, the original patches did not account for other searches for
the same capability key from virNodeDeviceNumOfCaps and virNodeDeviceListCaps
using nodeDeviceNumOfCaps and nodeDeviceListCaps. Since 'fc_host' and
'vports' are self defined bits of a 'scsi_host' device mere string
comparison against the basic/root type is not sufficient.

This patch adds the check for the 'fc_host' and 'vports' bits within
a 'scsi_host' device and allows the following python code to find the
capabilities for the device:

import libvirt
conn = libvirt.openReadOnly('qemu:///system')
devs = conn.listAllDevices()
for dev in devs:
    if 'fc_host' in dev.listCaps() or 'vports' in dev.listCaps():
        print dev.name(),dev.numOfCaps(),dev.listCaps()

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/node_device/node_device_driver.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index c1ad3cd..b8d9f4f 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -383,8 +383,20 @@ nodeDeviceNumOfCaps(virNodeDevicePtr dev)
     if (virNodeDeviceNumOfCapsEnsureACL(dev->conn, obj->def) < 0)
         goto cleanup;
 
-    for (caps = obj->def->caps; caps; caps = caps->next)
+    for (caps = obj->def->caps; caps; caps = caps->next) {
         ++ncaps;
+
+        if (caps->type == VIR_NODE_DEV_CAP_SCSI_HOST) {
+            if (caps->data.scsi_host.flags &
+                VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)
+                ncaps++;
+
+            if (caps->data.scsi_host.flags &
+                VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)
+                ncaps++;
+        }
+    }
+
     ret = ncaps;
 
  cleanup:
@@ -419,6 +431,24 @@ nodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames)
     for (caps = obj->def->caps; caps && ncaps < maxnames; caps = caps->next) {
         if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->type)) < 0)
             goto cleanup;
+
+        if (caps->type == VIR_NODE_DEV_CAP_SCSI_HOST) {
+            if (ncaps < maxnames &&
+                caps->data.scsi_host.flags &
+                VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) {
+                if (VIR_STRDUP(names[ncaps++],
+                               virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST)) < 0)
+                    goto cleanup;
+            }
+
+            if (ncaps < maxnames &&
+                caps->data.scsi_host.flags &
+                VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) {
+                if (VIR_STRDUP(names[ncaps++],
+                               virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS)) < 0)
+                    goto cleanup;
+            }
+        }
     }
     ret = ncaps;
 
-- 
2.1.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]