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@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