Predefine a second NPIV capable HBA as well as a vHBA using the first NPIV capable HBA. This will allow for a mechanism to perform more realistic create vHBA testing. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/test/test_driver.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 18428a8..4dd2956 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -499,10 +499,40 @@ static const char *defaultConnXML = " </capability>" " <capability type='vport_ops'>" " <max_vports>127</max_vports>" +" <vports>1</vports>" +" </capability>" +" </capability>" +"</device>" +"<device>" +" <name>scsi_host2</name>" +" <parent>computer</parent>" +" <capability type='scsi_host'>" +" <host>2</host>" +" <unique_id>1</unique_id>" +" <capability type='fc_host'>" +" <wwnn>2000000056785678</wwnn>" +" <wwpn>1000000056785678</wwpn>" +" <fabric_wwn>2000000087658765</fabric_wwn>" +" </capability>" +" <capability type='vport_ops'>" +" <max_vports>127</max_vports>" " <vports>0</vports>" " </capability>" " </capability>" "</device>" +"<device>" +" <name>scsi_host11</name>" +" <parent>scsi_host1</parent>" +" <capability type='scsi_host'>" +" <host>11</host>" +" <unique_id>10</unique_id>" +" <capability type='fc_host'>" +" <wwnn>2000000034563456</wwnn>" +" <wwpn>1000000034563456</wwpn>" +" <fabric_wwn>2000000043214321</fabric_wwn>" +" </capability>" +" </capability>" + "</device>" "</node>"; -- 2.7.4

Modify the code to react more like a real HBA -> vHBA creation. Currently the code would just modify the input XML definition to set the name to a wwpn and then modify the scsi_host capability entry for the defintion to change the scsi_host# and unique_id before adding that into the node device. This patch does things a bit better. It finds and copies a known existing vHBA (scsi_host11) in the node_device database and modifies that definition to change the name to scsi_host12 and set the wwnn/ wwpn to what the input XML would expect before adding the def to the node device object list. Then rather than create a returned "dev" using the (poorly) mocked name - perform the lookup using the new device name. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/test/test_driver.c | 75 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5e628b5..0a0fa8f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5623,32 +5623,59 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames) } -static int +static virNodeDeviceDefPtr testNodeDeviceMockCreateVport(virConnectPtr conn, - virNodeDeviceDefPtr def, + const char *wwnn, const char *wwpn) { - int ret = -1; testDriverPtr driver = conn->privateData; + virNodeDevicePtr devcpy = NULL; + char *xml = NULL; + virNodeDeviceDefPtr def = NULL; virNodeDevCapsDefPtr caps; virNodeDeviceObjPtr obj = NULL; virObjectEventPtr event = NULL; - /* 'name' is supposed to be filled in by the node device backend, which - * we don't have. Use WWPN instead. */ + /* In the real code, we'd call virVHBAManageVport which would take the + * wwnn/wwpn from the input XML in order to call the "vport_create" + * function for the parent. That in turn would set off a sequence of + * events resulting in the creation of a vHBA scsi_hostN in the + * node device objects list using the "next" host number with the + * wwnn/wwpn from the input XML. The following will mock this by + * using the scsi_host11 definition, changing the name and the + * scsi_host capability fields before calling virNodeDeviceAssignDef + * to add the def to the node device objects list. */ + if (!(devcpy = virNodeDeviceLookupByName(conn, "scsi_host11")) || + !(xml = virNodeDeviceGetXMLDesc(devcpy, 0)) || + !(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL))) + goto cleanup; + VIR_FREE(def->name); - if (VIR_STRDUP(def->name, wwpn) < 0) + if (VIR_STRDUP(def->name, "scsi_host12") < 0) goto cleanup; - /* Fill in a random 'host' and 'unique_id' value, - * since this would also come from the backend */ + /* Find the 'scsi_host' cap and alter the host # and unique_id and + * then for the 'fc_host' capability modify the wwnn/wwpn to be that + * of the input XML. */ caps = def->caps; while (caps) { if (caps->data.type != VIR_NODE_DEV_CAP_SCSI_HOST) continue; - caps->data.scsi_host.host = virRandomBits(10); - caps->data.scsi_host.unique_id = 2; + /* For the "fc_host" cap - change the wwnn/wwpn to match the input */ + if (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + VIR_FREE(caps->data.scsi_host.wwnn); + VIR_FREE(caps->data.scsi_host.wwpn); + if (VIR_STRDUP(caps->data.scsi_host.wwnn, wwnn) < 0 || + VIR_STRDUP(caps->data.scsi_host.wwpn, wwpn) < 0) + goto cleanup; + } else { + /* For the "scsi_host" cap, increment our host and unique_id to + * give the appearance that something new was created - then add + * that to the node device driver */ + caps->data.scsi_host.host++; + caps->data.scsi_host.unique_id++; + } caps = caps->next; } @@ -5661,10 +5688,16 @@ testNodeDeviceMockCreateVport(virConnectPtr conn, 0); testObjectEventQueue(driver, event); - ret = 0; - cleanup: - return ret; + VIR_FREE(xml); + if (!obj) { + virNodeDeviceDefFree(def); + def = NULL; + } + if (devcpy) + virNodeDeviceFree(devcpy); + + return def; } @@ -5678,6 +5711,7 @@ testNodeDeviceCreateXML(virConnectPtr conn, char *wwnn = NULL, *wwpn = NULL; int parent_host = -1; virNodeDevicePtr dev = NULL; + virNodeDeviceDefPtr newdef = NULL; virCheckFlags(0, NULL); @@ -5699,14 +5733,15 @@ testNodeDeviceCreateXML(virConnectPtr conn, /* In the real code, we'd call virVHBAManageVport followed by * find_new_device, but we cannot do that here since we're not - * mocking udev. So we just mock a creation by altering the - * input XML enough to make it look like a vHBA and add it - * to the list of node devices */ - if (testNodeDeviceMockCreateVport(conn, def, wwpn) < 0) - goto cleanup; + * mocking udev. The mock routine will copy an existing vHBA and + * rename a few fields to mock that. So in order to allow that to + * work properly, we need to drop our lock */ + testDriverUnlock(driver); + if ((newdef = testNodeDeviceMockCreateVport(conn, wwnn, wwpn))) + dev = virNodeDeviceLookupByName(conn, newdef->name); + testDriverLock(driver); + newdef = NULL; - dev = virGetNodeDevice(conn, def->name); - def = NULL; cleanup: testDriverUnlock(driver); virNodeDeviceDefFree(def); -- 2.7.4

