Similar to scsi_host and fc_host, there is a relation between a
scsi_target and its transport specific fc_remote_port. Let's expose this
relation and relevant information behind it.
An example for a virsh nodedev-dumpxml:
virsh # nodedev-dumpxml scsi_target0_0_0
<device>
<name>scsi_target0_0_0</name>
<path>/sys/devices/[...]/host0/rport-0:0-0/target0:0:0</path>
<parent>scsi_host0</parent>
<capability type='scsi_target'>
<target>target0:0:0</target>
<capability type='fc_remote_port'>
<rport>rport-0:0-0</rport>
<wwpn>0x9d73bc45f0e21a86</wwpn>
</capability>
</capability>
</device>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
---
docs/schemas/nodedev.rng | 20 ++++++++
src/conf/node_device_conf.c | 75 +++++++++++++++++++++++++++-
src/conf/node_device_conf.h | 7 +++
src/node_device/node_device_driver.c | 5 +-
src/node_device/node_device_linux_sysfs.c | 56 +++++++++++++++++++++
src/node_device/node_device_linux_sysfs.h | 2 +
src/node_device/node_device_udev.c | 4 +-
tests/nodedevschemadata/scsi_target1_0_0.xml | 12 +++++
tests/nodedevxml2xmltest.c | 1 +
9 files changed, 178 insertions(+), 4 deletions(-)
create mode 100644 tests/nodedevschemadata/scsi_target1_0_0.xml
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 87bfb0c28..5bcf31787 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -458,6 +458,20 @@
</optional>
</define>
+ <define name='capsfcrport'>
+ <attribute name='type'>
+ <value>fc_remote_port</value>
+ </attribute>
+
+ <element name='rport'>
+ <text/>
+ </element>
+
+ <element name='wwpn'>
+ <ref name='wwn'/>
+ </element>
+ </define>
+
<define name='capscsitarget'>
<attribute name='type'>
<value>scsi_target</value>
@@ -466,6 +480,12 @@
<element name='target'>
<text/>
</element>
+
+ <optional>
+ <element name='capability'>
+ <ref name='capsfcrport'/>
+ </element>
+ </optional>
</define>
<define name='capscsi'>
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 28d4907f5..ed003d37b 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -563,6 +563,16 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
case VIR_NODE_DEV_CAP_SCSI_TARGET:
virBufferEscapeString(&buf, "<target>%s</target>\n",
data->scsi_target.name);
+ if (data->scsi_target.flags & VIR_NODE_DEV_CAP_FLAG_FC_RPORT) {
+ virBufferAddLit(&buf, "<capability type='fc_remote_port'>\n");
+ virBufferAdjustIndent(&buf, 2);
+ virBufferAsprintf(&buf, "<rport>%s</rport>\n",
+ data->scsi_target.rport);
+ virBufferAsprintf(&buf, "<wwpn>0x%llx</wwpn>\n",
+ data->scsi_target.wwpn);
+ virBufferAdjustIndent(&buf, -2);
+ virBufferAddLit(&buf, "</capability>\n");
+ }
break;
case VIR_NODE_DEV_CAP_SCSI:
virNodeDeviceCapSCSIDefFormat(&buf, data);
@@ -932,8 +942,10 @@ virNodeDevCapSCSITargetParseXML(xmlXPathContextPtr ctxt,
xmlNodePtr node,
virNodeDevCapSCSITargetPtr scsi_target)
{
- xmlNodePtr orignode;
- int ret = -1;
+ xmlNodePtr orignode, *nodes = NULL;
+ int ret = -1, n = 0;
+ size_t i;
+ char *type = NULL, *wwpn = NULL;
orignode = ctxt->node;
ctxt->node = node;
@@ -946,10 +958,68 @@ virNodeDevCapSCSITargetParseXML(xmlXPathContextPtr ctxt,
goto out;
}
+ if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0)
+ goto out;
+
+ for (i = 0; i < n; ++i) {
+ type = virXMLPropString(nodes[i], "type");
+
+ if (!type) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing type for SCSI target capability for '%s'"),
+ def->name);
+ goto out;
+ }
+
+ if (STREQ(type, "fc_remote_port")) {
+ xmlNodePtr orignode2;
+
+ scsi_target->flags |= VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
+
+ orignode2 = ctxt->node;
+ ctxt->node = nodes[i];
+
+ if (virNodeDevCapsDefParseString("string(./rport[1])",
+ ctxt,
+ &scsi_target->rport) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing rport name for '%s'"), def->name);
+ goto out;
+ }
+
+ if (virNodeDevCapsDefParseString("string(./wwpn[1])",
+ ctxt, &wwpn) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing wwpn identifier for '%s'"),
+ def->name);
+ goto out;
+ }
+
+ if (virStrToLong_ullp(wwpn, NULL, 16, &scsi_target->wwpn) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid wwpn identifier '%s' for '%s'"),
+ wwpn, def->name);
+ goto out;
+ }
+
+ ctxt->node = orignode2;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown SCSI target capability type '%s' for '%s'"),
+ type, def->name);
+ goto out;
+ }
+
+ VIR_FREE(type);
+ VIR_FREE(wwpn);
+ }
+
ret = 0;
out:
ctxt->node = orignode;
+ VIR_FREE(type);
+ VIR_FREE(wwpn);
return ret;
}
@@ -2132,6 +2202,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
break;
case VIR_NODE_DEV_CAP_SCSI_TARGET:
VIR_FREE(data->scsi_target.name);
+ VIR_FREE(data->scsi_target.rport);
break;
case VIR_NODE_DEV_CAP_SCSI:
VIR_FREE(data->scsi.type);
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 285841edc..f5267efda 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -93,6 +93,10 @@ typedef enum {
} virNodeDevSCSIHostCapFlags;
typedef enum {
+ VIR_NODE_DEV_CAP_FLAG_FC_RPORT = (1 << 0),
+} virNodeDevSCSITargetCapsFlags;
+
+typedef enum {
VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0),
VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1),
VIR_NODE_DEV_CAP_FLAG_PCIE = (1 << 2),
@@ -227,6 +231,9 @@ typedef struct _virNodeDevCapSCSITarget virNodeDevCapSCSITarget;
typedef virNodeDevCapSCSITarget *virNodeDevCapSCSITargetPtr;
struct _virNodeDevCapSCSITarget {
char *name;
+ unsigned int flags; /* enum virNodeDevSCSITargetCapsFlags */
+ char *rport;
+ unsigned long long wwpn;
};
typedef struct _virNodeDevCapSCSI virNodeDevCapSCSI;
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 286af26bc..acb8b89af 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -56,6 +56,10 @@ static int update_caps(virNodeDeviceObjPtr dev)
case VIR_NODE_DEV_CAP_SCSI_HOST:
nodeDeviceSysfsGetSCSIHostCaps(&cap->data.scsi_host);
break;
+ case VIR_NODE_DEV_CAP_SCSI_TARGET:
+ nodeDeviceSysfsGetSCSITargetCaps(dev->def->sysfs_path,
+ &cap->data.scsi_target);
+ break;
case VIR_NODE_DEV_CAP_NET:
if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0)
return -1;
@@ -76,7 +80,6 @@ static int update_caps(virNodeDeviceObjPtr dev)
case VIR_NODE_DEV_CAP_SYSTEM:
case VIR_NODE_DEV_CAP_USB_DEV:
case VIR_NODE_DEV_CAP_USB_INTERFACE:
- case VIR_NODE_DEV_CAP_SCSI_TARGET:
case VIR_NODE_DEV_CAP_SCSI:
case VIR_NODE_DEV_CAP_STORAGE:
case VIR_NODE_DEV_CAP_FC_HOST:
diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
index 1b7aa9435..90452fb29 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -26,11 +26,13 @@
#include <sys/stat.h>
#include <stdlib.h>
+#include "dirname.h"
#include "node_device_driver.h"
#include "node_device_hal.h"
#include "node_device_linux_sysfs.h"
#include "virerror.h"
#include "viralloc.h"
+#include "virfcp.h"
#include "virlog.h"
#include "virfile.h"
#include "virscsihost.h"
@@ -124,6 +126,54 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host)
}
+int
+nodeDeviceSysfsGetSCSITargetCaps(const char *sysfsPath,
+ virNodeDevCapSCSITargetPtr scsi_target)
+{
+ int ret = -1;
+ char *dir = NULL, *rport = NULL, *wwpn = NULL;
+
+ VIR_DEBUG("Checking if '%s' is an FC remote port", scsi_target->name);
+
+ /* /sys/devices/[...]/host0/rport-0:0-0/target0:0:0 -> rport-0:0-0 */
+ if (!(dir = mdir_name(sysfsPath)))
+ return -1;
+
+ if (VIR_STRDUP(rport, last_component(dir)) < 0)
+ goto cleanup;
+
+ if (!virFCIsCapableRport(rport))
+ goto cleanup;
+
+ VIR_FREE(scsi_target->rport);
+ scsi_target->rport = rport;
+
+ if (virFCReadRportValue(scsi_target->rport, "port_name", &wwpn) < 0) {
+ VIR_WARN("Failed to read port_name for '%s'", scsi_target->rport);
+ goto cleanup;
+ }
+
+ if (virStrToLong_ullp(wwpn, NULL, 16, &scsi_target->wwpn) < 0) {
+ VIR_WARN("Failed to parse value of port_name '%s'", wwpn);
+ goto cleanup;
+ }
+
+ scsi_target->flags |= VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
+ ret = 0;
+
+ cleanup:
+ if (ret < 0) {
+ VIR_FREE(scsi_target->rport);
+ scsi_target->wwpn = 0;
+ scsi_target->flags &= ~VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
+ }
+ VIR_FREE(dir);
+ VIR_FREE(wwpn);
+
+ return ret;
+}
+
+
static int
nodeDeviceSysfsGetPCISRIOVCaps(const char *sysfsPath,
virNodeDevCapPCIDevPtr pci_dev)
@@ -230,6 +280,12 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host ATTRIBUTE_UNUS
return -1;
}
+int nodeDeviceSysfsGetSCSITargetCaps(const char *sysfsPath ATTRIBUTE_UNUSED,
+ virNodeDevCapSCSITargetPtr scsi_target ATTRIBUTE_UNUSED)
+{
+ return -1;
+}
+
int
nodeDeviceSysfsGetPCIRelatedDevCaps(const char *sysfsPath ATTRIBUTE_UNUSED,
virNodeDevCapPCIDevPtr pci_dev ATTRIBUTE_UNUSED)
diff --git a/src/node_device/node_device_linux_sysfs.h b/src/node_device/node_device_linux_sysfs.h
index 8deea6699..12cfe6341 100644
--- a/src/node_device/node_device_linux_sysfs.h
+++ b/src/node_device/node_device_linux_sysfs.h
@@ -26,6 +26,8 @@
# include "node_device_conf.h"
int nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host);
+int nodeDeviceSysfsGetSCSITargetCaps(const char *sysfsPath,
+ virNodeDevCapSCSITargetPtr scsi_target);
int nodeDeviceSysfsGetPCIRelatedDevCaps(const char *sysfsPath,
virNodeDevCapPCIDevPtr pci_dev);
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 7744c2637..49269b678 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -697,7 +697,7 @@ static int udevProcessSCSIHost(struct udev_device *device ATTRIBUTE_UNUSED,
}
-static int udevProcessSCSITarget(struct udev_device *device ATTRIBUTE_UNUSED,
+static int udevProcessSCSITarget(struct udev_device *device,
virNodeDeviceDefPtr def)
{
const char *sysname = NULL;
@@ -708,6 +708,8 @@ static int udevProcessSCSITarget(struct udev_device *device ATTRIBUTE_UNUSED,
if (VIR_STRDUP(scsi_target->name, sysname) < 0)
return -1;
+ nodeDeviceSysfsGetSCSITargetCaps(def->sysfs_path, &def->caps->data.scsi_target);
+
if (udevGenerateDeviceName(device, def, NULL) != 0)
return -1;
diff --git a/tests/nodedevschemadata/scsi_target1_0_0.xml b/tests/nodedevschemadata/scsi_target1_0_0.xml
new file mode 100644
index 000000000..b783c54fe
--- /dev/null
+++ b/tests/nodedevschemadata/scsi_target1_0_0.xml
@@ -0,0 +1,12 @@
+<device>
+ <name>scsi_target1_0_0</name>
+ <path>/sys/devices/css0/0.0.0000/0.0.0000/host1/rport-1:0-0/target1:0:0</path>
+ <parent>scsi_host0</parent>
+ <capability type='scsi_target'>
+ <target>target1:0:0</target>
+ <capability type='fc_remote_port'>
+ <rport>rport-1:0-0</rport>
+ <wwpn>0x9d73bc45f0e21a86</wwpn>
+ </capability>
+ </capability>
+</device>
diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
index 805deef26..5d9f4724d 100644
--- a/tests/nodedevxml2xmltest.c
+++ b/tests/nodedevxml2xmltest.c
@@ -95,6 +95,7 @@ mymain(void)
DO_TEST("pci_0000_00_02_0_header_type");
DO_TEST("pci_0000_00_1c_0_header_type");
DO_TEST("scsi_target0_0_0");
+ DO_TEST("scsi_target1_0_0");
DO_TEST("pci_0000_02_10_7_sriov");
DO_TEST("pci_0000_02_10_7_sriov_vfs");
DO_TEST("pci_0000_02_10_7_sriov_zero_vfs_max_count");
--
2.11.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
> Similar to scsi_host and fc_host, there is a relation between a
> scsi_target and its transport specific fc_remote_port. Let's expose this
> relation and relevant information behind it.
>
> An example for a virsh nodedev-dumpxml:
>
> virsh # nodedev-dumpxml scsi_target0_0_0
> <device>
> <name>scsi_target0_0_0</name>
> <path>/sys/devices/[...]/host0/rport-0:0-0/target0:0:0</path>
> <parent>scsi_host0</parent>
> <capability type='scsi_target'>
> <target>target0:0:0</target>
> <capability type='fc_remote_port'>
> <rport>rport-0:0-0</rport>
> <wwpn>0x9d73bc45f0e21a86</wwpn>
> </capability>
> </capability>
> </device>
>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
> ---
> docs/schemas/nodedev.rng | 20 ++++++++
> src/conf/node_device_conf.c | 75 +++++++++++++++++++++++++++-
> src/conf/node_device_conf.h | 7 +++
> src/node_device/node_device_driver.c | 5 +-
> src/node_device/node_device_linux_sysfs.c | 56 +++++++++++++++++++++
> src/node_device/node_device_linux_sysfs.h | 2 +
> src/node_device/node_device_udev.c | 4 +-
> tests/nodedevschemadata/scsi_target1_0_0.xml | 12 +++++
> tests/nodedevxml2xmltest.c | 1 +
> 9 files changed, 178 insertions(+), 4 deletions(-)
> create mode 100644 tests/nodedevschemadata/scsi_target1_0_0.xml
>
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 87bfb0c28..5bcf31787 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -458,6 +458,20 @@
> </optional>
> </define>
>
> + <define name='capsfcrport'>
> + <attribute name='type'>
> + <value>fc_remote_port</value>
> + </attribute>
> +
> + <element name='rport'>
> + <text/>
> + </element>
> +
> + <element name='wwpn'>
> + <ref name='wwn'/>
NB: 'wwn' is a string...
> + </element>
> + </define>
> +
> <define name='capscsitarget'>
> <attribute name='type'>
> <value>scsi_target</value>
> @@ -466,6 +480,12 @@
> <element name='target'>
> <text/>
> </element>
> +
> + <optional>
> + <element name='capability'>
> + <ref name='capsfcrport'/>
> + </element>
> + </optional>
> </define>
>
> <define name='capscsi'>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 28d4907f5..ed003d37b 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -563,6 +563,16 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
> case VIR_NODE_DEV_CAP_SCSI_TARGET:
> virBufferEscapeString(&buf, "<target>%s</target>\n",
> data->scsi_target.name);
> + if (data->scsi_target.flags & VIR_NODE_DEV_CAP_FLAG_FC_RPORT) {
> + virBufferAddLit(&buf, "<capability type='fc_remote_port'>\n");
> + virBufferAdjustIndent(&buf, 2);
> + virBufferAsprintf(&buf, "<rport>%s</rport>\n",
> + data->scsi_target.rport);
> + virBufferAsprintf(&buf, "<wwpn>0x%llx</wwpn>\n",
> + data->scsi_target.wwpn);
If you don't convert to a number, then you don't need to perform this
conversion as it's just a %s print.
> + virBufferAdjustIndent(&buf, -2);
> + virBufferAddLit(&buf, "</capability>\n");
> + }
> break;
> case VIR_NODE_DEV_CAP_SCSI:
> virNodeDeviceCapSCSIDefFormat(&buf, data);
> @@ -932,8 +942,10 @@ virNodeDevCapSCSITargetParseXML(xmlXPathContextPtr ctxt,
> xmlNodePtr node,
> virNodeDevCapSCSITargetPtr scsi_target)
> {
> - xmlNodePtr orignode;
> - int ret = -1;
> + xmlNodePtr orignode, *nodes = NULL;
> + int ret = -1, n = 0;
> + size_t i;
> + char *type = NULL, *wwpn = NULL;
>
> orignode = ctxt->node;
> ctxt->node = node;
> @@ -946,10 +958,68 @@ virNodeDevCapSCSITargetParseXML(xmlXPathContextPtr ctxt,
> goto out;
> }
>
> + if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0)
> + goto out;
> +
> + for (i = 0; i < n; ++i) {
> + type = virXMLPropString(nodes[i], "type");
> +
> + if (!type) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("missing type for SCSI target capability for '%s'"),
> + def->name);
> + goto out;
> + }
> +
> + if (STREQ(type, "fc_remote_port")) {
> + xmlNodePtr orignode2;
> +
> + scsi_target->flags |= VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
> +
> + orignode2 = ctxt->node;
> + ctxt->node = nodes[i];
Like the virNodeDevCapSCSIHostParseXML I assume this only parses the
first 'fc_remote_port'... Trying to think whether a VIR_FREE would be
necessary here (and perhaps from SCSIHost code just in case there were
more than one <rport> or <wwpn> provided...
> +
> + if (virNodeDevCapsDefParseString("string(./rport[1])",
> + ctxt,
> + &scsi_target->rport) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("missing rport name for '%s'"), def->name);
> + goto out;
> + }
> +
> + if (virNodeDevCapsDefParseString("string(./wwpn[1])",
> + ctxt, &wwpn) < 0) {
If you don't do the conversion, this could just be a straight parse into
scsi_target->wwpn
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("missing wwpn identifier for '%s'"),
> + def->name);
> + goto out;
> + }
> +
> + if (virStrToLong_ullp(wwpn, NULL, 16, &scsi_target->wwpn) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("invalid wwpn identifier '%s' for '%s'"),
> + wwpn, def->name);
Why even both with this conversion?
> + goto out;
> + }
> +
> + ctxt->node = orignode2;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unknown SCSI target capability type '%s' for '%s'"),
> + type, def->name);
> + goto out;
> + }
> +
> + VIR_FREE(type);
> + VIR_FREE(wwpn);
> + }
> +
> ret = 0;
>
> out:
> ctxt->node = orignode;
> + VIR_FREE(type);
> + VIR_FREE(wwpn);
If you don't do the conversion, then this VIR_FREE is unnecessary.
Coverity complained that nodes was leaked, so I added a VIR_FREE(nodes);
here.
> return ret;
> }
>
> @@ -2132,6 +2202,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
> break;
> case VIR_NODE_DEV_CAP_SCSI_TARGET:
> VIR_FREE(data->scsi_target.name);
> + VIR_FREE(data->scsi_target.rport);
Without the conversion there's a VIR_FREE here for wwpn
> break;
> case VIR_NODE_DEV_CAP_SCSI:
> VIR_FREE(data->scsi.type);
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index 285841edc..f5267efda 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -93,6 +93,10 @@ typedef enum {
> } virNodeDevSCSIHostCapFlags;
>
> typedef enum {
> + VIR_NODE_DEV_CAP_FLAG_FC_RPORT = (1 << 0),
> +} virNodeDevSCSITargetCapsFlags;
> +
> +typedef enum {
> VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0),
> VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1),
> VIR_NODE_DEV_CAP_FLAG_PCIE = (1 << 2),
> @@ -227,6 +231,9 @@ typedef struct _virNodeDevCapSCSITarget virNodeDevCapSCSITarget;
> typedef virNodeDevCapSCSITarget *virNodeDevCapSCSITargetPtr;
> struct _virNodeDevCapSCSITarget {
> char *name;
> + unsigned int flags; /* enum virNodeDevSCSITargetCapsFlags */
> + char *rport;
> + unsigned long long wwpn;
and this would be a char *wwpn;
> };
>
> typedef struct _virNodeDevCapSCSI virNodeDevCapSCSI;
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 286af26bc..acb8b89af 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -56,6 +56,10 @@ static int update_caps(virNodeDeviceObjPtr dev)
> case VIR_NODE_DEV_CAP_SCSI_HOST:
> nodeDeviceSysfsGetSCSIHostCaps(&cap->data.scsi_host);
> break;
> + case VIR_NODE_DEV_CAP_SCSI_TARGET:
> + nodeDeviceSysfsGetSCSITargetCaps(dev->def->sysfs_path,
> + &cap->data.scsi_target);
> + break;
> case VIR_NODE_DEV_CAP_NET:
> if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0)
> return -1;
> @@ -76,7 +80,6 @@ static int update_caps(virNodeDeviceObjPtr dev)
> case VIR_NODE_DEV_CAP_SYSTEM:
> case VIR_NODE_DEV_CAP_USB_DEV:
> case VIR_NODE_DEV_CAP_USB_INTERFACE:
> - case VIR_NODE_DEV_CAP_SCSI_TARGET:
> case VIR_NODE_DEV_CAP_SCSI:
> case VIR_NODE_DEV_CAP_STORAGE:
> case VIR_NODE_DEV_CAP_FC_HOST:
> diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
> index 1b7aa9435..90452fb29 100644
> --- a/src/node_device/node_device_linux_sysfs.c
> +++ b/src/node_device/node_device_linux_sysfs.c
> @@ -26,11 +26,13 @@
> #include <sys/stat.h>
> #include <stdlib.h>
>
> +#include "dirname.h"
> #include "node_device_driver.h"
> #include "node_device_hal.h"
> #include "node_device_linux_sysfs.h"
> #include "virerror.h"
> #include "viralloc.h"
> +#include "virfcp.h"
> #include "virlog.h"
> #include "virfile.h"
> #include "virscsihost.h"
> @@ -124,6 +126,54 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host)
> }
>
>
> +int
> +nodeDeviceSysfsGetSCSITargetCaps(const char *sysfsPath,
> + virNodeDevCapSCSITargetPtr scsi_target)
> +{
> + int ret = -1;
> + char *dir = NULL, *rport = NULL, *wwpn = NULL;
> +
> + VIR_DEBUG("Checking if '%s' is an FC remote port", scsi_target->name);
> +
> + /* /sys/devices/[...]/host0/rport-0:0-0/target0:0:0 -> rport-0:0-0 */
> + if (!(dir = mdir_name(sysfsPath)))
> + return -1;
> +
> + if (VIR_STRDUP(rport, last_component(dir)) < 0)
> + goto cleanup;
> +
> + if (!virFCIsCapableRport(rport))
> + goto cleanup;
> +
> + VIR_FREE(scsi_target->rport);
> + scsi_target->rport = rport;
VIR_STEAL_PTR(scsi_target->rport, rport);
> +
> + if (virFCReadRportValue(scsi_target->rport, "port_name", &wwpn) < 0) {
> + VIR_WARN("Failed to read port_name for '%s'", scsi_target->rport);
> + goto cleanup;
> + }
> +
> + if (virStrToLong_ullp(wwpn, NULL, 16, &scsi_target->wwpn) < 0) {
> + VIR_WARN("Failed to parse value of port_name '%s'", wwpn);
> + goto cleanup;
> + }
> +
> + scsi_target->flags |= VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
> + ret = 0;
> +
> + cleanup:
> + if (ret < 0) {
> + VIR_FREE(scsi_target->rport);
> + scsi_target->wwpn = 0;
> + scsi_target->flags &= ~VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
> + }
Coverity complained that rport could be leaked, so I added the
VIR_STEAL_PTR above and a VIR_FREE(rport); here.
> + VIR_FREE(dir);
> + VIR_FREE(wwpn);
> +
> + return ret;
> +}
> +
> +
> static int
> nodeDeviceSysfsGetPCISRIOVCaps(const char *sysfsPath,
> virNodeDevCapPCIDevPtr pci_dev)
> @@ -230,6 +280,12 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host ATTRIBUTE_UNUS
> return -1;
> }
>
Reviewed-by: John Ferlan <jferlan@redhat.com>
I can make the suggested changes if you'd like or you can send a v2 or a
squash diff... Let me know
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
John Ferlan <jferlan@redhat.com> [2017-05-25, 03:26PM -0400]:
>
>
>On 05/22/2017 02:38 AM, Bjoern Walk wrote:
>> Similar to scsi_host and fc_host, there is a relation between a
>> scsi_target and its transport specific fc_remote_port. Let's expose this
>> relation and relevant information behind it.
>>
>> An example for a virsh nodedev-dumpxml:
>>
>> virsh # nodedev-dumpxml scsi_target0_0_0
>> <device>
>> <name>scsi_target0_0_0</name>
>> <path>/sys/devices/[...]/host0/rport-0:0-0/target0:0:0</path>
>> <parent>scsi_host0</parent>
>> <capability type='scsi_target'>
>> <target>target0:0:0</target>
>> <capability type='fc_remote_port'>
>> <rport>rport-0:0-0</rport>
>> <wwpn>0x9d73bc45f0e21a86</wwpn>
>> </capability>
>> </capability>
>> </device>
>>
>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
>> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
>> ---
>> docs/schemas/nodedev.rng | 20 ++++++++
>> src/conf/node_device_conf.c | 75 +++++++++++++++++++++++++++-
>> src/conf/node_device_conf.h | 7 +++
>> src/node_device/node_device_driver.c | 5 +-
>> src/node_device/node_device_linux_sysfs.c | 56 +++++++++++++++++++++
>> src/node_device/node_device_linux_sysfs.h | 2 +
>> src/node_device/node_device_udev.c | 4 +-
>> tests/nodedevschemadata/scsi_target1_0_0.xml | 12 +++++
>> tests/nodedevxml2xmltest.c | 1 +
>> 9 files changed, 178 insertions(+), 4 deletions(-)
>> create mode 100644 tests/nodedevschemadata/scsi_target1_0_0.xml
>>
>> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
>> index 87bfb0c28..5bcf31787 100644
>> --- a/docs/schemas/nodedev.rng
>> +++ b/docs/schemas/nodedev.rng
>> @@ -458,6 +458,20 @@
>> </optional>
>> </define>
>>
>> + <define name='capsfcrport'>
>> + <attribute name='type'>
>> + <value>fc_remote_port</value>
>> + </attribute>
>> +
>> + <element name='rport'>
>> + <text/>
>> + </element>
>> +
>> + <element name='wwpn'>
>> + <ref name='wwn'/>
>
>NB: 'wwn' is a string...
>
>> + </element>
>> + </define>
>> +
>> <define name='capscsitarget'>
>> <attribute name='type'>
>> <value>scsi_target</value>
>> @@ -466,6 +480,12 @@
>> <element name='target'>
>> <text/>
>> </element>
>> +
>> + <optional>
>> + <element name='capability'>
>> + <ref name='capsfcrport'/>
>> + </element>
>> + </optional>
>> </define>
>>
>> <define name='capscsi'>
>> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>> index 28d4907f5..ed003d37b 100644
>> --- a/src/conf/node_device_conf.c
>> +++ b/src/conf/node_device_conf.c
>> @@ -563,6 +563,16 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
>> case VIR_NODE_DEV_CAP_SCSI_TARGET:
>> virBufferEscapeString(&buf, "<target>%s</target>\n",
>> data->scsi_target.name);
>> + if (data->scsi_target.flags & VIR_NODE_DEV_CAP_FLAG_FC_RPORT) {
>> + virBufferAddLit(&buf, "<capability type='fc_remote_port'>\n");
>> + virBufferAdjustIndent(&buf, 2);
>> + virBufferAsprintf(&buf, "<rport>%s</rport>\n",
>> + data->scsi_target.rport);
>> + virBufferAsprintf(&buf, "<wwpn>0x%llx</wwpn>\n",
>> + data->scsi_target.wwpn);
>
>If you don't convert to a number, then you don't need to perform this
>conversion as it's just a %s print.
>
>> + virBufferAdjustIndent(&buf, -2);
>> + virBufferAddLit(&buf, "</capability>\n");
>> + }
>> break;
>> case VIR_NODE_DEV_CAP_SCSI:
>> virNodeDeviceCapSCSIDefFormat(&buf, data);
>> @@ -932,8 +942,10 @@ virNodeDevCapSCSITargetParseXML(xmlXPathContextPtr ctxt,
>> xmlNodePtr node,
>> virNodeDevCapSCSITargetPtr scsi_target)
>> {
>> - xmlNodePtr orignode;
>> - int ret = -1;
>> + xmlNodePtr orignode, *nodes = NULL;
>> + int ret = -1, n = 0;
>> + size_t i;
>> + char *type = NULL, *wwpn = NULL;
>>
>> orignode = ctxt->node;
>> ctxt->node = node;
>> @@ -946,10 +958,68 @@ virNodeDevCapSCSITargetParseXML(xmlXPathContextPtr ctxt,
>> goto out;
>> }
>>
>> + if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0)
>> + goto out;
>> +
>> + for (i = 0; i < n; ++i) {
>> + type = virXMLPropString(nodes[i], "type");
>> +
>> + if (!type) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("missing type for SCSI target capability for '%s'"),
>> + def->name);
>> + goto out;
>> + }
>> +
>> + if (STREQ(type, "fc_remote_port")) {
>> + xmlNodePtr orignode2;
>> +
>> + scsi_target->flags |= VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
>> +
>> + orignode2 = ctxt->node;
>> + ctxt->node = nodes[i];
>
>Like the virNodeDevCapSCSIHostParseXML I assume this only parses the
>first 'fc_remote_port'... Trying to think whether a VIR_FREE would be
>necessary here (and perhaps from SCSIHost code just in case there were
>more than one <rport> or <wwpn> provided...
>
More then one fc_remote_port per scsi_target would not make any sense in
my opinion, at least if I understood the entity relation in the kernel
correctly. The RNG only (hopefully) allows for one fc_remote_port
capability as well.
>> +
>> + if (virNodeDevCapsDefParseString("string(./rport[1])",
>> + ctxt,
>> + &scsi_target->rport) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("missing rport name for '%s'"), def->name);
>> + goto out;
>> + }
>> +
>> + if (virNodeDevCapsDefParseString("string(./wwpn[1])",
>> + ctxt, &wwpn) < 0) {
>
>If you don't do the conversion, this could just be a straight parse into
>scsi_target->wwpn
>
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("missing wwpn identifier for '%s'"),
>> + def->name);
>> + goto out;
>> + }
>> +
>> + if (virStrToLong_ullp(wwpn, NULL, 16, &scsi_target->wwpn) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("invalid wwpn identifier '%s' for '%s'"),
>> + wwpn, def->name);
>
>Why even both with this conversion?
Yeah, actually you are right. I don't ever use the numerical value of
the WWPN. Somehow in the back of my head my SCSI guru yells at me: "WWN
should be stored in memory as 64-bit integer values!". But true,
without the conversion the code will be easier.
>
>> + goto out;
>> + }
>> +
>> + ctxt->node = orignode2;
>> + } else {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("unknown SCSI target capability type '%s' for '%s'"),
>> + type, def->name);
>> + goto out;
>> + }
>> +
>> + VIR_FREE(type);
>> + VIR_FREE(wwpn);
>> + }
>> +
>> ret = 0;
>>
>> out:
>> ctxt->node = orignode;
>> + VIR_FREE(type);
>> + VIR_FREE(wwpn);
>
>If you don't do the conversion, then this VIR_FREE is unnecessary.
>
>Coverity complained that nodes was leaked, so I added a VIR_FREE(nodes);
>here.
>
>> return ret;
>> }
>>
>> @@ -2132,6 +2202,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
>> break;
>> case VIR_NODE_DEV_CAP_SCSI_TARGET:
>> VIR_FREE(data->scsi_target.name);
>> + VIR_FREE(data->scsi_target.rport);
>
>Without the conversion there's a VIR_FREE here for wwpn
>
>> break;
>> case VIR_NODE_DEV_CAP_SCSI:
>> VIR_FREE(data->scsi.type);
>> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
>> index 285841edc..f5267efda 100644
>> --- a/src/conf/node_device_conf.h
>> +++ b/src/conf/node_device_conf.h
>> @@ -93,6 +93,10 @@ typedef enum {
>> } virNodeDevSCSIHostCapFlags;
>>
>> typedef enum {
>> + VIR_NODE_DEV_CAP_FLAG_FC_RPORT = (1 << 0),
>> +} virNodeDevSCSITargetCapsFlags;
>> +
>> +typedef enum {
>> VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0),
>> VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1),
>> VIR_NODE_DEV_CAP_FLAG_PCIE = (1 << 2),
>> @@ -227,6 +231,9 @@ typedef struct _virNodeDevCapSCSITarget virNodeDevCapSCSITarget;
>> typedef virNodeDevCapSCSITarget *virNodeDevCapSCSITargetPtr;
>> struct _virNodeDevCapSCSITarget {
>> char *name;
>> + unsigned int flags; /* enum virNodeDevSCSITargetCapsFlags */
>> + char *rport;
>> + unsigned long long wwpn;
>
>and this would be a char *wwpn;
>
>> };
>>
>> typedef struct _virNodeDevCapSCSI virNodeDevCapSCSI;
>> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
>> index 286af26bc..acb8b89af 100644
>> --- a/src/node_device/node_device_driver.c
>> +++ b/src/node_device/node_device_driver.c
>> @@ -56,6 +56,10 @@ static int update_caps(virNodeDeviceObjPtr dev)
>> case VIR_NODE_DEV_CAP_SCSI_HOST:
>> nodeDeviceSysfsGetSCSIHostCaps(&cap->data.scsi_host);
>> break;
>> + case VIR_NODE_DEV_CAP_SCSI_TARGET:
>> + nodeDeviceSysfsGetSCSITargetCaps(dev->def->sysfs_path,
>> + &cap->data.scsi_target);
>> + break;
>> case VIR_NODE_DEV_CAP_NET:
>> if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0)
>> return -1;
>> @@ -76,7 +80,6 @@ static int update_caps(virNodeDeviceObjPtr dev)
>> case VIR_NODE_DEV_CAP_SYSTEM:
>> case VIR_NODE_DEV_CAP_USB_DEV:
>> case VIR_NODE_DEV_CAP_USB_INTERFACE:
>> - case VIR_NODE_DEV_CAP_SCSI_TARGET:
>> case VIR_NODE_DEV_CAP_SCSI:
>> case VIR_NODE_DEV_CAP_STORAGE:
>> case VIR_NODE_DEV_CAP_FC_HOST:
>> diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
>> index 1b7aa9435..90452fb29 100644
>> --- a/src/node_device/node_device_linux_sysfs.c
>> +++ b/src/node_device/node_device_linux_sysfs.c
>> @@ -26,11 +26,13 @@
>> #include <sys/stat.h>
>> #include <stdlib.h>
>>
>> +#include "dirname.h"
>> #include "node_device_driver.h"
>> #include "node_device_hal.h"
>> #include "node_device_linux_sysfs.h"
>> #include "virerror.h"
>> #include "viralloc.h"
>> +#include "virfcp.h"
>> #include "virlog.h"
>> #include "virfile.h"
>> #include "virscsihost.h"
>> @@ -124,6 +126,54 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host)
>> }
>>
>>
>> +int
>> +nodeDeviceSysfsGetSCSITargetCaps(const char *sysfsPath,
>> + virNodeDevCapSCSITargetPtr scsi_target)
>> +{
>> + int ret = -1;
>> + char *dir = NULL, *rport = NULL, *wwpn = NULL;
>> +
>> + VIR_DEBUG("Checking if '%s' is an FC remote port", scsi_target->name);
>> +
>> + /* /sys/devices/[...]/host0/rport-0:0-0/target0:0:0 -> rport-0:0-0 */
>> + if (!(dir = mdir_name(sysfsPath)))
>> + return -1;
>> +
>> + if (VIR_STRDUP(rport, last_component(dir)) < 0)
>> + goto cleanup;
>> +
>> + if (!virFCIsCapableRport(rport))
>> + goto cleanup;
>> +
>> + VIR_FREE(scsi_target->rport);
>> + scsi_target->rport = rport;
>
> VIR_STEAL_PTR(scsi_target->rport, rport);
>
Yeah, that's better.
>> +
>> + if (virFCReadRportValue(scsi_target->rport, "port_name", &wwpn) < 0) {
>> + VIR_WARN("Failed to read port_name for '%s'", scsi_target->rport);
>> + goto cleanup;
>> + }
>> +
>> + if (virStrToLong_ullp(wwpn, NULL, 16, &scsi_target->wwpn) < 0) {
>> + VIR_WARN("Failed to parse value of port_name '%s'", wwpn);
>> + goto cleanup;
>> + }
>> +
>> + scsi_target->flags |= VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
>> + ret = 0;
>> +
>> + cleanup:
>> + if (ret < 0) {
>> + VIR_FREE(scsi_target->rport);
>> + scsi_target->wwpn = 0;
>> + scsi_target->flags &= ~VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
>> + }
>
>Coverity complained that rport could be leaked, so I added the
>VIR_STEAL_PTR above and a VIR_FREE(rport); here.
>
>> + VIR_FREE(dir);
>> + VIR_FREE(wwpn);
>> +
>> + return ret;
>> +}
>> +
>> +
>> static int
>> nodeDeviceSysfsGetPCISRIOVCaps(const char *sysfsPath,
>> virNodeDevCapPCIDevPtr pci_dev)
>> @@ -230,6 +280,12 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host ATTRIBUTE_UNUS
>> return -1;
>> }
>>
>
>Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks.
>
>I can make the suggested changes if you'd like or you can send a v2 or a
>squash diff... Let me know
>
>John
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
>
--
IBM Systems
Linux on z Systems & Virtualization Development
------------------------------------------------------------------------
IBM Deutschland
Schönaicher Str. 220
71032 Böblingen
Phone: +49 7031 16 1819
E-Mail: bwalk@de.ibm.com
------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.