[libvirt] [PATCH 03/11] nodedev: Add ability to find a vport capable vHBA

[John Ferlan]

On 01/02/2017 09:18 AM, Ján Tomko wrote:
> On Fri, Nov 18, 2016 at 09:26:29AM -0500, John Ferlan wrote:
>> If a <parent> is not supplied in the XML used to create a non-persistent
>> vHBA, then instead of failing, let's try to find a "vports" capable node
>> device and use that.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>> src/conf/node_device_conf.c          | 39
>> ++++++++++++++++++++++++++++++++++++
>> src/conf/node_device_conf.h          |  3 +++
>> src/libvirt_private.syms             |  1 +
>> src/node_device/node_device_driver.c | 15 +++++++++-----
>> 4 files changed, 53 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>> index 5396681..3aa77cf 100644
>> +
>> +int
>> +virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
>> +                                 int *parent_host)
>> +{
>> +    virNodeDeviceObjPtr parent = NULL;
>> +    int ret;
>> +
>> +    if (!(parent = virNodeDeviceFindByCap(devs, "vports"))) {
> 
> Why do we pass the capability as a string?
> 

OK - I'll alter this to:

  const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);

and use "cap" instead of "vports".

NB: 
I cannot search on the virNodeDevCapType because it's actually a
"subtype" of the 'scsi_host' type (as seen in virNodeDeviceHasCap)

The virNodeDeviceHasCap should also alter those hardcoded STREQ values
to use the virNodeDevCapTypeToString for VIR_NODE_DEV_CAP_FC_HOST and
VIR_NODE_DEV_CAP_VPORTS...  

I'll squash the following in (sorry - inline and will mess with mail
window widths, but I tested this it works):

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 42f6593..b13cb6b 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -75,14 +75,19 @@ virNodeDevCapsDefParseString(const char *xpath,
 int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const char *cap)
 {
     virNodeDevCapsDefPtr caps = dev->def->caps;
+    const char *fc_host_cap =
+        virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST);
+    const char *vports_cap =
+        virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
+
     while (caps) {
         if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type)))
             return 1;
         else if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST)
-            if ((STREQ(cap, "fc_host") &&
+            if ((STREQ(cap, fc_host_cap) &&
                 (caps->data.scsi_host.flags &
                  VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) ||
-                (STREQ(cap, "vports") &&
+                (STREQ(cap, vports_cap) &&
                 (caps->data.scsi_host.flags &
                  VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)))
                 return 1;
@@ -1851,9 +1856,10 @@ virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
                                  int *parent_host)
 {
     virNodeDeviceObjPtr parent = NULL;
+    const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
     int ret;
 
-    if (!(parent = virNodeDeviceFindByCap(devs, "vports"))) {
+    if (!(parent = virNodeDeviceFindByCap(devs, cap))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Could not find any vport capable device"));
         return -1;

>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("Could not find any vport capable device"));
>> +        return -1;
>> +    }
>> +
> 
>> diff --git a/src/node_device/node_device_driver.c
>> b/src/node_device/node_device_driver.c
>> index 91bb142..0e091fe 100644
>> --- a/src/node_device/node_device_driver.c
>> +++ b/src/node_device/node_device_driver.c
>> @@ -584,11 +584,16 @@ nodeDeviceCreateXML(virConnectPtr conn,
>>     if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1)
>>         goto cleanup;
>>
>> -    if (virNodeDeviceGetParentHost(&driver->devs,
>> -                                   def->name,
>> -                                   def->parent,
>> -                                   &parent_host) == -1) {
>> -        goto cleanup;
>> +    if (def->parent) {
>> +        if (virNodeDeviceGetParentHost(&driver->devs,
>> +                                       def->name,
>> +                                       def->parent,
>> +                                       &parent_host) < 0)
>> +            goto cleanup;
>> +    } else {
>> +        /* Try to find "a" vport capable scsi_host when no parent
>> supplied */
> 
> The quotes make me nervous.
> 

Removed...  I think the point of them was it'll be capable, but
there's no check if there's enough ports remaining for use. That'll
fail differently though.


John
>> +        if (virNodeDeviceFindVportParentHost(&driver->devs,
>> &parent_host) < 0)
>> +            goto cleanup;
>>     }
> 
> ACK
> 
> Jan