Rather than have them mixed in with the virutil apis, create a separate virvhba.c module and move the vHBA related calls into there. Soon there will be more added. Also modify the names of the functions and some arguments to be more indicative of what is really happening. Adjust the callers respectively. While I was changing fchosttest, rather than the non-descriptive names test1...test6, rename them to match what the test is doing. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/storage_conf.c | 5 +- src/libvirt_private.syms | 17 +- src/node_device/node_device_driver.c | 21 +- src/node_device/node_device_linux_sysfs.c | 19 +- src/storage/storage_backend_scsi.c | 41 +-- src/util/virscsi.c | 4 + src/util/virutil.c | 459 +------------------------- src/util/virutil.h | 27 -- src/util/virvhba.c | 532 ++++++++++++++++++++++++++++++ src/util/virvhba.h | 55 +++ tests/fchosttest.c | 80 ++--- 13 files changed, 684 insertions(+), 578 deletions(-) create mode 100644 src/util/virvhba.c create mode 100644 src/util/virvhba.h diff --git a/po/POTFILES.in b/po/POTFILES.in index e464c04..1331093 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -256,6 +256,7 @@ src/util/virtypedparam.c src/util/viruri.c src/util/virusb.c src/util/virutil.c +src/util/virvhba.c src/util/virxml.c src/vbox/vbox_MSCOMGlue.c src/vbox/vbox_XPCOMCGlue.c diff --git a/src/Makefile.am b/src/Makefile.am index dc26ddf..0fec45b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -184,6 +184,7 @@ UTIL_SOURCES = \ util/viruri.h util/viruri.c \ util/virutil.c util/virutil.h \ util/viruuid.c util/viruuid.h \ + util/virvhba.c util/virvhba.h \ util/virxdrdefs.h \ util/virxml.c util/virxml.h \ $(NULL) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index c53f080..f8b5228 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -45,6 +45,7 @@ #include "virfile.h" #include "virstring.h" #include "virlog.h" +#include "virvhba.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -2385,8 +2386,8 @@ matchFCHostToSCSIHost(virConnectPtr conn, /* If we find an fc_adapter name, then either libvirt created a vHBA * for this fc_host or a 'virsh nodedev-create' generated a vHBA. */ - if ((name = virGetFCHostNameByWWN(NULL, fc_adapter.data.fchost.wwnn, - fc_adapter.data.fchost.wwpn))) { + if ((name = virVHBAGetHostByWWN(NULL, fc_adapter.data.fchost.wwnn, + fc_adapter.data.fchost.wwpn))) { /* Get the scsi_hostN for the vHBA in order to see if it * matches our scsi_hostnum diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2866a3..429c133 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2675,15 +2675,12 @@ virUSBDeviceSetUsedBy; virDoubleToStr; virEnumFromString; virEnumToString; -virFindFCHostCapableVport; virFindSCSIHostByPCI; virFormatIntDecimal; virGetDeviceID; virGetDeviceUnprivSGIO; virGetEnvAllowSUID; virGetEnvBlockSUID; -virGetFCHostNameByFabricWWN; -virGetFCHostNameByWWN; virGetGroupID; virGetGroupList; virGetGroupName; @@ -2706,11 +2703,8 @@ virGetUserRuntimeDirectory; virGetUserShell; virHexToBin; virIndexToDiskName; -virIsCapableFCHost; -virIsCapableVport; virIsDevMapperDevice; virIsSUID; -virManageVport; virMemoryLimitIsSet; virMemoryLimitTruncate; virMemoryMaxValue; @@ -2718,7 +2712,6 @@ virParseNumber; virParseOwnershipIds; virParseVersionString; virPipeReadUntilEOF; -virReadFCHost; virReadSCSIUniqueId; virScaleInteger; virSetBlocking; @@ -2746,6 +2739,16 @@ virUUIDIsValid; virUUIDParse; +# util/virvhba.h +virVHBAFindVportHost; +virVHBAGetConfig; +virVHBAGetHostByFabricWWN; +virVHBAGetHostByWWN; +virVHBAIsVportCapable; +virVHBAManageVport; +virVHBAPathExists; + + # util/virxml.h virXMLCheckIllegalChars; virXMLChildElementCount; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 5238e23..99eb56c 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -39,7 +39,7 @@ #include "node_device_driver.h" #include "node_device_hal.h" #include "node_device_linux_sysfs.h" -#include "virutil.h" +#include "virvhba.h" #include "viraccessapicheck.h" #include "virnetdev.h" @@ -609,12 +609,8 @@ nodeDeviceCreateXML(virConnectPtr conn, goto cleanup; } - if (virManageVport(parent_host, - wwpn, - wwnn, - VPORT_CREATE) == -1) { + if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0) goto cleanup; - } dev = find_new_device(conn, wwnn, wwpn); /* We don't check the return value, because one way or another, @@ -672,19 +668,12 @@ nodeDeviceDestroy(virNodeDevicePtr dev) virNodeDeviceObjUnlock(obj); obj = NULL; - if (virNodeDeviceGetParentHost(&driver->devs, - dev->name, - parent_name, - &parent_host) == -1) { + if (virNodeDeviceGetParentHost(&driver->devs, dev->name, parent_name, + &parent_host) < 0) goto out; - } - if (virManageVport(parent_host, - wwpn, - wwnn, - VPORT_DELETE) == -1) { + if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) goto out; - } ret = 0; out: diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 13520cd..1c72b07 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -34,6 +34,7 @@ #include "virlog.h" #include "virfile.h" #include "virstring.h" +#include "virvhba.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV @@ -55,34 +56,34 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d) VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); - if (virIsCapableFCHost(NULL, d->scsi_host.host)) { + if (virVHBAPathExists(NULL, d->scsi_host.host)) { d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; - if (!(tmp = virReadFCHost(NULL, d->scsi_host.host, "port_name"))) { + if (!(tmp = virVHBAGetConfig(NULL, d->scsi_host.host, "port_name"))) { VIR_WARN("Failed to read WWPN for host%d", d->scsi_host.host); goto cleanup; } VIR_FREE(d->scsi_host.wwpn); VIR_STEAL_PTR(d->scsi_host.wwpn, tmp); - if (!(tmp = virReadFCHost(NULL, d->scsi_host.host, "node_name"))) { + if (!(tmp = virVHBAGetConfig(NULL, d->scsi_host.host, "node_name"))) { VIR_WARN("Failed to read WWNN for host%d", d->scsi_host.host); goto cleanup; } VIR_FREE(d->scsi_host.wwnn); VIR_STEAL_PTR(d->scsi_host.wwnn, tmp); - if ((tmp = virReadFCHost(NULL, d->scsi_host.host, "fabric_name"))) { + if ((tmp = virVHBAGetConfig(NULL, d->scsi_host.host, "fabric_name"))) { VIR_FREE(d->scsi_host.fabric_wwn); VIR_STEAL_PTR(d->scsi_host.fabric_wwn, tmp); } } - if (virIsCapableVport(NULL, d->scsi_host.host)) { + if (virVHBAIsVportCapable(NULL, d->scsi_host.host)) { d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; - if (!(tmp = virReadFCHost(NULL, d->scsi_host.host, - "max_npiv_vports"))) { + if (!(tmp = virVHBAGetConfig(NULL, d->scsi_host.host, + "max_npiv_vports"))) { VIR_WARN("Failed to read max_npiv_vports for host%d", d->scsi_host.host); goto cleanup; @@ -93,8 +94,8 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d) goto cleanup; } - if (!(tmp = virReadFCHost(NULL, d->scsi_host.host, - "npiv_vports_inuse"))) { + if (!(tmp = virVHBAGetConfig(NULL, d->scsi_host.host, + "npiv_vports_inuse"))) { VIR_WARN("Failed to read npiv_vports_inuse for host%d", d->scsi_host.host); goto cleanup; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 0cc1148..e037a93 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -34,6 +34,7 @@ #include "virfile.h" #include "vircommand.h" #include "virstring.h" +#include "virvhba.h" #include "storage_util.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -193,9 +194,9 @@ getAdapterName(virStoragePoolSourceAdapter adapter) ignore_value(VIR_STRDUP(name, adapter.data.scsi_host.name)); } } else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { - if (!(name = virGetFCHostNameByWWN(NULL, - adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn))) { + if (!(name = virVHBAGetHostByWWN(NULL, + adapter.data.fchost.wwnn, + adapter.data.fchost.wwpn))) { virReportError(VIR_ERR_XML_ERROR, _("Failed to find SCSI host with wwnn='%s', " "wwpn='%s'"), adapter.data.fchost.wwnn, @@ -269,8 +270,8 @@ createVport(virConnectPtr conn, * using the wwnn/wwpn, then a nodedev is already created for * this pool and we don't have to create the vHBA */ - if ((name = virGetFCHostNameByWWN(NULL, adapter->data.fchost.wwnn, - adapter->data.fchost.wwpn))) { + if ((name = virVHBAGetHostByWWN(NULL, adapter->data.fchost.wwnn, + adapter->data.fchost.wwpn))) { /* If a parent was provided, let's make sure the 'name' we've * retrieved has the same parent */ @@ -287,22 +288,22 @@ createVport(virConnectPtr conn, } else if (adapter->data.fchost.parent_wwnn && adapter->data.fchost.parent_wwpn) { if (!(parent_hoststr = - virGetFCHostNameByWWN(NULL, adapter->data.fchost.parent_wwnn, - adapter->data.fchost.parent_wwpn))) { + virVHBAGetHostByWWN(NULL, adapter->data.fchost.parent_wwnn, + adapter->data.fchost.parent_wwpn))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cannot find parent using provided wwnn/wwpn")); goto cleanup; } } else if (adapter->data.fchost.parent_fabric_wwn) { if (!(parent_hoststr = - virGetFCHostNameByFabricWWN(NULL, - adapter->data.fchost.parent_fabric_wwn))) { + virVHBAGetHostByFabricWWN(NULL, + adapter->data.fchost.parent_fabric_wwn))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cannot find parent using provided fabric_wwn")); goto cleanup; } } else { - if (!(parent_hoststr = virFindFCHostCapableVport(NULL))) { + if (!(parent_hoststr = virVHBAFindVportHost(NULL))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("'parent' for vHBA not specified, and " "cannot find one on this host")); @@ -322,9 +323,9 @@ createVport(virConnectPtr conn, * parent. Besides we have a way to determine the parent based on * the 'name' field. */ - if (!skip_capable_check && !virIsCapableFCHost(NULL, parent_host)) { + if (!skip_capable_check && !virVHBAPathExists(NULL, parent_host)) { virReportError(VIR_ERR_XML_ERROR, - _("parent '%s' specified for vHBA is not vport capable"), + _("parent '%s' specified for vHBA does not exist"), parent_hoststr); goto cleanup; } @@ -342,8 +343,8 @@ createVport(virConnectPtr conn, } } - if (virManageVport(parent_host, adapter->data.fchost.wwpn, - adapter->data.fchost.wwnn, VPORT_CREATE) < 0) + if (virVHBAManageVport(parent_host, adapter->data.fchost.wwpn, + adapter->data.fchost.wwnn, VPORT_CREATE) < 0) goto cleanup; virFileWaitForDevices(); @@ -353,8 +354,8 @@ createVport(virConnectPtr conn, * retry logic set to true. If the thread isn't created, then no big * deal since it's still possible to refresh the pool later. */ - if ((name = virGetFCHostNameByWWN(NULL, adapter->data.fchost.wwnn, - adapter->data.fchost.wwpn))) { + if ((name = virVHBAGetHostByWWN(NULL, adapter->data.fchost.wwnn, + adapter->data.fchost.wwpn))) { if (VIR_ALLOC(cbdata) == 0) { memcpy(cbdata->pool_uuid, pool->def->uuid, VIR_UUID_BUFLEN); VIR_STEAL_PTR(cbdata->fchost_name, name); @@ -399,8 +400,8 @@ deleteVport(virConnectPtr conn, return 0; /* Find our vHBA by searching the fc_host sysfs tree for our wwnn/wwpn */ - if (!(name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn))) { + if (!(name = virVHBAGetHostByWWN(NULL, adapter.data.fchost.wwnn, + adapter.data.fchost.wwpn))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to find fc_host for wwnn='%s' and wwpn='%s'"), adapter.data.fchost.wwnn, adapter.data.fchost.wwpn); @@ -422,8 +423,8 @@ deleteVport(virConnectPtr conn, goto cleanup; } - if (virManageVport(parent_host, adapter.data.fchost.wwpn, - adapter.data.fchost.wwnn, VPORT_DELETE) < 0) + if (virVHBAManageVport(parent_host, adapter.data.fchost.wwpn, + adapter.data.fchost.wwnn, VPORT_DELETE) < 0) goto cleanup; ret = 0; diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 4fd8838..22f2677 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -35,6 +35,7 @@ #include <sys/stat.h> #include <unistd.h> +#include "virlog.h" #include "virscsi.h" #include "viralloc.h" #include "virfile.h" @@ -46,6 +47,9 @@ /* For virReportOOMError() and virReportSystemError() */ #define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("util.scsi"); + struct _virUsedByInfo { char *drvname; /* which driver */ char *domname; /* which domain */ diff --git a/src/util/virutil.c b/src/util/virutil.c index 91178d1..51a1394 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1777,7 +1777,7 @@ virGetDeviceUnprivSGIO(const char *path, } #ifdef __linux__ -# define SYSFS_FC_HOST_PATH "/sys/class/fc_host" + # define SYSFS_SCSI_HOST_PATH "/sys/class/scsi_host" /* virReadSCSIUniqueId: @@ -2004,404 +2004,8 @@ virGetSCSIHostNameByParentaddr(unsigned int domain, return name; } -/* virReadFCHost: - * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH - * @host: Host number, E.g. 5 of "fc_host/host5" - * @entry: Name of the sysfs entry to read - * - * Read the value of sysfs "fc_host" entry. - * - * Returns result as a string on success, caller must free @result after - * Otherwise returns NULL. - */ -char * -virReadFCHost(const char *sysfs_prefix, - int host, - const char *entry) -{ - char *sysfs_path = NULL; - char *p = NULL; - char *buf = NULL; - char *result = NULL; - - if (virAsprintf(&sysfs_path, "%s/host%d/%s", - sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, - host, entry) < 0) - goto cleanup; - - if (!virFileExists(sysfs_path)) - goto cleanup; - - if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; - - if ((p = strchr(buf, '\n'))) - *p = '\0'; - - if ((p = strstr(buf, "0x"))) - p += strlen("0x"); - else - p = buf; - - ignore_value(VIR_STRDUP(result, p)); - - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(buf); - return result; -} - -bool -virIsCapableFCHost(const char *sysfs_prefix, - int host) -{ - char *sysfs_path = NULL; - bool ret = false; - - if (virAsprintf(&sysfs_path, "%s/host%d", - sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, - host) < 0) - return false; - - if (virFileExists(sysfs_path)) - ret = true; - - VIR_FREE(sysfs_path); - return ret; -} - -bool -virIsCapableVport(const char *sysfs_prefix, - int host) -{ - char *scsi_host_path = NULL; - char *fc_host_path = NULL; - bool ret = false; - - if (virAsprintf(&fc_host_path, - "%s/host%d/%s", - sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, - host, - "vport_create") < 0) - return false; - - if (virAsprintf(&scsi_host_path, - "%s/host%d/%s", - sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH, - host, - "vport_create") < 0) - goto cleanup; - - if (virFileExists(fc_host_path) || - virFileExists(scsi_host_path)) - ret = true; - - cleanup: - VIR_FREE(fc_host_path); - VIR_FREE(scsi_host_path); - return ret; -} - -int -virManageVport(const int parent_host, - const char *wwpn, - const char *wwnn, - int operation) -{ - int ret = -1; - char *operation_path = NULL, *vport_name = NULL; - const char *operation_file = NULL; - - switch (operation) { - case VPORT_CREATE: - operation_file = "vport_create"; - break; - case VPORT_DELETE: - operation_file = "vport_delete"; - break; - default: - virReportError(VIR_ERR_OPERATION_INVALID, - _("Invalid vport operation (%d)"), operation); - goto cleanup; - } - - if (virAsprintf(&operation_path, - "%s/host%d/%s", - SYSFS_FC_HOST_PATH, - parent_host, - operation_file) < 0) - goto cleanup; - - if (!virFileExists(operation_path)) { - VIR_FREE(operation_path); - if (virAsprintf(&operation_path, - "%s/host%d/%s", - SYSFS_SCSI_HOST_PATH, - parent_host, - operation_file) < 0) - goto cleanup; - - if (!virFileExists(operation_path)) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("vport operation '%s' is not supported for host%d"), - operation_file, parent_host); - goto cleanup; - } - } - - if (virAsprintf(&vport_name, - "%s:%s", - wwpn, - wwnn) < 0) - goto cleanup; - - if (virFileWriteStr(operation_path, vport_name, 0) == 0) - ret = 0; - else - virReportSystemError(errno, - _("Write of '%s' to '%s' during " - "vport create/delete failed"), - vport_name, operation_path); - - cleanup: - VIR_FREE(vport_name); - VIR_FREE(operation_path); - return ret; -} - - -/* virReadCompareWWN - * @prefix: path to the wwn file - * @d_name: name of the current directory - * @f_name: file name to read - * - * Read/compare the on-disk file with the passed wwn value. - * - * Returns: - * -1 : Error - * 0 : No match - * 1 : Match - */ -static int -virReadCompareWWN(const char *prefix, - const char *d_name, - const char *f_name, - const char *wwn) -{ - char *path; - char *buf = NULL; - char *p; - int ret = -1; - - if (virAsprintf(&path, "%s/%s/%s", prefix, d_name, f_name) < 0) - return -1; - - if (!virFileExists(path)) { - ret = 0; - goto cleanup; - } - - if (virFileReadAll(path, 1024, &buf) < 0) - goto cleanup; - - if ((p = strchr(buf, '\n'))) - *p = '\0'; - if (STRPREFIX(buf, "0x")) - p = buf + strlen("0x"); - else - p = buf; - - if (STRNEQ(wwn, p)) - ret = 0; - else - ret = 1; - - cleanup: - VIR_FREE(path); - VIR_FREE(buf); - - return ret; -} - -/* virGetFCHostNameByWWN: - * - * Iterate over the sysfs tree to get FC host name (e.g. host5) - * by the provided "wwnn,wwpn" pair. - * - * Returns the FC host name which must be freed by the caller, - * or NULL on failure. - */ -char * -virGetFCHostNameByWWN(const char *sysfs_prefix, - const char *wwnn, - const char *wwpn) -{ - const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; - struct dirent *entry = NULL; - DIR *dir = NULL; - char *ret = NULL; - - if (virDirOpen(&dir, prefix) < 0) - return NULL; - - while (virDirRead(dir, &entry, prefix) > 0) { - int rc; - - if ((rc = virReadCompareWWN(prefix, entry->d_name, - "node_name", wwnn)) < 0) - goto cleanup; - - if (rc == 0) - continue; - - if ((rc = virReadCompareWWN(prefix, entry->d_name, - "port_name", wwpn)) < 0) - goto cleanup; - - if (rc == 0) - continue; - - ignore_value(VIR_STRDUP(ret, entry->d_name)); - break; - } - - cleanup: - VIR_DIR_CLOSE(dir); - return ret; -} - -/* virGetFCHostNameByFabricWWN: - * - * Iterate over the sysfs tree to get FC host name (e.g. host5) - * by the provided "fabric_wwn". This would find a host on a SAN. - * - * Returns the FC host name which must be freed by the caller, - * or NULL on failure. - */ -char * -virGetFCHostNameByFabricWWN(const char *sysfs_prefix, - const char *fabric_wwn) -{ - const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; - struct dirent *entry = NULL; - DIR *dir = NULL; - char *vport_create_path = NULL; - char *ret = NULL; - - if (virDirOpen(&dir, prefix) < 0) - return NULL; - - while (virDirRead(dir, &entry, prefix) > 0) { - int rc; - - VIR_FREE(vport_create_path); - - /* Existing vHBA's will have the same fabric_name, but won't - * have the vport_create file - so we check for both */ - if (virAsprintf(&vport_create_path, "%s/%s/vport_create", prefix, - entry->d_name) < 0) - goto cleanup; - - if (!virFileExists(vport_create_path)) - continue; - - if ((rc = virReadCompareWWN(prefix, entry->d_name, - "fabric_name", fabric_wwn)) < 0) - goto cleanup; - - if (rc == 0) - continue; - - ignore_value(VIR_STRDUP(ret, entry->d_name)); - break; - } - - cleanup: - VIR_DIR_CLOSE(dir); - VIR_FREE(vport_create_path); - return ret; -} - -# define PORT_STATE_ONLINE "Online" - -/* virFindFCHostCapableVport: - * - * Iterate over the sysfs and find out the first online HBA which - * supports vport, and not saturated. Returns the host name (e.g. - * host5) on success, or NULL on failure. - */ -char * -virFindFCHostCapableVport(const char *sysfs_prefix) -{ - const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; - DIR *dir = NULL; - struct dirent *entry = NULL; - char *max_vports = NULL; - char *vports = NULL; - char *state = NULL; - char *ret = NULL; - - if (virDirOpen(&dir, prefix) < 0) - return NULL; - - while (virDirRead(dir, &entry, prefix) > 0) { - unsigned int host; - char *p = NULL; - - p = entry->d_name + strlen("host"); - if (virStrToLong_ui(p, NULL, 10, &host) == -1) { - VIR_DEBUG("Failed to parse host number from '%s'", - entry->d_name); - continue; - } - - if (!virIsCapableVport(prefix, host)) - continue; - - if (!(state = virReadFCHost(prefix, host, "port_state"))) { - VIR_DEBUG("Failed to read port_state for host%d", host); - continue; - } - - /* Skip the not online FC host */ - if (STRNEQ(state, PORT_STATE_ONLINE)) { - VIR_FREE(state); - continue; - } - VIR_FREE(state); - - if (!(max_vports = virReadFCHost(prefix, host, "max_npiv_vports"))) { - VIR_DEBUG("Failed to read max_npiv_vports for host%d", host); - continue; - } - - if (!(vports = virReadFCHost(prefix, host, "npiv_vports_inuse"))) { - VIR_DEBUG("Failed to read npiv_vports_inuse for host%d", host); - VIR_FREE(max_vports); - continue; - } - - /* Compare from the strings directly, instead of converting - * the strings to integers first - */ - if ((strlen(max_vports) >= strlen(vports)) || - ((strlen(max_vports) == strlen(vports)) && - strcmp(max_vports, vports) > 0)) { - ignore_value(VIR_STRDUP(ret, entry->d_name)); - goto cleanup; - } - - VIR_FREE(max_vports); - VIR_FREE(vports); - } - - cleanup: - VIR_DIR_CLOSE(dir); - VIR_FREE(max_vports); - VIR_FREE(vports); - return ret; -} #else + int virReadSCSIUniqueId(const char *sysfs_prefix ATTRIBUTE_UNUSED, int host ATTRIBUTE_UNUSED, @@ -2439,65 +2043,6 @@ virGetSCSIHostNameByParentaddr(unsigned int domain ATTRIBUTE_UNUSED, return NULL; } -char * -virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, - int host ATTRIBUTE_UNUSED, - const char *entry ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return NULL; -} - -bool -virIsCapableFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, - int host ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return false; -} - -bool -virIsCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED, - int host ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return false; -} - -int -virManageVport(const int parent_host ATTRIBUTE_UNUSED, - const char *wwpn ATTRIBUTE_UNUSED, - const char *wwnn ATTRIBUTE_UNUSED, - int operation ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return -1; -} - -char * -virGetFCHostNameByWWN(const char *sysfs_prefix ATTRIBUTE_UNUSED, - const char *wwnn ATTRIBUTE_UNUSED, - const char *wwpn ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return NULL; -} - -char * -virGetFCHostNameByFabricWWN(const char *sysfs_prefix ATTRIBUTE_UNUSED, - const char *fabric_wwn ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return NULL; -} - -char * -virFindFCHostCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return NULL; -} - #endif /* __linux__ */ /** diff --git a/src/util/virutil.h b/src/util/virutil.h index 3fbd7b0..b320c06 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -182,35 +182,8 @@ virGetSCSIHostNameByParentaddr(unsigned int domain, unsigned int slot, unsigned int function, unsigned int unique_id); -char *virReadFCHost(const char *sysfs_prefix, - int host, - const char *entry) - ATTRIBUTE_NONNULL(3); - -bool virIsCapableFCHost(const char *sysfs_prefix, int host); -bool virIsCapableVport(const char *sysfs_prefix, int host); - -enum { - VPORT_CREATE, - VPORT_DELETE, -}; - -int virManageVport(const int parent_host, - const char *wwpn, - const char *wwnn, - int operation) - ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); - -char *virGetFCHostNameByWWN(const char *sysfs_prefix, - const char *wwnn, - const char *wwpn) - ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); -char *virGetFCHostNameByFabricWWN(const char *sysfs_prefix, - const char *fabric_wwn) - ATTRIBUTE_NONNULL(2); -char *virFindFCHostCapableVport(const char *sysfs_prefix); int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr); diff --git a/src/util/virvhba.c b/src/util/virvhba.c new file mode 100644 index 0000000..e5637d7 --- /dev/null +++ b/src/util/virvhba.c @@ -0,0 +1,532 @@ +/* + * virvhba.c: Generic vHBA management utility functions + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "viralloc.h" +#include "virerror.h" +#include "virfile.h" +#include "virlog.h" +#include "virstring.h" +#include "virvhba.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("util.vhba"); + +#ifdef __linux__ + +# define SYSFS_SCSI_HOST_PATH "/sys/class/scsi_host" +# define SYSFS_FC_HOST_PATH "/sys/class/fc_host" +# define PORT_STATE_ONLINE "Online" + + +/* virVHBAPathExists: + * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH + * @host: Host number, E.g. 5 of "fc_host/host5" + * + * Check if the "fc_host" to provided host# exists. This path may be either + * a vHBA capable path or a vHBA itself. + * + * Returns true if it does, false if not + */ +bool +virVHBAPathExists(const char *sysfs_prefix, + int host) +{ + char *sysfs_path = NULL; + bool ret = false; + + if (virAsprintf(&sysfs_path, "%s/host%d", + sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, + host) < 0) + return false; + + if (virFileExists(sysfs_path)) + ret = true; + + VIR_FREE(sysfs_path); + return ret; +} + + +/* virVHBAIsVportCapable: + * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH + * @host: Host number, E.g. 5 of "fc_host/host5" + * + * Not all vHBA paths can create/delete a vport - only the parent NPIV + * capable HBA has the "vport_create" and "vport_delete" functions. + * A vHBA created path does not have the function files. + * + * NB: Checks both the "fc_host" and "scsi_host" paths. + * + * Returns true if capable, false if not + */ +bool +virVHBAIsVportCapable(const char *sysfs_prefix, + int host) +{ + char *scsi_host_path = NULL; + char *fc_host_path = NULL; + bool ret = false; + + if (virAsprintf(&fc_host_path, + "%s/host%d/%s", + sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, + host, + "vport_create") < 0) + return false; + + if (virAsprintf(&scsi_host_path, + "%s/host%d/%s", + sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH, + host, + "vport_create") < 0) + goto cleanup; + + if (virFileExists(fc_host_path) || virFileExists(scsi_host_path)) + ret = true; + + cleanup: + VIR_FREE(fc_host_path); + VIR_FREE(scsi_host_path); + return ret; +} + + +/* virVHBAGetConfig: + * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH + * @host: Host number, E.g. 5 of "fc_host/host5" + * @entry: Name of the FC sysfs entry to read + * + * Read the value of a vHBA sysfs "fc_host" entry (if it exists). + * + * Returns result as a string on success, caller must free @result after + * Otherwise returns NULL. + */ +char * +virVHBAGetConfig(const char *sysfs_prefix, + int host, + const char *entry) +{ + char *sysfs_path = NULL; + char *p = NULL; + char *buf = NULL; + char *result = NULL; + + if (virAsprintf(&sysfs_path, "%s/host%d/%s", + sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, + host, entry) < 0) + goto cleanup; + + if (!virFileExists(sysfs_path)) + goto cleanup; + + if (virFileReadAll(sysfs_path, 1024, &buf) < 0) + goto cleanup; + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + + if ((p = strstr(buf, "0x"))) + p += strlen("0x"); + else + p = buf; + + ignore_value(VIR_STRDUP(result, p)); + + cleanup: + VIR_FREE(sysfs_path); + VIR_FREE(buf); + return result; +} + + +/* virVHBAFindVportHost: + * + * Iterate over the sysfs and find out the first online HBA which + * supports vport, and is not saturated. Returns the host name (e.g. + * host5) on success, or NULL on failure. + * + * It's up to the caller to free the returned string. + */ +char * +virVHBAFindVportHost(const char *sysfs_prefix) +{ + const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; + DIR *dir = NULL; + struct dirent *entry = NULL; + char *max_vports = NULL; + char *vports = NULL; + char *state = NULL; + char *ret = NULL; + + if (virDirOpen(&dir, prefix) < 0) + return NULL; + + while (virDirRead(dir, &entry, prefix) > 0) { + unsigned int host; + char *p = NULL; + + p = entry->d_name + strlen("host"); + if (virStrToLong_ui(p, NULL, 10, &host) == -1) { + VIR_DEBUG("Failed to parse host number from '%s'", + entry->d_name); + continue; + } + + if (!virVHBAPathExists(prefix, host)) + continue; + + if (!(state = virVHBAGetConfig(prefix, host, "port_state"))) { + VIR_DEBUG("Failed to read port_state for host%d", host); + continue; + } + + /* Skip the not online FC host */ + if (STRNEQ(state, PORT_STATE_ONLINE)) { + VIR_FREE(state); + continue; + } + VIR_FREE(state); + + if (!(max_vports = virVHBAGetConfig(prefix, host, "max_npiv_vports"))) { + VIR_DEBUG("Failed to read max_npiv_vports for host%d", host); + continue; + } + + if (!(vports = virVHBAGetConfig(prefix, host, "npiv_vports_inuse"))) { + VIR_DEBUG("Failed to read npiv_vports_inuse for host%d", host); + VIR_FREE(max_vports); + continue; + } + + /* Compare from the strings directly, instead of converting + * the strings to integers first + */ + if ((strlen(max_vports) >= strlen(vports)) || + ((strlen(max_vports) == strlen(vports)) && + strcmp(max_vports, vports) > 0)) { + ignore_value(VIR_STRDUP(ret, entry->d_name)); + goto cleanup; + } + + VIR_FREE(max_vports); + VIR_FREE(vports); + } + + cleanup: + VIR_DIR_CLOSE(dir); + VIR_FREE(max_vports); + VIR_FREE(vports); + return ret; +} + +/* virVHBAManageVport: + * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH + * @wwnn: world wide node name used to create/delete the vport + * @wwpn: world wide port name used to create/delete the vport + * @operation: create or delete + * + * NB: Checks both the "fc_host" and "scsi_host" paths. + * Returns true if capable, false if not + */ +int +virVHBAManageVport(const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) +{ + int ret = -1; + char *operation_path = NULL, *vport_name = NULL; + const char *operation_file = NULL; + + switch (operation) { + case VPORT_CREATE: + operation_file = "vport_create"; + break; + case VPORT_DELETE: + operation_file = "vport_delete"; + break; + default: + virReportError(VIR_ERR_OPERATION_INVALID, + _("Invalid vport operation (%d)"), operation); + goto cleanup; + } + + if (virAsprintf(&operation_path, "%s/host%d/%s", + SYSFS_FC_HOST_PATH, parent_host, operation_file) < 0) + goto cleanup; + + if (!virFileExists(operation_path)) { + VIR_FREE(operation_path); + if (virAsprintf(&operation_path, "%s/host%d/%s", + SYSFS_SCSI_HOST_PATH, parent_host, operation_file) < 0) + goto cleanup; + + if (!virFileExists(operation_path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("vport operation '%s' is not supported " + "for host%d"), + operation_file, parent_host); + goto cleanup; + } + } + + /* Create/Delete is handle through the file passing the wwpn:wwnn as + * a parameter. This results in the kernel managing the port and an + * event posted. That event will be recogized and the Node Device driver + * will then take over either creating or deleting the vHBA */ + if (virAsprintf(&vport_name, "%s:%s", wwpn, wwnn) < 0) + goto cleanup; + + if (virFileWriteStr(operation_path, vport_name, 0) == 0) + ret = 0; + else + virReportSystemError(errno, + _("Write of '%s' to '%s' during " + "vport create/delete failed"), + vport_name, operation_path); + + cleanup: + VIR_FREE(vport_name); + VIR_FREE(operation_path); + return ret; +} + + +/* vhbaReadCompareWWN + * @prefix: path to the wwn file + * @d_name: name of the current directory + * @f_name: file name to read + * + * Read/compare the on-disk file with the passed wwn value. + * + * Returns: + * -1 : Error + * 0 : No match + * 1 : Match + */ +static int +vhbaReadCompareWWN(const char *prefix, + const char *d_name, + const char *f_name, + const char *wwn) +{ + char *path; + char *buf = NULL; + char *p; + int ret = -1; + + if (virAsprintf(&path, "%s/%s/%s", prefix, d_name, f_name) < 0) + return -1; + + if (!virFileExists(path)) { + ret = 0; + goto cleanup; + } + + if (virFileReadAll(path, 1024, &buf) < 0) + goto cleanup; + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + if (STRPREFIX(buf, "0x")) + p = buf + strlen("0x"); + else + p = buf; + + if (STRNEQ(wwn, p)) + ret = 0; + else + ret = 1; + + cleanup: + VIR_FREE(path); + VIR_FREE(buf); + + return ret; +} + +/* virVHBAGetHostByWWN: + * + * Iterate over the sysfs tree to get FC host name (e.g. host5) + * by the provided "wwnn,wwpn" pair. + * + * Returns the FC host name which must be freed by the caller, + * or NULL on failure. + */ +char * +virVHBAGetHostByWWN(const char *sysfs_prefix, + const char *wwnn, + const char *wwpn) +{ + const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; + struct dirent *entry = NULL; + DIR *dir = NULL; + char *ret = NULL; + + if (virDirOpen(&dir, prefix) < 0) + return NULL; + + while (virDirRead(dir, &entry, prefix) > 0) { + int rc; + + if ((rc = vhbaReadCompareWWN(prefix, entry->d_name, + "node_name", wwnn)) < 0) + goto cleanup; + + if (rc == 0) + continue; + + if ((rc = vhbaReadCompareWWN(prefix, entry->d_name, + "port_name", wwpn)) < 0) + goto cleanup; + + if (rc == 0) + continue; + + ignore_value(VIR_STRDUP(ret, entry->d_name)); + break; + } + + cleanup: + VIR_DIR_CLOSE(dir); + return ret; +} + +/* virVHBAGetHostByFabricWWN: + * + * Iterate over the sysfs tree to get FC host name (e.g. host5) + * by the provided "fabric_wwn". This would find a host on a SAN. + * + * Returns the FC host name which must be freed by the caller, + * or NULL on failure. + */ +char * +virVHBAGetHostByFabricWWN(const char *sysfs_prefix, + const char *fabric_wwn) +{ + const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; + struct dirent *entry = NULL; + DIR *dir = NULL; + char *vport_create_path = NULL; + char *ret = NULL; + + if (virDirOpen(&dir, prefix) < 0) + return NULL; + + while (virDirRead(dir, &entry, prefix) > 0) { + int rc; + + VIR_FREE(vport_create_path); + + /* Existing vHBA's will have the same fabric_name, but won't + * have the vport_create file - so we check for both */ + if (virAsprintf(&vport_create_path, "%s/%s/vport_create", prefix, + entry->d_name) < 0) + goto cleanup; + + if (!virFileExists(vport_create_path)) + continue; + + if ((rc = vhbaReadCompareWWN(prefix, entry->d_name, + "fabric_name", fabric_wwn)) < 0) + goto cleanup; + + if (rc == 0) + continue; + + ignore_value(VIR_STRDUP(ret, entry->d_name)); + break; + } + + cleanup: + VIR_DIR_CLOSE(dir); + VIR_FREE(vport_create_path); + return ret; +} + +#else + +bool +virVHBAPathExists(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return false; +} + + +bool +virVHBAVportIsCapable(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return false; +} + + +char * +virVHBAGetConfig(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED, + const char *entry ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + + +char * +virVHBAFindVportHost(const char *sysfs_prefix ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + + +int +virVHBAManageVport(const int parent_host ATTRIBUTE_UNUSED, + const char *wwpn ATTRIBUTE_UNUSED, + const char *wwnn ATTRIBUTE_UNUSED, + int operation ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + + +char * +virVHBAGetHostByWWN(const char *sysfs_prefix ATTRIBUTE_UNUSED, + const char *wwnn ATTRIBUTE_UNUSED, + const char *wwpn ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + + +char * +virVHBAGetHostByFabricWWN(const char *sysfs_prefix ATTRIBUTE_UNUSED, + const char *fabric_wwn ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + +#endif /* __linux__ */ diff --git a/src/util/virvhba.h b/src/util/virvhba.h new file mode 100644 index 0000000..e338f96 --- /dev/null +++ b/src/util/virvhba.h @@ -0,0 +1,55 @@ +/* + * virvhba.h: Generic vHBA management utility functions + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#ifndef __VIR_VHBA_H__ +# define __VIR_VHBA_H__ + +# include "internal.h" + +enum { + VPORT_CREATE, + VPORT_DELETE, +}; + +bool virVHBAPathExists(const char *sysfs_prefix, int host); + +bool virVHBAIsVportCapable(const char *sysfs_prefix, int host); + +char *virVHBAGetConfig(const char *sysfs_prefix, + int host, + const char *entry) + ATTRIBUTE_NONNULL(3); + +char *virVHBAFindVportHost(const char *sysfs_prefix); + +int virVHBAManageVport(const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +char *virVHBAGetHostByWWN(const char *sysfs_prefix, + const char *wwnn, + const char *wwpn) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +char *virVHBAGetHostByFabricWWN(const char *sysfs_prefix, + const char *fabric_wwn) + ATTRIBUTE_NONNULL(2); + +#endif /* __VIR_VBHA_H__ */ diff --git a/tests/fchosttest.c b/tests/fchosttest.c index 0b4a8f2..39a06f3 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -20,7 +20,7 @@ #include <config.h> #include "virstring.h" -#include "virutil.h" +#include "virvhba.h" #include "testutils.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -31,31 +31,28 @@ static char *fchost_prefix; #define TEST_FC_HOST_NUM 5 #define TEST_FC_HOST_NUM_NO_FAB 6 -/* Test virIsCapableFCHost */ +/* Test virIsVHBACapable */ static int test1(const void *data ATTRIBUTE_UNUSED) { - if (virIsCapableFCHost(TEST_FC_HOST_PREFIX, - TEST_FC_HOST_NUM) && - virIsCapableFCHost(TEST_FC_HOST_PREFIX, - TEST_FC_HOST_NUM_NO_FAB)) + if (virVHBAPathExists(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM) && + virVHBAPathExists(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM_NO_FAB)) return 0; return -1; } -/* Test virIsCapableVport */ +/* Test virVHBAIsVportCapable */ static int test2(const void *data ATTRIBUTE_UNUSED) { - if (virIsCapableVport(TEST_FC_HOST_PREFIX, - TEST_FC_HOST_NUM)) + if (virVHBAIsVportCapable(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM)) return 0; return -1; } -/* Test virReadFCHost */ +/* Test virVHBAGetConfig */ static int test3(const void *data ATTRIBUTE_UNUSED) { @@ -71,25 +68,25 @@ test3(const void *data ATTRIBUTE_UNUSED) char *vports = NULL; int ret = -1; - if (!(wwnn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, - "node_name"))) + if (!(wwnn = virVHBAGetConfig(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, + "node_name"))) return -1; - if (!(wwpn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, - "port_name"))) + if (!(wwpn = virVHBAGetConfig(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, + "port_name"))) goto cleanup; - if (!(fabric_wwn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, - "fabric_name"))) + if (!(fabric_wwn = virVHBAGetConfig(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, + "fabric_name"))) goto cleanup; - if (!(max_vports = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, - "max_npiv_vports"))) + if (!(max_vports = virVHBAGetConfig(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, + "max_npiv_vports"))) goto cleanup; - if (!(vports = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, - "npiv_vports_inuse"))) + if (!(vports = virVHBAGetConfig(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM, + "npiv_vports_inuse"))) goto cleanup; if (STRNEQ(expect_wwnn, wwnn) || @@ -109,7 +106,7 @@ test3(const void *data ATTRIBUTE_UNUSED) return ret; } -/* Test virGetFCHostNameByWWN */ +/* Test virVHBAGetHostByWWN */ static int test4(const void *data ATTRIBUTE_UNUSED) { @@ -117,9 +114,9 @@ test4(const void *data ATTRIBUTE_UNUSED) char *hostname = NULL; int ret = -1; - if (!(hostname = virGetFCHostNameByWWN(TEST_FC_HOST_PREFIX, - "2001001b32a9da4e", - "2101001b32a9da4e"))) + if (!(hostname = virVHBAGetHostByWWN(TEST_FC_HOST_PREFIX, + "2001001b32a9da4e", + "2101001b32a9da4e"))) return -1; if (STRNEQ(hostname, expect_hostname)) @@ -131,7 +128,10 @@ test4(const void *data ATTRIBUTE_UNUSED) return ret; } -/* Test virFindFCHostCapableVport (host4 is not Online) */ +/* Test virVHBAFindVportHost + * + * NB: host4 is not Online, so it should not be found + */ static int test5(const void *data ATTRIBUTE_UNUSED) { @@ -139,7 +139,7 @@ test5(const void *data ATTRIBUTE_UNUSED) char *hostname = NULL; int ret = -1; - if (!(hostname = virFindFCHostCapableVport(TEST_FC_HOST_PREFIX))) + if (!(hostname = virVHBAFindVportHost(TEST_FC_HOST_PREFIX))) return -1; if (STRNEQ(hostname, expect_hostname)) @@ -151,7 +151,7 @@ test5(const void *data ATTRIBUTE_UNUSED) return ret; } -/* Test virReadFCHost fabric name optional */ +/* Test virVHBAGetConfig fabric name optional */ static int test6(const void *data ATTRIBUTE_UNUSED) { @@ -162,17 +162,17 @@ test6(const void *data ATTRIBUTE_UNUSED) char *fabric_wwn = NULL; int ret = -1; - if (!(wwnn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM_NO_FAB, - "node_name"))) + if (!(wwnn = virVHBAGetConfig(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM_NO_FAB, + "node_name"))) return -1; - if (!(wwpn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM_NO_FAB, - "port_name"))) + if (!(wwpn = virVHBAGetConfig(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM_NO_FAB, + "port_name"))) goto cleanup; - if ((fabric_wwn = virReadFCHost(TEST_FC_HOST_PREFIX, - TEST_FC_HOST_NUM_NO_FAB, - "fabric_name"))) + if ((fabric_wwn = virVHBAGetConfig(TEST_FC_HOST_PREFIX, + TEST_FC_HOST_NUM_NO_FAB, + "fabric_name"))) goto cleanup; if (STRNEQ(expect_wwnn, wwnn) || @@ -198,17 +198,17 @@ mymain(void) goto cleanup; } - if (virTestRun("test1", test1, NULL) < 0) + if (virTestRun("virVHBAPathExists", test1, NULL) < 0) ret = -1; - if (virTestRun("test2", test2, NULL) < 0) + if (virTestRun("virVHBAIsVportCapable", test2, NULL) < 0) ret = -1; - if (virTestRun("test3", test3, NULL) < 0) + if (virTestRun("virVHBAGetConfig", test3, NULL) < 0) ret = -1; - if (virTestRun("test4", test4, NULL) < 0) + if (virTestRun("virVHBAGetHostByWWN", test4, NULL) < 0) ret = -1; - if (virTestRun("test5", test5, NULL) < 0) + if (virTestRun("virVHBAFindVportHost", test5, NULL) < 0) ret = -1; - if (virTestRun("test6", test6, NULL) < 0) + if (virTestRun("virVHBAGetConfig-empty-fabric_wwn", test6, NULL) < 0) ret = -1; cleanup: -- 2.7.4

