Alter "test-scsi-host-vport" to be "scsi_host1" to match the real environment. This is the vport capable HBA - IOW the NPIV device. Add more fields to scsi_host1 as well. Alter the XML being used by the objecttest to create a vHBA in order to match the scsi_host1 parent name and to use validateable wwnn/wwpn. This will allow for realistic testing. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/test/test_driver.c | 9 +++++++-- tests/objecteventtest.c | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6820298..18428a8 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -487,15 +487,20 @@ static const char *defaultConnXML = " </capability>" "</device>" "<device>" -" <name>test-scsi-host-vport</name>" +" <name>scsi_host1</name>" " <parent>computer</parent>" " <capability type='scsi_host'>" " <host>1</host>" +" <unique_id>0</unique_id>" " <capability type='fc_host'>" " <wwnn>2000000012341234</wwnn>" " <wwpn>1000000012341234</wwpn>" +" <fabric_wwn>2000000043214321</fabric_wwn>" +" </capability>" +" <capability type='vport_ops'>" +" <max_vports>127</max_vports>" +" <vports>0</vports>" " </capability>" -" <capability type='vport_ops'/>" " </capability>" "</device>" "</node>"; diff --git a/tests/objecteventtest.c b/tests/objecteventtest.c index d25a9e2..5df6ff9 100644 --- a/tests/objecteventtest.c +++ b/tests/objecteventtest.c @@ -63,11 +63,11 @@ static const char storagePoolDef[] = static const char nodeDeviceDef[] = "<device>\n" -" <parent>test-scsi-host-vport</parent>\n" +" <parent>scsi_host1</parent>\n" " <capability type='scsi_host'>\n" " <capability type='fc_host'>\n" -" <wwpn>1111222233334444</wwpn>\n" -" <wwnn>5555666677778888</wwnn>\n" +" <wwpn>1000000023452345</wwpn>\n" +" <wwnn>2000000023452345</wwnn>\n" " </capability>\n" " </capability>\n" "</device>\n"; -- 2.7.4

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

