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