Rather that getting the XML, parsing it, and grabbing the parent name field, just call the virNodeDeviceGetParent which is far more efficient. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virvhba.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/util/virvhba.c b/src/util/virvhba.c index 8c4da3a..2ad3520 100644 --- a/src/util/virvhba.c +++ b/src/util/virvhba.c @@ -577,16 +577,10 @@ virVHBAGetParent(virConnectPtr conn, goto cleanup; } - if (!(xml = virNodeDeviceGetXMLDesc(device, 0))) - goto cleanup; - - if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL))) - goto cleanup; - /* The caller checks whether the returned value is NULL or not * before continuing */ - ignore_value(VIR_STRDUP(vhba_parent, def->parent)); + ignore_value(VIR_STRDUP(vhba_parent, virNodeDeviceGetParent(device))); cleanup: VIR_FREE(nodedev_name); -- 2.7.4

Add a test that will mimic creation and destruction of a vHBA by using node device XML. The design will allow for testing the multiple mechanisms. The first test uses just <parent> in the node device XML. This is somewhat similar to the existing objecteventtest, except that this test will not provide input wwnn/wwpn's (similar to how the process is described for the the libvirt wiki). This requires mocking the virRandomGenerateWWN since parsing the input XML (virNodeDevCapSCSIHostParseXML) requires either a provided wwnn/wwpn in the XML or the ability to randomly generate the wwnn/wwpn. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- tests/fchosttest.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++- tests/virrandommock.c | 9 ++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/tests/fchosttest.c b/tests/fchosttest.c index 39a06f3..715571e 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -19,18 +19,31 @@ #include <config.h> +#include "virlog.h" #include "virstring.h" #include "virvhba.h" #include "testutils.h" #define VIR_FROM_THIS VIR_FROM_NONE +VIR_LOG_INIT("tests.fchosttest"); + static char *fchost_prefix; #define TEST_FC_HOST_PREFIX fchost_prefix #define TEST_FC_HOST_NUM 5 #define TEST_FC_HOST_NUM_NO_FAB 6 +/* virNodeDeviceCreateXML using "<parent>" to find the vport capable HBA */ +static const char test7_xml[] = +"<device>" +" <parent>scsi_host1</parent>" +" <capability type='scsi_host'>" +" <capability type='fc_host'>" +" </capability>" +" </capability>" +"</device>"; + /* Test virIsVHBACapable */ static int test1(const void *data ATTRIBUTE_UNUSED) @@ -187,6 +200,50 @@ test6(const void *data ATTRIBUTE_UNUSED) return ret; } + + +/* Test manageVHBAByNodeDevice + * - Test both virNodeDeviceCreateXML and virNodeDeviceDestroy + * - Create a node device vHBA allowing usage of various different + * methods based on the input data/xml argument. + * - Be sure that it's possible to destroy the node device as well. + */ +static int +manageVHBAByNodeDevice(const void *data) +{ + const char *expect_hostname = "scsi_host12"; + virConnectPtr conn = NULL; + virNodeDevicePtr dev = NULL; + int ret = -1; + const char *vhba = data; + + if (!(conn = virConnectOpen("test:///default"))) + return -1; + + if (!(dev = virNodeDeviceCreateXML(conn, vhba, 0))) + goto cleanup; + + if (virNodeDeviceDestroy(dev) < 0) + goto cleanup; + + if (STRNEQ(virNodeDeviceGetName(dev), expect_hostname)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected hostname: '%s' got: '%s'", + expect_hostname, virNodeDeviceGetName(dev)); + goto cleanup; + } + + ret = 0; + + cleanup: + if (dev) + virNodeDeviceFree(dev); + if (conn) + virConnectClose(conn); + return ret; +} + + static int mymain(void) { @@ -210,10 +267,13 @@ mymain(void) ret = -1; if (virTestRun("virVHBAGetConfig-empty-fabric_wwn", test6, NULL) < 0) ret = -1; + if (virTestRun("manageVHBAByNodeDevice-by-parent", manageVHBAByNodeDevice, + test7_xml) < 0) + ret = -1; cleanup: VIR_FREE(fchost_prefix); return ret; } -VIRT_TEST_MAIN(mymain) +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virrandommock.so") diff --git a/tests/virrandommock.c b/tests/virrandommock.c index a69712a..fd1a61f 100644 --- a/tests/virrandommock.c +++ b/tests/virrandommock.c @@ -23,6 +23,7 @@ #ifndef WIN32 # include "internal.h" +# include "virstring.h" # include "virrandom.h" # include "virmock.h" @@ -41,6 +42,14 @@ virRandomBytes(unsigned char *buf, } +int virRandomGenerateWWN(char **wwn, + const char *virt_type ATTRIBUTE_UNUSED) +{ + return virAsprintf(wwn, "5100000%09llx", + (unsigned long long)virRandomBits(36)); +} + + # ifdef WITH_GNUTLS # include <stdio.h> # include <gnutls/gnutls.h> -- 2.7.4

Rework the code to perform the various searches by parent, parent_wwnn/ parent_wwpn, parent_fabric_wwn, or vport capable in order to return the 'parent_host' number that is vHBA capable. The former virNodeDeviceGetParentHost is renamed to add the ByParent on it fixes an issue where if no parent was supplied in the XML to create the vHBA, then virNodeDeviceFindByName was called with a NULL second parameter which had bad results. The reworked code will make the various calls to fetch the NPIV host by the passed parameter options or if none are provided find a vport capable NPIV HBA to perform the create. If the call is from the delete path, then this option won't be allowed. Each of virNodeDeviceGetParentHostBy* functions is now static, so remove them external definitions. Alter the calling logic to match the A secondary benefit of this is the test_driver now can make use of the new API to add some new tests to test the various creation options. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/node_device_conf.c | 68 +++++++++++++++++++++++------------- src/conf/node_device_conf.h | 19 ++-------- src/libvirt_private.syms | 3 -- src/node_device/node_device_driver.c | 55 ++++++++--------------------- src/test/test_driver.c | 24 ++++++------- 5 files changed, 71 insertions(+), 98 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4d3268f..f73fede 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1877,17 +1877,15 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def, */ /* virNodeDeviceFindFCParentHost: * @parent: Pointer to node device object - * @parent_host: Pointer to return parent host number * * Search the capabilities for the device to find the FC capabilities * in order to set the parent_host value. * * Returns: - * 0 on success with parent_host set, -1 otherwise; + * parent_host value on success (>= 0), -1 otherwise. */ static int -virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent, - int *parent_host) +virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent) { virNodeDevCapsDefPtr cap = virNodeDeviceFindVPORTCapDef(parent); @@ -1899,16 +1897,14 @@ virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent, return -1; } - *parent_host = cap->data.scsi_host.host; - return 0; + return cap->data.scsi_host.host; } -int -virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, - const char *dev_name, - const char *parent_name, - int *parent_host) +static int +virNodeDeviceGetParentHostByParent(virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_name) { virNodeDeviceObjPtr parent = NULL; int ret; @@ -1920,7 +1916,7 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, return -1; } - ret = virNodeDeviceFindFCParentHost(parent, parent_host); + ret = virNodeDeviceFindFCParentHost(parent); virNodeDeviceObjUnlock(parent); @@ -1928,12 +1924,11 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, } -int +static int virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, const char *dev_name, const char *parent_wwnn, - const char *parent_wwpn, - int *parent_host) + const char *parent_wwpn) { virNodeDeviceObjPtr parent = NULL; int ret; @@ -1945,7 +1940,7 @@ virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, return -1; } - ret = virNodeDeviceFindFCParentHost(parent, parent_host); + ret = virNodeDeviceFindFCParentHost(parent); virNodeDeviceObjUnlock(parent); @@ -1953,11 +1948,10 @@ virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, } -int +static int virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, const char *dev_name, - const char *parent_fabric_wwn, - int *parent_host) + const char *parent_fabric_wwn) { virNodeDeviceObjPtr parent = NULL; int ret; @@ -1969,7 +1963,7 @@ virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, return -1; } - ret = virNodeDeviceFindFCParentHost(parent, parent_host); + ret = virNodeDeviceFindFCParentHost(parent); virNodeDeviceObjUnlock(parent); @@ -1977,9 +1971,8 @@ virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, } -int -virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, - int *parent_host) +static int +virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs) { virNodeDeviceObjPtr parent = NULL; const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS); @@ -1991,7 +1984,7 @@ virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, return -1; } - ret = virNodeDeviceFindFCParentHost(parent, parent_host); + ret = virNodeDeviceFindFCParentHost(parent); virNodeDeviceObjUnlock(parent); @@ -1999,6 +1992,33 @@ virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, } +int +virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, + virNodeDeviceDefPtr def, + int create) +{ + int parent_host = -1; + + if (def->parent) { + parent_host = virNodeDeviceGetParentHostByParent(devs, def->name, + def->parent); + } else if (def->parent_wwnn && def->parent_wwpn) { + parent_host = virNodeDeviceGetParentHostByWWNs(devs, def->name, + def->parent_wwnn, + def->parent_wwpn); + } else if (def->parent_fabric_wwn) { + parent_host = + virNodeDeviceGetParentHostByFabricWWN(devs, def->name, + def->parent_fabric_wwn); + } else if (create == CREATE_DEVICE) { + /* Try to find a vport capable scsi_host when no parent supplied */ + parent_host = virNodeDeviceFindVportParentHost(devs); + } + + return parent_host; +} + + void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) { size_t i = 0; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 1634483..4bdccf9 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -272,23 +272,8 @@ int virNodeDeviceGetWWNs(virNodeDeviceDefPtr def, char **wwpn); int virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, - const char *dev_name, - const char *parent_name, - int *parent_host); - -int virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs, - const char *dev_name, - const char *parent_wwnn, - const char *parent_wwpn, - int *parent_host); - -int virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs, - const char *dev_name, - const char *parent_fabric_wwn, - int *parent_host); - -int virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs, - int *parent_host); + virNodeDeviceDefPtr def, + int create); void virNodeDeviceDefFree(virNodeDeviceDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ec3cab7..127addb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -701,10 +701,7 @@ virNodeDeviceDefParseNode; virNodeDeviceDefParseString; virNodeDeviceFindByName; virNodeDeviceFindBySysfsPath; -virNodeDeviceFindVportParentHost; virNodeDeviceGetParentHost; -virNodeDeviceGetParentHostByFabricWWN; -virNodeDeviceGetParentHostByWWNs; virNodeDeviceGetWWNs; virNodeDeviceHasCap; virNodeDeviceObjListExport; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 52f8058..d478cb1 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -574,8 +574,7 @@ nodeDeviceCreateXML(virConnectPtr conn, nodeDeviceLock(); - def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type); - if (def == NULL) + if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type))) goto cleanup; if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) @@ -584,30 +583,9 @@ nodeDeviceCreateXML(virConnectPtr conn, if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) goto cleanup; - if (def->parent) { - if (virNodeDeviceGetParentHost(&driver->devs, - def->name, - def->parent, - &parent_host) < 0) - goto cleanup; - } else if (def->parent_wwnn && def->parent_wwpn) { - if (virNodeDeviceGetParentHostByWWNs(&driver->devs, - def->name, - def->parent_wwnn, - def->parent_wwpn, - &parent_host) < 0) - goto cleanup; - } else if (def->parent_fabric_wwn) { - if (virNodeDeviceGetParentHostByFabricWWN(&driver->devs, - def->name, - def->parent_fabric_wwn, - &parent_host) < 0) - goto cleanup; - } else { - /* Try to find a vport capable scsi_host when no parent supplied */ - if (virNodeDeviceFindVportParentHost(&driver->devs, &parent_host) < 0) - goto cleanup; - } + if ((parent_host = virNodeDeviceGetParentHost(&driver->devs, def, + CREATE_DEVICE)) < 0) + goto cleanup; if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0) goto cleanup; @@ -635,7 +613,8 @@ nodeDeviceDestroy(virNodeDevicePtr dev) { int ret = -1; virNodeDeviceObjPtr obj = NULL; - char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; + virNodeDeviceDefPtr def; + char *wwnn = NULL, *wwpn = NULL; int parent_host = -1; nodeDeviceLock(); @@ -652,32 +631,26 @@ nodeDeviceDestroy(virNodeDevicePtr dev) if (virNodeDeviceGetWWNs(obj->def, &wwnn, &wwpn) < 0) goto cleanup; - - /* virNodeDeviceGetParentHost will cause the device object's lock to be - * taken, so we have to dup the parent's name and drop the lock - * before calling it. We don't need the reference to the object - * any more once we have the parent's name. */ - if (VIR_STRDUP(parent_name, obj->def->parent) < 0) { - virNodeDeviceObjUnlock(obj); - obj = NULL; - goto cleanup; - } + /* virNodeDeviceGetParentHost will cause the device object's lock + * to be taken, so grab the object def which will have the various + * fields used to search (name, parent, parent_wwnn, parent_wwpn, + * or parent_fabric_wwn) and drop the object lock. */ + def = obj->def; virNodeDeviceObjUnlock(obj); obj = NULL; - - if (virNodeDeviceGetParentHost(&driver->devs, dev->name, parent_name, - &parent_host) < 0) + if ((parent_host = virNodeDeviceGetParentHost(&driver->devs, def, + EXISTING_DEVICE)) < 0) goto cleanup; if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) goto cleanup; ret = 0; + cleanup: nodeDeviceUnlock(); if (obj) virNodeDeviceObjUnlock(obj); - VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn); return ret; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0a0fa8f..feae563 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5709,7 +5709,6 @@ testNodeDeviceCreateXML(virConnectPtr conn, testDriverPtr driver = conn->privateData; virNodeDeviceDefPtr def = NULL; char *wwnn = NULL, *wwpn = NULL; - int parent_host = -1; virNodeDevicePtr dev = NULL; virNodeDeviceDefPtr newdef = NULL; @@ -5720,15 +5719,16 @@ testNodeDeviceCreateXML(virConnectPtr conn, if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL))) goto cleanup; - /* We run these next two simply for validation - they are essentially - * 'validating' that the input XML either has a wwnn/wwpn or the - * virNodeDevCapSCSIHostParseXML generated a wwnn/wwpn and that the - * input XML has a parent host defined. */ + /* We run this simply for validation - it essentially validates that + * the input XML either has a wwnn/wwpn or virNodeDevCapSCSIHostParseXML + * generated a wwnn/wwpn */ if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0) goto cleanup; - if (virNodeDeviceGetParentHost(&driver->devs, def->name, - def->parent, &parent_host) < 0) + /* Unlike the "real" code we don't need the parent_host in order to + * call virVHBAManageVport, but still let's make sure the code finds + * something valid and no one messed up the mock environment. */ + if (virNodeDeviceGetParentHost(&driver->devs, def, CREATE_DEVICE) < 0) goto cleanup; /* In the real code, we'd call virVHBAManageVport followed by @@ -5757,7 +5757,6 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) testDriverPtr driver = dev->conn->privateData; virNodeDeviceObjPtr obj = NULL; char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; - int parent_host = -1; virObjectEventPtr event = NULL; testDriverLock(driver); @@ -5783,11 +5782,10 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) * any more once we have the parent's name. */ virNodeDeviceObjUnlock(obj); - /* We do this just for basic validation */ - if (virNodeDeviceGetParentHost(&driver->devs, - dev->name, - parent_name, - &parent_host) == -1) { + /* We do this just for basic validation, but also avoid finding a + * vport capable HBA if for some reason our vHBA doesn't exist */ + if (virNodeDeviceGetParentHost(&driver->devs, obj->def, + EXISTING_DEVICE) < 0) { obj = NULL; goto out; } -- 2.7.4

Add a test that allows providing the parent wwnn/wwpn in the input XML in order to create the vHBA. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- tests/fchosttest.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/fchosttest.c b/tests/fchosttest.c index 15cda75..d083104 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -53,6 +53,17 @@ static const char test8_xml[] = " </capability>" "</device>"; +/* virNodeDeviceCreateXML using "<parent wwnn='%s' wwpn='%s'/>" to find + * the vport capable HBA */ +static const char test9_xml[] = +"<device>" +" <parent wwnn='2000000012341234' wwpn='1000000012341234'/>" +" <capability type='scsi_host'>" +" <capability type='fc_host'>" +" </capability>" +" </capability>" +"</device>"; + /* Test virIsVHBACapable */ static int test1(const void *data ATTRIBUTE_UNUSED) @@ -282,6 +293,9 @@ mymain(void) if (virTestRun("manageVHBAByNodeDevice-no-parent", manageVHBAByNodeDevice, test8_xml) < 0) ret = -1; + if (virTestRun("manageVHBAByNodeDevice-parent-wwn", manageVHBAByNodeDevice, + test9_xml) < 0) + ret = -1; cleanup: VIR_FREE(fchost_prefix); -- 2.7.4

On Wed, Jan 25, 2017 at 03:27:33PM -0500, John Ferlan wrote: This is Vhba named function but it isn't only VHBA related code. With some modification it could be generic node device function to get a parent of a device specified by it's name. The only VHBA related code in this function is construction of the node device name. Even without this context we should not import anything from src/conf/*.h in the src/util/* code. NACK Pavel

On Wed, Jan 25, 2017 at 03:27:34PM -0500, John Ferlan wrote: ACK, this is unrelated to this series so you can push it as a fix for the code in src/conf/storage_conf.c.

On 02/17/2017 11:41 AM, Pavel Hrdina wrote: While I agree it's doesn't appear to be VHBA related since it's not looking at the sysfs file structure for the answer, but there is a reason for it. One could argue that virStoragePoolGetVhbaSCSIHostParent doesn't really belong in storage_conf.c either. IIRC it was placed there because of that awful matchFCHostToSCSIHost function (which theoretically could be moved too). There are other util functions that query a driver to get an answer (auth, closecallbacks), so that part isn't something new. It also doesn't "do" something _conf specific such as format/alter - it's just getting an answer from the node device driver. There's nothing storage_conf specific about it. It is VHBA related if you consider that it's a way to take a name ("scsi_hostN") that's already been verified as a VHBA by some other virVHBA* sysfs perusing API and return its parent. Since one doesn't get the answer of parent via the file system, one must ask the driver. I'll have to look at my local branches to recall whether or not the "next" stages will need this. The next stages being the ability to add/define a VHBA to a domain directly with the ultimate goal being to add all the LUNs from the VHBA to the domain automagically. I believe the reason I made this generic is because it would be "duplicitous" to have a domain_conf.c function that does the same thing... John

On Wed, Jan 25, 2017 at 03:27:35PM -0500, John Ferlan wrote: [...] It's better to name it virStorateIsSameHostnum. ACK Pavel

Use the new virNodeDeviceGetParentName instead. Modify the callers to build the node device scsi_host# name string in order to call the new function so that proper lookup occurs. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- Changes since v1: This is the replacement/followup for the previous patch 7 which rename the virStoragePoolGetVhbaSCSIHostParent. Instead the code will now use the new virNodeDeviceGetParentName function passing the scsi_hostN name as generated in each of the callers (what would have been done by the calling code in the v1 code). src/conf/storage_conf.c | 70 ++++++-------------------------------- src/conf/storage_conf.h | 5 --- src/libvirt_private.syms | 1 - src/storage/storage_backend_scsi.c | 23 +++++++++---- 4 files changed, 27 insertions(+), 72 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index fff7285..80ec438 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -35,6 +35,7 @@ #include "virerror.h" #include "datatypes.h" +#include "node_device_conf.h" #include "storage_conf.h" #include "virstoragefile.h" @@ -2275,64 +2276,6 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, return ret; } -/* - * virStoragePoolGetVhbaSCSIHostParent: - * - * Using the Node Device Driver, find the host# name found via wwnn/wwpn - * lookup in the fc_host sysfs tree (e.g. virGetFCHostNameByWWN) to get - * the parent 'scsi_host#'. - * - * @conn: Connection pointer (must be non-NULL on entry) - * @name: Pointer a string from a virGetFCHostNameByWWN (e.g., "host#") - * - * Returns a "scsi_host#" string of the parent of the vHBA - */ -char * -virStoragePoolGetVhbaSCSIHostParent(virConnectPtr conn, - const char *name) -{ - char *nodedev_name = NULL; - virNodeDevicePtr device = NULL; - char *xml = NULL; - virNodeDeviceDefPtr def = NULL; - char *vhba_parent = NULL; - - VIR_DEBUG("conn=%p, name=%s", conn, name); - - /* We get passed "host#" from the return from virGetFCHostNameByWWN, - * so we need to adjust that to what the nodedev lookup expects - */ - if (virAsprintf(&nodedev_name, "scsi_%s", name) < 0) - goto cleanup; - - /* Compare the scsi_host for the name with the provided parent - * if not the same, then fail - */ - if (!(device = virNodeDeviceLookupByName(conn, nodedev_name))) { - virReportError(VIR_ERR_XML_ERROR, - _("Cannot find '%s' in node device database"), - nodedev_name); - goto cleanup; - } - - if (!(xml = virNodeDeviceGetXMLDesc(device, 0))) - goto cleanup; - - if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL))) - goto cleanup; - - /* The caller checks whether the returned value is NULL or not - * before continuing - */ - ignore_value(VIR_STRDUP(vhba_parent, def->parent)); - - cleanup: - VIR_FREE(nodedev_name); - virNodeDeviceDefFree(def); - VIR_FREE(xml); - virObjectUnref(device); - return vhba_parent; -} static int getSCSIHostNumber(virStoragePoolSourceAdapter adapter, @@ -2383,6 +2326,7 @@ matchFCHostToSCSIHost(virConnectPtr conn, unsigned int scsi_hostnum) { char *name = NULL; + char *scsi_host_name = NULL; char *parent_name = NULL; unsigned int fc_hostnum; @@ -2415,8 +2359,13 @@ matchFCHostToSCSIHost(virConnectPtr conn, * have a match. */ if (conn && !fc_adapter.data.fchost.parent) { - parent_name = virStoragePoolGetVhbaSCSIHostParent(conn, name); - if (parent_name) { + if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) { + VIR_FREE(name); + return false; + } + if ((parent_name = virNodeDeviceGetParentName(conn, + scsi_host_name))) { + VIR_FREE(scsi_host_name); if (virGetSCSIHostNumber(parent_name, &fc_hostnum) == 0 && scsi_hostnum == fc_hostnum) { VIR_FREE(parent_name); @@ -2428,6 +2377,7 @@ matchFCHostToSCSIHost(virConnectPtr conn, /* Throw away the error and fall through */ virResetLastError(); VIR_DEBUG("Could not determine parent vHBA"); + VIR_FREE(scsi_host_name); } } VIR_FREE(name); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index e952f5f..1723afc 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -30,7 +30,6 @@ # include "virbitmap.h" # include "virthread.h" # include "device_conf.h" -# include "node_device_conf.h" # include "object_event.h" # include <libxml/tree.h> @@ -418,10 +417,6 @@ int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, unsigned int check_active); -char *virStoragePoolGetVhbaSCSIHostParent(virConnectPtr conn, - const char *name) - ATTRIBUTE_NONNULL(1); - int virStoragePoolSourceFindDuplicate(virConnectPtr conn, virStoragePoolObjListPtr pools, virStoragePoolDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9ad0b0a..6806fc7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -889,7 +889,6 @@ virStoragePoolFormatDiskTypeToString; virStoragePoolFormatFileSystemNetTypeToString; virStoragePoolFormatFileSystemTypeToString; virStoragePoolFormatLogicalTypeToString; -virStoragePoolGetVhbaSCSIHostParent; virStoragePoolLoadAllConfigs; virStoragePoolLoadAllState; virStoragePoolObjAssignDef; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index e037a93..106394d 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -36,6 +36,7 @@ #include "virstring.h" #include "virvhba.h" #include "storage_util.h" +#include "node_device_conf.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -214,10 +215,11 @@ getAdapterName(virStoragePoolSourceAdapter adapter) * sysfs tree to get the parent 'scsi_host#' to ensure it matches. */ static bool -checkVhbaSCSIHostParent(virConnectPtr conn, - const char *name, - const char *parent_name) +checkParent(virConnectPtr conn, + const char *name, + const char *parent_name) { + char *scsi_host_name = NULL; char *vhba_parent = NULL; bool retval = false; @@ -227,7 +229,10 @@ checkVhbaSCSIHostParent(virConnectPtr conn, if (!conn) return true; - if (!(vhba_parent = virStoragePoolGetVhbaSCSIHostParent(conn, name))) + if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) + goto cleanup; + + if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name))) goto cleanup; if (STRNEQ(parent_name, vhba_parent)) { @@ -242,6 +247,7 @@ checkVhbaSCSIHostParent(virConnectPtr conn, cleanup: VIR_FREE(vhba_parent); + VIR_FREE(scsi_host_name); return retval; } @@ -276,7 +282,7 @@ createVport(virConnectPtr conn, * retrieved has the same parent */ if (adapter->data.fchost.parent && - checkVhbaSCSIHostParent(conn, name, adapter->data.fchost.parent)) + checkParent(conn, name, adapter->data.fchost.parent)) ret = 0; goto cleanup; @@ -383,6 +389,7 @@ deleteVport(virConnectPtr conn, { unsigned int parent_host; char *name = NULL; + char *scsi_host_name = NULL; char *vhba_parent = NULL; int ret = -1; @@ -416,7 +423,10 @@ deleteVport(virConnectPtr conn, if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) goto cleanup; } else { - if (!(vhba_parent = virStoragePoolGetVhbaSCSIHostParent(conn, name))) + if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) + goto cleanup; + + if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name))) goto cleanup; if (virGetSCSIHostNumber(vhba_parent, &parent_host) < 0) @@ -431,6 +441,7 @@ deleteVport(virConnectPtr conn, cleanup: VIR_FREE(name); VIR_FREE(vhba_parent); + VIR_FREE(scsi_host_name); return ret; } -- 2.9.3