Rather than inline the dummy creation of a vHBA to add to the node devices - create a helper to do that work. Also just tidy up a couple of things while at it... Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/test/test_driver.c | 82 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 30 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 4dd2956..5e628b5 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5622,39 +5622,18 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames) return ret; } -static virNodeDevicePtr -testNodeDeviceCreateXML(virConnectPtr conn, - const char *xmlDesc, - unsigned int flags) + +static int +testNodeDeviceMockCreateVport(virConnectPtr conn, + virNodeDeviceDefPtr def, + const char *wwpn) { + int ret = -1; testDriverPtr driver = conn->privateData; - virNodeDeviceDefPtr def = NULL; - virNodeDeviceObjPtr obj = NULL; - char *wwnn = NULL, *wwpn = NULL; - int parent_host = -1; - virNodeDevicePtr dev = NULL; virNodeDevCapsDefPtr caps; + virNodeDeviceObjPtr obj = NULL; virObjectEventPtr event = NULL; - virCheckFlags(0, NULL); - - testDriverLock(driver); - - def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL); - if (def == NULL) - goto cleanup; - - /* We run these next two simply for validation */ - if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) - goto cleanup; - - if (virNodeDeviceGetParentHost(&driver->devs, - def->name, - def->parent, - &parent_host) == -1) { - goto cleanup; - } - /* 'name' is supposed to be filled in by the node device backend, which * we don't have. Use WWPN instead. */ VIR_FREE(def->name); @@ -5673,7 +5652,6 @@ testNodeDeviceCreateXML(virConnectPtr conn, caps = caps->next; } - if (!(obj = virNodeDeviceAssignDef(&driver->devs, def))) goto cleanup; virNodeDeviceObjUnlock(obj); @@ -5681,13 +5659,57 @@ testNodeDeviceCreateXML(virConnectPtr conn, event = virNodeDeviceEventLifecycleNew(def->name, VIR_NODE_DEVICE_EVENT_CREATED, 0); + testObjectEventQueue(driver, event); + + ret = 0; + + cleanup: + return ret; +} + + +static virNodeDevicePtr +testNodeDeviceCreateXML(virConnectPtr conn, + const char *xmlDesc, + unsigned int flags) +{ + testDriverPtr driver = conn->privateData; + virNodeDeviceDefPtr def = NULL; + char *wwnn = NULL, *wwpn = NULL; + int parent_host = -1; + virNodeDevicePtr dev = NULL; + + virCheckFlags(0, NULL); + + testDriverLock(driver); + + 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. */ + if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0) + goto cleanup; + + if (virNodeDeviceGetParentHost(&driver->devs, def->name, + def->parent, &parent_host) < 0) + goto cleanup; + + /* 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; dev = virGetNodeDevice(conn, def->name); def = NULL; cleanup: testDriverUnlock(driver); virNodeDeviceDefFree(def); - testObjectEventQueue(driver, event); VIR_FREE(wwnn); VIR_FREE(wwpn); return dev; -- 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

Commit id '666bee3' made fabric_name optional; however, if fabric name was present, then a leak would occur. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- tests/fchosttest.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/fchosttest.c b/tests/fchosttest.c index bb35b88..0b4a8f2 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -159,6 +159,7 @@ test6(const void *data ATTRIBUTE_UNUSED) const char *expect_wwpn = "2102001b32a9da4e"; char *wwnn = NULL; char *wwpn = NULL; + char *fabric_wwn = NULL; int ret = -1; if (!(wwnn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM_NO_FAB, @@ -169,8 +170,9 @@ test6(const void *data ATTRIBUTE_UNUSED) "port_name"))) goto cleanup; - if (virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM_NO_FAB, - "fabric_name")) + if ((fabric_wwn = virReadFCHost(TEST_FC_HOST_PREFIX, + TEST_FC_HOST_NUM_NO_FAB, + "fabric_name"))) goto cleanup; if (STRNEQ(expect_wwnn, wwnn) || @@ -181,6 +183,7 @@ test6(const void *data ATTRIBUTE_UNUSED) cleanup: VIR_FREE(wwnn); VIR_FREE(wwpn); + VIR_FREE(fabric_wwn); return ret; } -- 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

On 02/17/2017 11:00 AM, Pavel Hrdina wrote: While I understand your sentiment, the series was already at 15 patches. Changing the names in a separate patch didn't feel "right" since I was creating a new util module... I suppose an argument could also be made that every changed function name should have it's own separate patch. That was just too tedious and way too much work for the same result. Patch 9 has a similar move/rename going on... The changes in patch 3 were really innocuous - I think it was combining an "if" construct, fixing the == -1, and adding more comments [...] This is the same comment as the former virReadFCHost. I'm open to suggestions as for a rewording... ;-) Should the reworded comment be a separate commit... [...] No it's not necessary, it's a shorthand explanation for what's going on... The "event" posted by the kernel is a udev event which is recognized by udevEventHandleCallback. That in turn calls udevAddOneDevice which eventually calls virNodeDeviceEventLifecycleNew to create the libvirt event that applications can key off of. I can adjust or drop the comment - it doesn't really matter, but for anyone that stumbles headfirst into this code - I believe it provides an explanation regarding what's about to take place. There's then enough bread crumbs for that poor hapless soul to attempt to figure out how this stuff works... [...] Doesn't matter to me - I can make it two lines... Guess I'm following what I think I've seen in most header files. In source files we also try to have one argument per line; whereas, for header files that's not always followed. Doing as you suggest just increases the number of lines - I really don't mind either way, but I do believe the majority of the header files follow this construct. I'm not sure I see value in the multiline header construct, but I'm not opposed to it... Of course the same comment will be made in patch 9 I suppose... John

Move the function and rename to simply virVHBAGetParent Since I'm changing related code, rather than have a Vhba named function in storage_backend_scsi rename it to simply checkParent as it's already within the scope of checking VHBA related features. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/storage_conf.c | 61 +---------------------------------- src/conf/storage_conf.h | 5 --- src/libvirt_private.syms | 2 +- src/storage/storage_backend_scsi.c | 12 +++---- src/util/virvhba.c | 65 ++++++++++++++++++++++++++++++++++++++ src/util/virvhba.h | 4 +++ 6 files changed, 77 insertions(+), 72 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index f8b5228..5e13bbf 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2264,64 +2264,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, @@ -2404,8 +2346,7 @@ matchFCHostToSCSIHost(virConnectPtr conn, * have a match. */ if (conn && !fc_adapter.data.fchost.parent) { - parent_name = virStoragePoolGetVhbaSCSIHostParent(conn, name); - if (parent_name) { + if ((parent_name = virVHBAGetParent(conn, name))) { if (virGetSCSIHostNumber(parent_name, &fc_hostnum) == 0 && scsi_hostnum == fc_hostnum) { VIR_FREE(parent_name); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index b35471d..426e8b8 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> @@ -417,10 +416,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 429c133..b58742d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -887,7 +887,6 @@ virStoragePoolFormatDiskTypeToString; virStoragePoolFormatFileSystemNetTypeToString; virStoragePoolFormatFileSystemTypeToString; virStoragePoolFormatLogicalTypeToString; -virStoragePoolGetVhbaSCSIHostParent; virStoragePoolLoadAllConfigs; virStoragePoolLoadAllState; virStoragePoolObjAssignDef; @@ -2744,6 +2743,7 @@ virVHBAFindVportHost; virVHBAGetConfig; virVHBAGetHostByFabricWWN; virVHBAGetHostByWWN; +virVHBAGetParent; virVHBAIsVportCapable; virVHBAManageVport; virVHBAPathExists; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index e037a93..2da15dd 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -214,9 +214,9 @@ 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 *vhba_parent = NULL; bool retval = false; @@ -227,7 +227,7 @@ checkVhbaSCSIHostParent(virConnectPtr conn, if (!conn) return true; - if (!(vhba_parent = virStoragePoolGetVhbaSCSIHostParent(conn, name))) + if (!(vhba_parent = virVHBAGetParent(conn, name))) goto cleanup; if (STRNEQ(parent_name, vhba_parent)) { @@ -276,7 +276,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; @@ -416,7 +416,7 @@ deleteVport(virConnectPtr conn, if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) goto cleanup; } else { - if (!(vhba_parent = virStoragePoolGetVhbaSCSIHostParent(conn, name))) + if (!(vhba_parent = virVHBAGetParent(conn, name))) goto cleanup; if (virGetSCSIHostNumber(vhba_parent, &parent_host) < 0) diff --git a/src/util/virvhba.c b/src/util/virvhba.c index e5637d7..8c4da3a 100644 --- a/src/util/virvhba.c +++ b/src/util/virvhba.c @@ -18,6 +18,8 @@ #include <config.h> +#include "node_device_conf.h" + #include "viralloc.h" #include "virerror.h" #include "virfile.h" @@ -530,3 +532,66 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix ATTRIBUTE_UNUSED, } #endif /* __linux__ */ + + +/* The following will be more generic - using the Node Device driver in + * order to perform the lookup rather than looking through the file system + * in order to find the answer */ +/* + * virVHBAGetParent: + * + * 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 * +virVHBAGetParent(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; +} diff --git a/src/util/virvhba.h b/src/util/virvhba.h index e338f96..b8d73ab 100644 --- a/src/util/virvhba.h +++ b/src/util/virvhba.h @@ -52,4 +52,8 @@ char *virVHBAGetHostByFabricWWN(const char *sysfs_prefix, const char *fabric_wwn) ATTRIBUTE_NONNULL(2); +char *virVHBAGetParent(virConnectPtr conn, + const char *name) + ATTRIBUTE_NONNULL(1); + #endif /* __VIR_VBHA_H__ */ -- 2.7.4

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

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

Create a virscsihost.c and place the functions there. That removes the last #ifdef __linux__ from virutil.c. Take the opporunity to also change the function names and in one case the parameters slightly Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/storage_conf.c | 34 ++-- src/libvirt_private.syms | 10 +- src/node_device/node_device_linux_sysfs.c | 5 +- src/storage/storage_backend_scsi.c | 15 +- src/util/virscsihost.c | 297 ++++++++++++++++++++++++++++++ src/util/virscsihost.h | 40 ++++ src/util/virutil.c | 269 --------------------------- src/util/virutil.h | 20 -- tests/scsihosttest.c | 16 +- 11 files changed, 386 insertions(+), 322 deletions(-) create mode 100644 src/util/virscsihost.c create mode 100644 src/util/virscsihost.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 1331093..e9a3569 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -240,6 +240,7 @@ src/util/virqemu.c src/util/virrandom.c src/util/virrotatingfile.c src/util/virscsi.c +src/util/virscsihost.c src/util/virscsivhost.c src/util/virsecret.c src/util/virsexpr.c diff --git a/src/Makefile.am b/src/Makefile.am index 0fec45b..5db0ea9 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -164,6 +164,7 @@ UTIL_SOURCES = \ util/virrandom.h util/virrandom.c \ util/virrotatingfile.h util/virrotatingfile.c \ util/virscsi.c util/virscsi.h \ + util/virscsihost.c util/virscsihost.h \ util/virscsivhost.c util/virscsivhost.h \ util/virseclabel.c util/virseclabel.h \ util/virsecret.c util/virsecret.h \ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5e13bbf..8289ccc 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -43,6 +43,7 @@ #include "virbuffer.h" #include "viralloc.h" #include "virfile.h" +#include "virscsihost.h" #include "virstring.h" #include "virlog.h" #include "virvhba.h" @@ -2277,16 +2278,16 @@ getSCSIHostNumber(virStoragePoolSourceAdapter adapter, virPCIDeviceAddress addr = adapter.data.scsi_host.parentaddr; unsigned int unique_id = adapter.data.scsi_host.unique_id; - if (!(name = virGetSCSIHostNameByParentaddr(addr.domain, + if (!(name = virSCSIHostGetNameByParentaddr(addr.domain, addr.bus, addr.slot, addr.function, unique_id))) goto cleanup; - if (virGetSCSIHostNumber(name, &num) < 0) + if (virSCSIHostGetNumber(name, &num) < 0) goto cleanup; } else { - if (virGetSCSIHostNumber(adapter.data.scsi_host.name, &num) < 0) + if (virSCSIHostGetNumber(adapter.data.scsi_host.name, &num) < 0) goto cleanup; } @@ -2298,6 +2299,20 @@ getSCSIHostNumber(virStoragePoolSourceAdapter adapter, return ret; } + +static bool +isSameHostnum(const char *name, unsigned int scsi_hostnum) +{ + unsigned int fc_hostnum; + + if (virSCSIHostGetNumber(name, &fc_hostnum) == 0 && + scsi_hostnum == fc_hostnum) + return true; + + return false; +} + + /* * matchFCHostToSCSIHost: * @@ -2315,14 +2330,12 @@ matchFCHostToSCSIHost(virConnectPtr conn, { char *name = NULL; char *parent_name = NULL; - unsigned int fc_hostnum; /* If we have a parent defined, get its hostnum, and compare to the * scsi_hostnum. If they are the same, then we have a match */ if (fc_adapter.data.fchost.parent && - virGetSCSIHostNumber(fc_adapter.data.fchost.parent, &fc_hostnum) == 0 && - scsi_hostnum == fc_hostnum) + isSameHostnum(fc_adapter.data.fchost.parent, scsi_hostnum)) return true; /* If we find an fc_adapter name, then either libvirt created a vHBA @@ -2334,11 +2347,11 @@ matchFCHostToSCSIHost(virConnectPtr conn, /* Get the scsi_hostN for the vHBA in order to see if it * matches our scsi_hostnum */ - if (virGetSCSIHostNumber(name, &fc_hostnum) == 0 && - scsi_hostnum == fc_hostnum) { + if (isSameHostnum(name, scsi_hostnum)) { VIR_FREE(name); return true; } + VIR_FREE(name); /* We weren't provided a parent, so we have to query the node * device driver in order to ascertain the parent of the vHBA. @@ -2347,10 +2360,8 @@ matchFCHostToSCSIHost(virConnectPtr conn, */ if (conn && !fc_adapter.data.fchost.parent) { if ((parent_name = virVHBAGetParent(conn, name))) { - if (virGetSCSIHostNumber(parent_name, &fc_hostnum) == 0 && - scsi_hostnum == fc_hostnum) { + if (isSameHostnum(parent_name, scsi_hostnum)) { VIR_FREE(parent_name); - VIR_FREE(name); return true; } VIR_FREE(parent_name); @@ -2360,7 +2371,6 @@ matchFCHostToSCSIHost(virConnectPtr conn, VIR_DEBUG("Could not determine parent vHBA"); } } - VIR_FREE(name); } /* NB: Lack of a name means that this vHBA hasn't yet been created, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b58742d..ec3cab7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2349,6 +2349,12 @@ virSCSIDeviceNew; virSCSIDeviceSetUsedBy; +# util/virscsihost.h +virSCSIHostFindByPCI; +virSCSIHostGetNameByParentaddr; +virSCSIHostGetNumber; +virSCSIHostGetUniqueId; + # util/virscsivhost.h virSCSIVHostDeviceFileIterate; virSCSIVHostDeviceFree; @@ -2674,7 +2680,6 @@ virUSBDeviceSetUsedBy; virDoubleToStr; virEnumFromString; virEnumToString; -virFindSCSIHostByPCI; virFormatIntDecimal; virGetDeviceID; virGetDeviceUnprivSGIO; @@ -2686,8 +2691,6 @@ virGetGroupName; virGetHostname; virGetHostnameQuiet; virGetListenFDs; -virGetSCSIHostNameByParentaddr; -virGetSCSIHostNumber; virGetSelfLastChanged; virGetSystemPageSize; virGetSystemPageSizeKB; @@ -2711,7 +2714,6 @@ virParseNumber; virParseOwnershipIds; virParseVersionString; virPipeReadUntilEOF; -virReadSCSIUniqueId; virScaleInteger; virSetBlocking; virSetCloseExec; diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 1c72b07..8ac8bf6 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -33,6 +33,7 @@ #include "viralloc.h" #include "virlog.h" #include "virfile.h" +#include "virscsihost.h" #include "virstring.h" #include "virvhba.h" @@ -48,8 +49,8 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d) char *tmp = NULL; int ret = -1; - if (virReadSCSIUniqueId(NULL, d->scsi_host.host, - &d->scsi_host.unique_id) < 0) { + if ((d->scsi_host.unique_id = + virSCSIHostGetUniqueId(NULL, d->scsi_host.host)) < 0) { VIR_DEBUG("Failed to read unique_id for host%d", d->scsi_host.host); d->scsi_host.unique_id = -1; } diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 2da15dd..da3b847 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -33,6 +33,7 @@ #include "virlog.h" #include "virfile.h" #include "vircommand.h" +#include "virscsihost.h" #include "virstring.h" #include "virvhba.h" #include "storage_util.h" @@ -159,7 +160,7 @@ virStoragePoolFCRefreshThread(void *opaque) pool->def->allocation = pool->def->capacity = pool->def->available = 0; if (virStoragePoolObjIsActive(pool) && - virGetSCSIHostNumber(fchost_name, &host) == 0 && + virSCSIHostGetNumber(fchost_name, &host) == 0 && virStorageBackendSCSITriggerRescan(host) == 0) { virStoragePoolObjClearVols(pool); found = virStorageBackendSCSIFindLUs(pool, host); @@ -184,7 +185,7 @@ getAdapterName(virStoragePoolSourceAdapter adapter) virPCIDeviceAddress addr = adapter.data.scsi_host.parentaddr; unsigned int unique_id = adapter.data.scsi_host.unique_id; - if (!(name = virGetSCSIHostNameByParentaddr(addr.domain, + if (!(name = virSCSIHostGetNameByParentaddr(addr.domain, addr.bus, addr.slot, addr.function, @@ -312,7 +313,7 @@ createVport(virConnectPtr conn, skip_capable_check = true; } - if (virGetSCSIHostNumber(parent_hoststr, &parent_host) < 0) + if (virSCSIHostGetNumber(parent_hoststr, &parent_host) < 0) goto cleanup; /* NOTE: @@ -413,13 +414,13 @@ deleteVport(virConnectPtr conn, * the parent scsi_host which we did not save at startup time */ if (adapter.data.fchost.parent) { - if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) + if (virSCSIHostGetNumber(adapter.data.fchost.parent, &parent_host) < 0) goto cleanup; } else { if (!(vhba_parent = virVHBAGetParent(conn, name))) goto cleanup; - if (virGetSCSIHostNumber(vhba_parent, &parent_host) < 0) + if (virSCSIHostGetNumber(vhba_parent, &parent_host) < 0) goto cleanup; } @@ -460,7 +461,7 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, } } - if (virGetSCSIHostNumber(name, &host) < 0) + if (virSCSIHostGetNumber(name, &host) < 0) goto cleanup; if (virAsprintf(&path, "%s/host%d", @@ -489,7 +490,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (!(name = getAdapterName(pool->def->source.adapter))) return -1; - if (virGetSCSIHostNumber(name, &host) < 0) + if (virSCSIHostGetNumber(name, &host) < 0) goto out; VIR_DEBUG("Scanning host%u", host); diff --git a/src/util/virscsihost.c b/src/util/virscsihost.c new file mode 100644 index 0000000..eea0474 --- /dev/null +++ b/src/util/virscsihost.c @@ -0,0 +1,297 @@ +/* + * virscsihost.c: Generic scsi_host 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 <dirent.h> + +#include "viralloc.h" +#include "virerror.h" +#include "virfile.h" +#include "virlog.h" +#include "virscsihost.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("util.scsi_host"); + +#ifdef __linux__ + +# define SYSFS_SCSI_HOST_PATH "/sys/class/scsi_host" + +/* virSCSIHostGetUniqueId: + * @sysfs_prefix: "scsi_host" sysfs path, defaults to SYSFS_SCSI_HOST_PATH + * @host: Host number, E.g. 5 of "scsi_host/host5" + * + * Read the value of the "scsi_host" unique_id file. + * + * Returns the value on success or -1 on failure. + */ +int +virSCSIHostGetUniqueId(const char *sysfs_prefix, + int host) +{ + char *sysfs_path = NULL; + char *p = NULL; + int ret = -1; + char *buf = NULL; + int unique_id; + + if (virAsprintf(&sysfs_path, "%s/host%d/unique_id", + sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH, + host) < 0) + return -1; + + if (virFileReadAll(sysfs_path, 1024, &buf) < 0) + goto cleanup; + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + + if (virStrToLong_i(buf, NULL, 10, &unique_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to parse unique_id: %s"), buf); + + goto cleanup; + } + + ret = unique_id; + + cleanup: + VIR_FREE(sysfs_path); + VIR_FREE(buf); + return ret; +} + + +/* virSCSIHostFindByPCI: + * @sysfs_prefix: "scsi_host" sysfs path, defaults to SYSFS_SCSI_HOST_PATH + * @parentaddr: string of the PCI address "scsi_host" device to be found + * @unique_id: unique_id value of the to be found "scsi_host" device + * @result: Return the host# of the matching "scsi_host" device + * + * Iterate over the SYSFS_SCSI_HOST_PATH entries looking for a matching + * PCI Address in the expected format (dddd:bb:ss.f, where 'dddd' is the + * 'domain' value, 'bb' is the 'bus' value, 'ss' is the 'slot' value, and + * 'f' is the 'function' value from the PCI address) with a unique_id file + * entry having the value expected. Unlike virReadSCSIUniqueId() we don't + * have a host number yet and that's what we're looking for. + * + * Returns the host name of the "scsi_host" which must be freed by the caller, + * or NULL on failure + */ +char * +virSCSIHostFindByPCI(const char *sysfs_prefix, + const char *parentaddr, + unsigned int unique_id) +{ + const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH; + struct dirent *entry = NULL; + DIR *dir = NULL; + char *host_link = NULL; + char *host_path = NULL; + char *p = NULL; + char *ret = NULL; + char *buf = NULL; + char *unique_path = NULL; + unsigned int read_unique_id; + + if (virDirOpen(&dir, prefix) < 0) + return NULL; + + while (virDirRead(dir, &entry, prefix) > 0) { + if (!virFileIsLink(entry->d_name)) + continue; + + if (virAsprintf(&host_link, "%s/%s", prefix, entry->d_name) < 0) + goto cleanup; + + if (virFileResolveLink(host_link, &host_path) < 0) + goto cleanup; + + if (!strstr(host_path, parentaddr)) { + VIR_FREE(host_link); + VIR_FREE(host_path); + continue; + } + VIR_FREE(host_link); + VIR_FREE(host_path); + + if (virAsprintf(&unique_path, "%s/%s/unique_id", prefix, + entry->d_name) < 0) + goto cleanup; + + if (!virFileExists(unique_path)) { + VIR_FREE(unique_path); + continue; + } + + if (virFileReadAll(unique_path, 1024, &buf) < 0) + goto cleanup; + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + + if (virStrToLong_ui(buf, NULL, 10, &read_unique_id) < 0) + goto cleanup; + + VIR_FREE(buf); + + if (read_unique_id != unique_id) { + VIR_FREE(unique_path); + continue; + } + + ignore_value(VIR_STRDUP(ret, entry->d_name)); + break; + } + + cleanup: + VIR_DIR_CLOSE(dir); + VIR_FREE(unique_path); + VIR_FREE(host_link); + VIR_FREE(host_path); + VIR_FREE(buf); + return ret; +} + + +/* virSCSIHostGetNumber: + * @adapter_name: Name of the host adapter + * @result: Return the entry value as unsigned int + * + * Convert the various forms of scsi_host names into the numeric + * host# value that can be used in order to scan sysfs looking for + * the specific host. + * + * Names can be either "scsi_host#" or just "host#", where + * "host#" is the back-compat format, but both equate to + * the same source adapter. First check if both pool and def + * are using same format (easier) - if so, then compare + * + * Returns 0 on success, and @result has the host number. + * Otherwise returns -1. + */ +int +virSCSIHostGetNumber(const char *adapter_name, + unsigned int *result) +{ + /* Specifying adapter like 'host5' is still supported for + * back-compat reason. + */ + if (STRPREFIX(adapter_name, "scsi_host")) { + adapter_name += strlen("scsi_host"); + } else if (STRPREFIX(adapter_name, "fc_host")) { + adapter_name += strlen("fc_host"); + } else if (STRPREFIX(adapter_name, "host")) { + adapter_name += strlen("host"); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid adapter name '%s' for SCSI pool"), + adapter_name); + return -1; + } + + if (virStrToLong_ui(adapter_name, NULL, 10, result) == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid adapter name '%s' for SCSI pool"), + adapter_name); + return -1; + } + + return 0; +} + +/* virSCSIHostGetNameByParentaddr: + * @domain: The domain from the scsi_host parentaddr + * @bus: The bus from the scsi_host parentaddr + * @slot: The slot from the scsi_host parentaddr + * @function: The function from the scsi_host parentaddr + * @unique_id: The unique id value for parentaddr + * + * Generate a parentaddr and find the scsi_host host# for + * the provided parentaddr PCI address fields. + * + * Returns the "host#" string which must be free'd by + * the caller or NULL on error + */ +char * +virSCSIHostGetNameByParentaddr(unsigned int domain, + unsigned int bus, + unsigned int slot, + unsigned int function, + unsigned int unique_id) +{ + char *name = NULL; + char *parentaddr = NULL; + + if (virAsprintf(&parentaddr, "%04x:%02x:%02x.%01x", + domain, bus, slot, function) < 0) + goto cleanup; + if (!(name = virSCSIHostFindByPCI(NULL, parentaddr, unique_id))) { + virReportError(VIR_ERR_XML_ERROR, + _("Failed to find scsi_host using PCI '%s' " + "and unique_id='%u'"), + parentaddr, unique_id); + goto cleanup; + } + + cleanup: + VIR_FREE(parentaddr); + return name; +} + +#else + +int +virSCSIHostGetUniqueId(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + +char * +virSCSIHostFindByPCI(const char *sysfs_prefix ATTRIBUTE_UNUSED, + const char *parentaddr ATTRIBUTE_UNUSED, + unsigned int unique_id ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + +int +virSCSIHostGetNumber(const char *adapter_name ATTRIBUTE_UNUSED, + unsigned int *result ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + +char * +virSCSIHostGetNameByParentaddr(unsigned int domain ATTRIBUTE_UNUSED, + unsigned int bus ATTRIBUTE_UNUSED, + unsigned int slot ATTRIBUTE_UNUSED, + unsigned int function ATTRIBUTE_UNUSED, + unsigned int unique_id ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + +#endif /* __linux__ */ diff --git a/src/util/virscsihost.h b/src/util/virscsihost.h new file mode 100644 index 0000000..c35ccb9 --- /dev/null +++ b/src/util/virscsihost.h @@ -0,0 +1,40 @@ +/* + * virscsihost.h: Generic scsi_host 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_SCSI_HOST_H__ +# define __VIR_SCSI_HOST_H__ + +# include "internal.h" + +int virSCSIHostGetUniqueId(const char *sysfs_prefix, int host); + +char *virSCSIHostFindByPCI(const char *sysfs_prefix, + const char *parentaddr, + unsigned int unique_id); + +int virSCSIHostGetNumber(const char *adapter_name, + unsigned int *result) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +char *virSCSIHostGetNameByParentaddr(unsigned int domain, + unsigned int bus, + unsigned int slot, + unsigned int function, + unsigned int unique_id); + +#endif /* __VIR_SCSI_HOST_H__ */ diff --git a/src/util/virutil.c b/src/util/virutil.c index 51a1394..6fb70db 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -27,7 +27,6 @@ #include <config.h> #include <stdlib.h> -#include <dirent.h> #include <stdio.h> #include <stdarg.h> #include <unistd.h> @@ -1776,274 +1775,6 @@ virGetDeviceUnprivSGIO(const char *path, return ret; } -#ifdef __linux__ - -# define SYSFS_SCSI_HOST_PATH "/sys/class/scsi_host" - -/* virReadSCSIUniqueId: - * @sysfs_prefix: "scsi_host" sysfs path, defaults to SYSFS_SCSI_HOST_PATH - * @host: Host number, E.g. 5 of "scsi_host/host5" - * @result: Return the entry value as an unsigned int - * - * Read the value of the "scsi_host" unique_id file. - * - * Returns 0 on success, and @result is filled with the unique_id value - * Otherwise returns -1 - */ -int -virReadSCSIUniqueId(const char *sysfs_prefix, - int host, - int *result) -{ - char *sysfs_path = NULL; - char *p = NULL; - int ret = -1; - char *buf = NULL; - int unique_id; - - if (virAsprintf(&sysfs_path, "%s/host%d/unique_id", - sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH, - host) < 0) - goto cleanup; - - if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; - - if ((p = strchr(buf, '\n'))) - *p = '\0'; - - if (virStrToLong_i(buf, NULL, 10, &unique_id) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to parse unique_id: %s"), buf); - - goto cleanup; - } - - *result = unique_id; - ret = 0; - - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(buf); - return ret; -} - -/* virFindSCSIHostByPCI: - * @sysfs_prefix: "scsi_host" sysfs path, defaults to SYSFS_SCSI_HOST_PATH - * @parentaddr: string of the PCI address "scsi_host" device to be found - * @unique_id: unique_id value of the to be found "scsi_host" device - * @result: Return the host# of the matching "scsi_host" device - * - * Iterate over the SYSFS_SCSI_HOST_PATH entries looking for a matching - * PCI Address in the expected format (dddd:bb:ss.f, where 'dddd' is the - * 'domain' value, 'bb' is the 'bus' value, 'ss' is the 'slot' value, and - * 'f' is the 'function' value from the PCI address) with a unique_id file - * entry having the value expected. Unlike virReadSCSIUniqueId() we don't - * have a host number yet and that's what we're looking for. - * - * Returns the host name of the "scsi_host" which must be freed by the caller, - * or NULL on failure - */ -char * -virFindSCSIHostByPCI(const char *sysfs_prefix, - const char *parentaddr, - unsigned int unique_id) -{ - const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH; - struct dirent *entry = NULL; - DIR *dir = NULL; - char *host_link = NULL; - char *host_path = NULL; - char *p = NULL; - char *ret = NULL; - char *buf = NULL; - char *unique_path = NULL; - unsigned int read_unique_id; - - if (virDirOpen(&dir, prefix) < 0) - return NULL; - - while (virDirRead(dir, &entry, prefix) > 0) { - if (!virFileIsLink(entry->d_name)) - continue; - - if (virAsprintf(&host_link, "%s/%s", prefix, entry->d_name) < 0) - goto cleanup; - - if (virFileResolveLink(host_link, &host_path) < 0) - goto cleanup; - - if (!strstr(host_path, parentaddr)) { - VIR_FREE(host_link); - VIR_FREE(host_path); - continue; - } - VIR_FREE(host_link); - VIR_FREE(host_path); - - if (virAsprintf(&unique_path, "%s/%s/unique_id", prefix, - entry->d_name) < 0) - goto cleanup; - - if (!virFileExists(unique_path)) { - VIR_FREE(unique_path); - continue; - } - - if (virFileReadAll(unique_path, 1024, &buf) < 0) - goto cleanup; - - if ((p = strchr(buf, '\n'))) - *p = '\0'; - - if (virStrToLong_ui(buf, NULL, 10, &read_unique_id) < 0) - goto cleanup; - - VIR_FREE(buf); - - if (read_unique_id != unique_id) { - VIR_FREE(unique_path); - continue; - } - - ignore_value(VIR_STRDUP(ret, entry->d_name)); - break; - } - - cleanup: - VIR_DIR_CLOSE(dir); - VIR_FREE(unique_path); - VIR_FREE(host_link); - VIR_FREE(host_path); - VIR_FREE(buf); - return ret; -} - -/* virGetSCSIHostNumber: - * @adapter_name: Name of the host adapter - * @result: Return the entry value as unsigned int - * - * Convert the various forms of scsi_host names into the numeric - * host# value that can be used in order to scan sysfs looking for - * the specific host. - * - * Names can be either "scsi_host#" or just "host#", where - * "host#" is the back-compat format, but both equate to - * the same source adapter. First check if both pool and def - * are using same format (easier) - if so, then compare - * - * Returns 0 on success, and @result has the host number. - * Otherwise returns -1. - */ -int -virGetSCSIHostNumber(const char *adapter_name, - unsigned int *result) -{ - /* Specifying adapter like 'host5' is still supported for - * back-compat reason. - */ - if (STRPREFIX(adapter_name, "scsi_host")) { - adapter_name += strlen("scsi_host"); - } else if (STRPREFIX(adapter_name, "fc_host")) { - adapter_name += strlen("fc_host"); - } else if (STRPREFIX(adapter_name, "host")) { - adapter_name += strlen("host"); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid adapter name '%s' for SCSI pool"), - adapter_name); - return -1; - } - - if (virStrToLong_ui(adapter_name, NULL, 10, result) == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid adapter name '%s' for SCSI pool"), - adapter_name); - return -1; - } - - return 0; -} - -/* virGetSCSIHostNameByParentaddr: - * @domain: The domain from the scsi_host parentaddr - * @bus: The bus from the scsi_host parentaddr - * @slot: The slot from the scsi_host parentaddr - * @function: The function from the scsi_host parentaddr - * @unique_id: The unique id value for parentaddr - * - * Generate a parentaddr and find the scsi_host host# for - * the provided parentaddr PCI address fields. - * - * Returns the "host#" string which must be free'd by - * the caller or NULL on error - */ -char * -virGetSCSIHostNameByParentaddr(unsigned int domain, - unsigned int bus, - unsigned int slot, - unsigned int function, - unsigned int unique_id) -{ - char *name = NULL; - char *parentaddr = NULL; - - if (virAsprintf(&parentaddr, "%04x:%02x:%02x.%01x", - domain, bus, slot, function) < 0) - goto cleanup; - if (!(name = virFindSCSIHostByPCI(NULL, parentaddr, unique_id))) { - virReportError(VIR_ERR_XML_ERROR, - _("Failed to find scsi_host using PCI '%s' " - "and unique_id='%u'"), - parentaddr, unique_id); - goto cleanup; - } - - cleanup: - VIR_FREE(parentaddr); - return name; -} - -#else - -int -virReadSCSIUniqueId(const char *sysfs_prefix ATTRIBUTE_UNUSED, - int host ATTRIBUTE_UNUSED, - int *result ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return -1; -} - -char * -virFindSCSIHostByPCI(const char *sysfs_prefix ATTRIBUTE_UNUSED, - const char *parentaddr ATTRIBUTE_UNUSED, - unsigned int unique_id ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return NULL; -} - -int -virGetSCSIHostNumber(const char *adapter_name ATTRIBUTE_UNUSED, - unsigned int *result ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return -1; -} - -char * -virGetSCSIHostNameByParentaddr(unsigned int domain ATTRIBUTE_UNUSED, - unsigned int bus ATTRIBUTE_UNUSED, - unsigned int slot ATTRIBUTE_UNUSED, - unsigned int function ATTRIBUTE_UNUSED, - unsigned int unique_id ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); - return NULL; -} - -#endif /* __linux__ */ /** * virParseOwnershipIds: diff --git a/src/util/virutil.h b/src/util/virutil.h index b320c06..877207c 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -164,26 +164,6 @@ int virGetDeviceUnprivSGIO(const char *path, int *unpriv_sgio); char *virGetUnprivSGIOSysfsPath(const char *path, const char *sysfs_dir); -int virReadSCSIUniqueId(const char *sysfs_prefix, - int host, - int *result) - ATTRIBUTE_NONNULL(3); -char * -virFindSCSIHostByPCI(const char *sysfs_prefix, - const char *parentaddr, - unsigned int unique_id); -int -virGetSCSIHostNumber(const char *adapter_name, - unsigned int *result) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -char * -virGetSCSIHostNameByParentaddr(unsigned int domain, - unsigned int bus, - unsigned int slot, - unsigned int function, - unsigned int unique_id); - - int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr); diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c index 7f16e04..9efc2d2 100644 --- a/tests/scsihosttest.c +++ b/tests/scsihosttest.c @@ -26,9 +26,9 @@ # include <fcntl.h> # include <sys/stat.h> # include "virstring.h" -# include "virutil.h" # include "virerror.h" # include "virlog.h" +# include "virscsihost.h" # define VIR_FROM_THIS VIR_FROM_NONE @@ -173,8 +173,8 @@ testVirReadSCSIUniqueId(const void *data ATTRIBUTE_UNUSED) int hostnum, unique_id; for (hostnum = 0; hostnum < 4; hostnum++) { - if (virReadSCSIUniqueId(TEST_SCSIHOST_CLASS_PATH, - hostnum, &unique_id) < 0) { + if ((unique_id = virSCSIHostGetUniqueId(TEST_SCSIHOST_CLASS_PATH, + hostnum)) < 0) { fprintf(stderr, "Failed to read hostnum=%d unique_id\n", hostnum); return -1; } @@ -196,7 +196,7 @@ testVirReadSCSIUniqueId(const void *data ATTRIBUTE_UNUSED) return 0; } -/* Test virFindSCSIHostByPCI */ +/* Test virSCSIHostFindByPCI */ static int testVirFindSCSIHostByPCI(const void *data ATTRIBUTE_UNUSED) { @@ -212,25 +212,25 @@ testVirFindSCSIHostByPCI(const void *data ATTRIBUTE_UNUSED) "sysfs/class/scsi_host") < 0) goto cleanup; - if (!(ret_host = virFindSCSIHostByPCI(TEST_SCSIHOST_CLASS_PATH, + if (!(ret_host = virSCSIHostFindByPCI(TEST_SCSIHOST_CLASS_PATH, pci_addr1, unique_id1)) || STRNEQ(ret_host, "host0")) goto cleanup; VIR_FREE(ret_host); - if (!(ret_host = virFindSCSIHostByPCI(TEST_SCSIHOST_CLASS_PATH, + if (!(ret_host = virSCSIHostFindByPCI(TEST_SCSIHOST_CLASS_PATH, pci_addr1, unique_id2)) || STRNEQ(ret_host, "host1")) goto cleanup; VIR_FREE(ret_host); - if (!(ret_host = virFindSCSIHostByPCI(TEST_SCSIHOST_CLASS_PATH, + if (!(ret_host = virSCSIHostFindByPCI(TEST_SCSIHOST_CLASS_PATH, pci_addr2, unique_id1)) || STRNEQ(ret_host, "host2")) goto cleanup; VIR_FREE(ret_host); - if (!(ret_host = virFindSCSIHostByPCI(TEST_SCSIHOST_CLASS_PATH, + if (!(ret_host = virSCSIHostFindByPCI(TEST_SCSIHOST_CLASS_PATH, pci_addr2, unique_id2)) || STRNEQ(ret_host, "host3")) goto cleanup; -- 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

While perhaps improbable, it could be possible that after finding our object that another thread running essentially in parallel could attempt to delete the same vHBA. So rather than dropping the lock right after finding the object, keep the lock around while we drop the object lock and work on deleting the object. Once the delete occurs we can safely drop the driver lock again. Cleanup some of the usage of cleanup instead out for the goto label. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/node_device/node_device_driver.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 99eb56c..52f8058 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -639,21 +639,18 @@ nodeDeviceDestroy(virNodeDevicePtr dev) int parent_host = -1; nodeDeviceLock(); - obj = virNodeDeviceFindByName(&driver->devs, dev->name); - nodeDeviceUnlock(); - - if (!obj) { + if (!(obj = virNodeDeviceFindByName(&driver->devs, dev->name))) { virReportError(VIR_ERR_NO_NODE_DEVICE, _("no node device with matching name '%s'"), dev->name); - goto out; + goto cleanup; } if (virNodeDeviceDestroyEnsureACL(dev->conn, obj->def) < 0) - goto out; + goto cleanup; - if (virNodeDeviceGetWWNs(obj->def, &wwnn, &wwpn) == -1) - goto out; + if (virNodeDeviceGetWWNs(obj->def, &wwnn, &wwpn) < 0) + goto cleanup; /* virNodeDeviceGetParentHost will cause the device object's lock to be @@ -663,20 +660,21 @@ nodeDeviceDestroy(virNodeDevicePtr dev) if (VIR_STRDUP(parent_name, obj->def->parent) < 0) { virNodeDeviceObjUnlock(obj); obj = NULL; - goto out; + goto cleanup; } virNodeDeviceObjUnlock(obj); obj = NULL; if (virNodeDeviceGetParentHost(&driver->devs, dev->name, parent_name, &parent_host) < 0) - goto out; + goto cleanup; if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) - goto out; + goto cleanup; ret = 0; - out: + cleanup: + nodeDeviceUnlock(); if (obj) virNodeDeviceObjUnlock(obj); VIR_FREE(parent_name); -- 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 not providing a parent in the input XML, but still being able to create finding a VPORT capable NPIV HBA. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- tests/fchosttest.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/fchosttest.c b/tests/fchosttest.c index 715571e..15cda75 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -44,6 +44,15 @@ static const char test7_xml[] = " </capability>" "</device>"; +/* virNodeDeviceCreateXML without "<parent>" to find the vport capable HBA */ +static const char test8_xml[] = +"<device>" +" <capability type='scsi_host'>" +" <capability type='fc_host'>" +" </capability>" +" </capability>" +"</device>"; + /* Test virIsVHBACapable */ static int test1(const void *data ATTRIBUTE_UNUSED) @@ -270,6 +279,9 @@ mymain(void) if (virTestRun("manageVHBAByNodeDevice-by-parent", manageVHBAByNodeDevice, test7_xml) < 0) ret = -1; + if (virTestRun("manageVHBAByNodeDevice-no-parent", manageVHBAByNodeDevice, + test8_xml) < 0) + ret = -1; cleanup: VIR_FREE(fchost_prefix); -- 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

Add a test that allows providing the parent fabric_wwn in the input XML in order to create the vHBA. This also fixes a mixed setting of the fabric_wwn field from the read test driver XML strings. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/node_device_conf.c | 8 ++++++++ tests/fchosttest.c | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f73fede..414ab47 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -37,9 +37,12 @@ #include "virbuffer.h" #include "viruuid.h" #include "virrandom.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV +VIR_LOG_INIT("conf.node_device_conf"); + VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST, "system", "pci", @@ -1033,6 +1036,11 @@ virNodeDevCapSCSIHostParseXML(xmlXPathContextPtr ctxt, } } + if (virNodeDevCapsDefParseString("string(./fabric_wwn[1])", + ctxt, + &data->scsi_host.fabric_wwn) < 0) + VIR_DEBUG("No fabric_wwn defined for '%s'", def->name); + ctxt->node = orignode2; } else { diff --git a/tests/fchosttest.c b/tests/fchosttest.c index d083104..51fdcbd 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -64,6 +64,17 @@ static const char test9_xml[] = " </capability>" "</device>"; +/* virNodeDeviceCreateXML using "<parent fabric_wwn='%s'/>" to find the + * vport capable HBA */ +static const char test10_xml[] = +"<device>" +" <parent fabric_wwn='2000000043214321'/>" +" <capability type='scsi_host'>" +" <capability type='fc_host'>" +" </capability>" +" </capability>" +"</device>"; + /* Test virIsVHBACapable */ static int test1(const void *data ATTRIBUTE_UNUSED) @@ -296,6 +307,9 @@ mymain(void) if (virTestRun("manageVHBAByNodeDevice-parent-wwn", manageVHBAByNodeDevice, test9_xml) < 0) ret = -1; + if (virTestRun("manageVHBAByNodeDevice-parent-fabric-wwn", + manageVHBAByNodeDevice, test10_xml) < 0) + ret = -1; cleanup: VIR_FREE(fchost_prefix); -- 2.7.4

Create a function which takes a node device "name" entry to lookup and returns a string containing the parent name for the node device. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- Adjustment from v1... Instead of moving function to virvhba.c, create a new node_device_conf API which does something similar to what the changed code from patch 7 and 8 (e.g. virVHBAGetParent) would have done. src/conf/node_device_conf.c | 30 ++++++++++++++++++++++++++++++ src/conf/node_device_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 34 insertions(+) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index b3063d9..34ca0ef 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2285,3 +2285,33 @@ virNodeDeviceObjListExport(virConnectPtr conn, VIR_FREE(tmp_devices); return ret; } + + +/* virNodeDeviceGetParentName + * @conn: Connection pointer + * @nodedev_name: Node device to lookup + * + * Lookup the node device by name and return the parent name + * + * Returns parent name on success, caller is responsible for freeing; + * otherwise, returns NULL on failure + */ +char * +virNodeDeviceGetParentName(virConnectPtr conn, + const char *nodedev_name) +{ + virNodeDevicePtr device = NULL; + char *parent; + + if (!(device = virNodeDeviceLookupByName(conn, nodedev_name))) { + virReportError(VIR_ERR_XML_ERROR, + _("Cannot find '%s' in node device database"), + nodedev_name); + return NULL; + } + + ignore_value(VIR_STRDUP(parent, virNodeDeviceGetParent(device))); + virObjectUnref(device); + + return parent; +} diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 40e930a..6c43546 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -352,4 +352,7 @@ int virNodeDeviceObjListExport(virConnectPtr conn, virNodeDeviceObjListFilter filter, unsigned int flags); +char *virNodeDeviceGetParentName(virConnectPtr conn, + const char *nodedev_name); + #endif /* __VIR_NODE_DEVICE_CONF_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d721c12..9ad0b0a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -706,6 +706,7 @@ virNodeDeviceFindVportParentHost; virNodeDeviceGetParentHost; virNodeDeviceGetParentHostByFabricWWN; virNodeDeviceGetParentHostByWWNs; +virNodeDeviceGetParentName; virNodeDeviceGetWWNs; virNodeDeviceHasCap; virNodeDeviceObjListExport; -- 2.9.3

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