From 0fffd7d7d2767d8f03407c2c61da6baf0ac181c5 Mon Sep 17 00:00:00 2001

Message-Id: <0fffd7d7d2767d8f03407c2c61da6baf0ac181c5@dist-git>

From: John Ferlan <jferlan@redhat.com>

Date: Tue, 12 Mar 2019 13:55:58 -0400

Subject: [PATCH] util: Introduce virStorageFileGetNPIVKey

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

https://bugzilla.redhat.com/show_bug.cgi?id=1687715 (7.6.z)

https://bugzilla.redhat.com/show_bug.cgi?id=1657468 (7.7.0)

The vHBA/NPIV LUNs created via the udev processing of the

VPORT_CREATE command end up using the same serial value

as seen/generated by the /lib/udev/scsi_id as returned

during virStorageFileGetSCSIKey. Therefore, in order to

generate a unique enough key to be used when adding the

LUN as a volume during virStoragePoolObjAddVol a more

unique key needs to be generated for an NPIV volume.

The problem is illustrated by the following example, where

scsi_host5 is a vHBA used with the following LUNs:

$ lsscsi -tg

...

[5:0:4:0]    disk    fc:0x5006016844602198,0x101f00  /dev/sdh   /dev/sg23

[5:0:5:0]    disk    fc:0x5006016044602198,0x102000  /dev/sdi   /dev/sg24

...

Calling virStorageFileGetSCSIKey would return:

/lib/udev/scsi_id --device /dev/sdh --whitelisted --replace-whitespace /dev/sdh

350060160c460219850060160c4602198

/lib/udev/scsi_id --device /dev/sdh --whitelisted --replace-whitespace /dev/sdi

350060160c460219850060160c4602198

Note that althrough /dev/sdh and /dev/sdi are separate LUNs, they

end up with the same serial number used for the vol->key value.

When virStoragePoolFCRefreshThread calls virStoragePoolObjAddVol

the second LUN fails to be added with the following message

getting logged:

    virHashAddOrUpdateEntry:341 : internal error: Duplicate key

To resolve this, virStorageFileGetNPIVKey will use a similar call

sequence as virStorageFileGetSCSIKey, except that it will add the

"--export" option to the call. This results in more detailed output

which needs to be parsed in order to formulate a unique enough key

to be used. In order to be unique enough, the returned value will

concatenate the target port as returned in the "ID_TARGET_PORT"

field from the command to the "ID_SERIAL" value.

Signed-off-by: John Ferlan <jferlan@redhat.com>

ACKed-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

(cherry picked from commit 5f9e211c9319311fc296cb91ca8e330917ac9494)

Handled build failure of not having VIR_AUTOFREE in the next-7.6 sources,

by not using VIR_AUTOFREE and using VIR_FREE(outbuf) explicitly.

Signed-off-by: John Ferlan <jferlan@redhat.com>

Message-Id: <20190312175559.13583-4-jferlan@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 src/libvirt_private.syms  |  1 +

 src/util/virstoragefile.c | 80 +++++++++++++++++++++++++++++++++++++++

 src/util/virstoragefile.h |  2 +

 3 files changed, 83 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms

index 86846f3b08..636891eabd 100644

--- a/src/libvirt_private.syms

+++ b/src/libvirt_private.syms

@@ -2820,6 +2820,7 @@ virStorageFileGetMetadata;

 virStorageFileGetMetadataFromBuf;

 virStorageFileGetMetadataFromFD;

 virStorageFileGetMetadataInternal;

+virStorageFileGetNPIVKey;

 virStorageFileGetRelativeBackingPath;

 virStorageFileGetSCSIKey;

 virStorageFileGetUniqueIdentifier;

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c

index 56d38b467e..52c9dc0e1a 100644

--- a/src/util/virstoragefile.c

+++ b/src/util/virstoragefile.c

@@ -1503,6 +1503,86 @@ int virStorageFileGetSCSIKey(const char *path,

 #endif

+#ifdef WITH_UDEV

+/* virStorageFileGetNPIVKey

+ * @path: Path to the NPIV device

+ * @key: Unique key to be returned

+ *

+ * Using a udev specific function, query the @path to get and return a

+ * unique @key for the caller to use. Unlike the GetSCSIKey method, an

+ * NPIV LUN is uniquely identified by its ID_TARGET_PORT value.

+ *

+ * Returns:

+ *     0 On success, with the @key filled in or @key=NULL if the

+ *       returned output string didn't have the data we need to

+ *       formulate a unique key value

+ *    -1 When WITH_UDEV is undefined and a system error is reported

+ *    -2 When WITH_UDEV is defined, but calling virCommandRun fails

+ */

+# define ID_SERIAL "ID_SERIAL="

+# define ID_TARGET_PORT "ID_TARGET_PORT="

+int

+virStorageFileGetNPIVKey(const char *path,

+                         char **key)

+{

+    int status;

+    char *outbuf = NULL;

+    const char *serial;

+    const char *port;

+    virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id",

+                                             "--replace-whitespace",

+                                             "--whitelisted",

+                                             "--export",

+                                             "--device", path,

+                                             NULL

+                                             );

+    int ret = -2;

+

+    *key = NULL;

+

+    /* Run the program and capture its output */

+    virCommandSetOutputBuffer(cmd, &outbuf);

+    if (virCommandRun(cmd, &status) < 0)

+        goto cleanup;

+

+    /* Explicitly check status == 0, rather than passing NULL

+     * to virCommandRun because we don't want to raise an actual

+     * error in this scenario, just return a NULL key.

+     */

+    if (status == 0 && *outbuf &&

+        (serial = strstr(outbuf, ID_SERIAL)) &&

+        (port = strstr(outbuf, ID_TARGET_PORT))) {

+        char *tmp;

+

+        serial += strlen(ID_SERIAL);

+        port += strlen(ID_TARGET_PORT);

+

+        if ((tmp = strchr(serial, '\n')))

+            *tmp = '\0';

+

+        if ((tmp = strchr(port, '\n')))

+            *tmp = '\0';

+

+        if (*serial != '\0' && *port != '\0')

+            ignore_value(virAsprintf(key, "%s_PORT%s", serial, port));

+    }

+

+    ret = 0;

+

+ cleanup:

+    virCommandFree(cmd);

+    VIR_FREE(outbuf);

+

+    return ret;

+}

+#else

+int virStorageFileGetNPIVKey(const char *path,

+                             char **key ATTRIBUTE_UNUSED)

+{

+    return -1;

+}

+#endif

+

 /**

  * virStorageFileParseBackingStoreStr:

  * @str: backing store specifier string to parse

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h

index bbb0b8b3c1..7d28dcfe65 100644

--- a/src/util/virstoragefile.h

+++ b/src/util/virstoragefile.h

@@ -386,6 +386,8 @@ int virStorageFileGetLVMKey(const char *path,

 int virStorageFileGetSCSIKey(const char *path,

                              char **key,

                              bool ignoreError);

+int virStorageFileGetNPIVKey(const char *path,

+                             char **key);

 void virStorageAuthDefFree(virStorageAuthDefPtr def);

 virStorageAuthDefPtr virStorageAuthDefCopy(const virStorageAuthDef *src);

-- 

2.21.0