[libvirt] [PATCH 0/7] extend node_device for CCW and fc_remote_port
Let's introduce two new capabilities to the node_device driver and fix two smaller issues on the way. The first is for CCW devices, most common on the S390 architecture. We expose the relevant cssid, ssid and devno values. The second is for fibre channel-backed SCSI devices, where we introduce the fc_remote_port subcapability for SCSI targets. Here we expose the relevant rport name as well as the port_name (WWPN). Bjoern Walk (5): node_device: detect CCW devices virsh: nodedev: ability to filter CCW capabilities util: helper functions for fibre channel devices node_device: introduce new capability FC_RPORT docs: update news.xml Marc Hartmayer (2): node_device: Use the iterator variable node_device: Unlock obj in case of an error too docs/formatnode.html.in | 12 ++ docs/news.xml | 11 ++ docs/schemas/basictypes.rng | 31 +++++ docs/schemas/domaincommon.rng | 30 ----- docs/schemas/nodedev.rng | 36 ++++++ include/libvirt/libvirt-nodedev.h | 1 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/node_device_conf.c | 150 +++++++++++++++++++++- src/conf/node_device_conf.h | 21 ++- src/conf/virnodedeviceobj.c | 3 +- src/libvirt-nodedev.c | 1 + src/libvirt_private.syms | 5 + src/node_device/node_device_driver.c | 16 ++- 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 | 39 +++++- src/util/virfcp.c | 96 ++++++++++++++ src/util/virfcp.h | 32 +++++ tests/nodedevschemadata/ccw_0_0_10000-invalid.xml | 10 ++ tests/nodedevschemadata/ccw_0_0_ffff.xml | 10 ++ tests/nodedevschemadata/scsi_target1_0_0.xml | 12 ++ tests/nodedevxml2xmltest.c | 2 + tools/virsh-nodedev.c | 3 + tools/virsh.pod | 2 +- 25 files changed, 539 insertions(+), 44 deletions(-) create mode 100644 src/util/virfcp.c create mode 100644 src/util/virfcp.h create mode 100644 tests/nodedevschemadata/ccw_0_0_10000-invalid.xml create mode 100644 tests/nodedevschemadata/ccw_0_0_ffff.xml create mode 100644 tests/nodedevschemadata/scsi_target1_0_0.xml -- 2.11.2
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> As the switch statement checks data.type of the iterator variable @cap it must use this variable for the update too. Suggested-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/node_device/node_device_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index ba3da6288..002c7616f 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -54,7 +54,7 @@ static int update_caps(virNodeDeviceObjPtr dev) while (cap) { switch (cap->data.type) { case VIR_NODE_DEV_CAP_SCSI_HOST: - nodeDeviceSysfsGetSCSIHostCaps(&dev->def->caps->data.scsi_host); + nodeDeviceSysfsGetSCSIHostCaps(&cap->data.scsi_host); break; case VIR_NODE_DEV_CAP_NET: if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0) @@ -65,7 +65,7 @@ static int update_caps(virNodeDeviceObjPtr dev) break; case VIR_NODE_DEV_CAP_PCI_DEV: if (nodeDeviceSysfsGetPCIRelatedDevCaps(dev->def->sysfs_path, - &dev->def->caps->data.pci_dev) < 0) + &cap->data.pci_dev) < 0) return -1; break; -- 2.11.2
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
As the switch statement checks data.type of the iterator variable @cap it must use this variable for the update too.
s/must/can... I can adjust the commit message before pushing though, so no need for a v2...
Suggested-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/node_device/node_device_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John (at least I understand why you jumped on my other series now ;-))
John Ferlan <jferlan@redhat.com> [2017-05-25, 02:37PM -0400]:
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
As the switch statement checks data.type of the iterator variable @cap it must use this variable for the update too.
s/must/can...
I can adjust the commit message before pushing though, so no need for a v2...
Suggested-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/node_device/node_device_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks alot.
John
(at least I understand why you jumped on my other series now ;-))
Yeah, kind of 'one hand washes the other' or something. -- 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
On Thu, May 25, 2017 at 08:37 PM +0200, John Ferlan <jferlan@redhat.com> wrote:
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
As the switch statement checks data.type of the iterator variable @cap it must use this variable for the update too.
s/must/can...
I can adjust the commit message before pushing though, so no need for a v2...
Thanks.
Suggested-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/node_device/node_device_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
(at least I understand why you jumped on my other series now ;-))
-- Beste Grüße / Kind regards Marc Hartmayer 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
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Unlock @obj in case of an error too. Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/node_device/node_device_driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 002c7616f..74507c214 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -282,7 +282,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, STREQ(cap->data.scsi_host.wwpn, wwpn)) { if (virNodeDeviceLookupSCSIHostByWWNEnsureACL(conn, obj->def) < 0) - goto out; + goto error; if ((dev = virGetNodeDevice(conn, obj->def->name))) { if (VIR_STRDUP(dev->parent, obj->def->parent) < 0) @@ -302,6 +302,10 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, out: nodeDeviceUnlock(); return dev; + + error: + virNodeDeviceObjUnlock(obj); + goto out; } -- 2.11.2
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Unlock @obj in case of an error too.
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/node_device/node_device_driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John A patch beyond one I've posted also caught this, but I must not have been thinking about pulling the unlock out for earlier patch...
Make CCW devices available to the node_device driver. The devices are already seen by udev so let's implement necessary code for detecting them properly. Topologically, CCW devices are similar to PCI devices, e.g.: +- ccw_0_0_1a2b | +- scsi_host0 | +- scsi_target0_0_0 | +- scsi_0_0_0_0 Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/schemas/basictypes.rng | 31 ++++++++++ docs/schemas/domaincommon.rng | 30 --------- docs/schemas/nodedev.rng | 16 +++++ src/conf/node_device_conf.c | 75 ++++++++++++++++++++++- src/conf/node_device_conf.h | 10 +++ src/node_device/node_device_driver.c | 1 + src/node_device/node_device_udev.c | 35 ++++++++++- tests/nodedevschemadata/ccw_0_0_10000-invalid.xml | 10 +++ tests/nodedevschemadata/ccw_0_0_ffff.xml | 10 +++ tests/nodedevxml2xmltest.c | 1 + tools/virsh-nodedev.c | 2 + 11 files changed, 188 insertions(+), 33 deletions(-) create mode 100644 tests/nodedevschemadata/ccw_0_0_10000-invalid.xml create mode 100644 tests/nodedevschemadata/ccw_0_0_ffff.xml diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index cc560e66f..1ea667cdf 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -318,6 +318,37 @@ </data> </define> + <define name="ccwCssidRange"> + <choice> + <data type="string"> + <param name="pattern">0x[0-9a-eA-E][0-9a-fA-F]?</param> + </data> + <data type="string"> + <param name="pattern">0x[fF][0-9a-eA-E]?</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">254</param> + </data> + </choice> + </define> + <define name="ccwSsidRange"> + <data type="string"> + <param name="pattern">(0x)?[0-3]</param> + </data> + </define> + <define name="ccwDevnoRange"> + <choice> + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{1,4}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">65535</param> + </data> + </choice> + </define> + <define name="cpuset"> <data type="string"> <param name="pattern">([0-9]+(-[0-9]+)?|\^[0-9]+)(,([0-9]+(-[0-9]+)?|\^[0-9]+))*</param> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f88e84ab6..94de10749 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5770,36 +5770,6 @@ </element> <empty/> </define> - <define name="ccwCssidRange"> - <choice> - <data type="string"> - <param name="pattern">0x[0-9a-eA-E][0-9a-fA-F]?</param> - </data> - <data type="string"> - <param name="pattern">0x[fF][0-9a-eA-E]?</param> - </data> - <data type="int"> - <param name="minInclusive">0</param> - <param name="maxInclusive">254</param> - </data> - </choice> - </define> - <define name="ccwSsidRange"> - <data type="string"> - <param name="pattern">(0x)?[0-3]</param> - </data> - </define> - <define name="ccwDevnoRange"> - <choice> - <data type="string"> - <param name="pattern">0x[0-9a-fA-F]{1,4}</param> - </data> - <data type="int"> - <param name="minInclusive">0</param> - <param name="maxInclusive">65535</param> - </data> - </choice> - </define> <define name="panic"> <element name="panic"> <optional> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 924f73861..87bfb0c28 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -84,6 +84,7 @@ <ref name="capstorage"/> <ref name="capdrm"/> <ref name="capmdev"/> + <ref name="capccwdev"/> </choice> </element> </define> @@ -597,6 +598,21 @@ </element> </define> + <define name='capccwdev'> + <attribute name='type'> + <value>ccw</value> + </attribute> + <element name='cssid'> + <ref name='ccwCssidRange'/> + </element> + <element name='ssid'> + <ref name='ccwSsidRange'/> + </element> + <element name='devno'> + <ref name='ccwDevnoRange'/> + </element> + </define> + <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 9cb63860f..28d4907f5 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -62,7 +62,8 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST, "scsi_generic", "drm", "mdev_types", - "mdev") + "mdev", + "ccw") VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", @@ -581,6 +582,14 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferAsprintf(&buf, "<iommuGroup number='%u'/>\n", data->mdev.iommuGroupNumber); break; + case VIR_NODE_DEV_CAP_CCW_DEV: + virBufferAsprintf(&buf, "<cssid>0x%x</cssid>\n", + data->ccw_dev.cssid); + virBufferAsprintf(&buf, "<ssid>0x%x</ssid>\n", + data->ccw_dev.ssid); + virBufferAsprintf(&buf, "<devno>0x%04x</devno>\n", + data->ccw_dev.devno); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -718,6 +727,66 @@ virNodeDevCapDRMParseXML(xmlXPathContextPtr ctxt, static int +virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def, + xmlNodePtr node, + virNodeDevCapCCWPtr ccw_dev) +{ + xmlNodePtr orignode; + int ret = -1; + char *cssid = NULL, *ssid = NULL, *devno = NULL; + + orignode = ctxt->node; + ctxt->node = node; + + if (!(cssid = virXPathString("string(./cssid[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing cssid value for '%s'"), def->name); + goto out; + } + + if (virStrToLong_uip(cssid, NULL, 0, &ccw_dev->cssid) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid cssid value '%s' for '%s'"), + cssid, def->name); + goto out; + } + + if (!(ssid = virXPathString("string(./ssid[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing ssid value for '%s'"), def->name); + goto out; + } + + if (virStrToLong_uip(ssid, NULL, 0, &ccw_dev->ssid) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid ssid value '%s' for '%s'"), + cssid, def->name); + goto out; + } + + if (!(devno = virXPathString("string(./devno[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing devno value for '%s'"), def->name); + goto out; + } + + if (virStrToLong_uip(devno, NULL, 16, &ccw_dev->devno) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid devno value '%s' for '%s'"), + devno, def->name); + goto out; + } + + ret = 0; + + out: + ctxt->node = orignode; + return ret; +} + + +static int virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, @@ -1754,6 +1823,9 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, case VIR_NODE_DEV_CAP_MDEV: ret = virNodeDevCapMdevParseXML(ctxt, def, node, &caps->data.mdev); break; + case VIR_NODE_DEV_CAP_CCW_DEV: + ret = virNodeDevCapCCWParseXML(ctxt, def, node, &caps->data.ccw_dev); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -2083,6 +2155,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_DRM: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: + case VIR_NODE_DEV_CAP_CCW_DEV: case VIR_NODE_DEV_CAP_LAST: /* This case is here to shutup the compiler */ break; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 5743f9d3e..bf9d5fce5 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -66,6 +66,7 @@ typedef enum { VIR_NODE_DEV_CAP_DRM, /* DRM device */ VIR_NODE_DEV_CAP_MDEV_TYPES, /* Device capable of mediated devices */ VIR_NODE_DEV_CAP_MDEV, /* Mediated device */ + VIR_NODE_DEV_CAP_CCW_DEV, /* s390 CCW device */ VIR_NODE_DEV_CAP_LAST } virNodeDevCapType; @@ -267,6 +268,14 @@ struct _virNodeDevCapDRM { virNodeDevDRMType type; }; +typedef struct _virNodeDevCapCCW virNodeDevCapCCW; +typedef virNodeDevCapCCW *virNodeDevCapCCWPtr; +struct _virNodeDevCapCCW { + unsigned int cssid; + unsigned int ssid; + unsigned int devno; +}; + typedef struct _virNodeDevCapData virNodeDevCapData; typedef virNodeDevCapData *virNodeDevCapDataPtr; struct _virNodeDevCapData { @@ -284,6 +293,7 @@ struct _virNodeDevCapData { virNodeDevCapSCSIGeneric sg; virNodeDevCapDRM drm; virNodeDevCapMdev mdev; + virNodeDevCapCCW ccw_dev; }; }; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 74507c214..286af26bc 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -84,6 +84,7 @@ static int update_caps(virNodeDeviceObjPtr dev) case VIR_NODE_DEV_CAP_SCSI_GENERIC: case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_MDEV: + case VIR_NODE_DEV_CAP_CCW_DEV: case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 4ecb0b18f..7744c2637 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1105,6 +1105,33 @@ udevProcessMediatedDevice(struct udev_device *dev, } static int +udevProcessCCW(struct udev_device *device, virNodeDeviceDefPtr def) +{ + int online; + char *p; + virNodeDevCapDataPtr data = &def->caps->data; + + /* process only online devices to keep the list sane */ + if (udevGetIntSysfsAttr(device, "online", &online, 0) < 0 || online != 1) + return -1; + + if ((p = strrchr(def->sysfs_path, '/')) == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.cssid) < 0 || p == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.ssid) < 0 || p == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.devno) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse the CCW address from sysfs path: '%s'"), + def->sysfs_path); + return -1; + } + + if (udevGenerateDeviceName(device, def, NULL) != 0) + return -1; + + return 0; +} + +static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) { @@ -1172,8 +1199,8 @@ udevGetDeviceType(struct udev_device *device, if (udevHasDeviceProperty(device, "INTERFACE")) *type = VIR_NODE_DEV_CAP_NET; - /* Neither SCSI generic devices nor mediated devices set DEVTYPE - * property, therefore we need to rely on the SUBSYSTEM property */ + /* The following devices do not set the DEVTYPE property, therefore + * we need to rely on the SUBSYSTEM property */ if (udevGetStringProperty(device, "SUBSYSTEM", &subsystem) < 0) return -1; @@ -1181,6 +1208,8 @@ udevGetDeviceType(struct udev_device *device, *type = VIR_NODE_DEV_CAP_SCSI_GENERIC; else if (STREQ_NULLABLE(subsystem, "mdev")) *type = VIR_NODE_DEV_CAP_MDEV; + else if (STREQ_NULLABLE(subsystem, "ccw")) + *type = VIR_NODE_DEV_CAP_CCW_DEV; VIR_FREE(subsystem); } @@ -1222,6 +1251,8 @@ static int udevGetDeviceDetails(struct udev_device *device, return udevProcessDRMDevice(device, def); case VIR_NODE_DEV_CAP_MDEV: return udevProcessMediatedDevice(device, def); + case VIR_NODE_DEV_CAP_CCW_DEV: + return udevProcessCCW(device, def); case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml new file mode 100644 index 000000000..d840555c0 --- /dev/null +++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml @@ -0,0 +1,10 @@ +<device> + <name>ccw_0_0_10000</name> + <path>/sys/devices/css0/0.0.0000/0.0.10000</path> + <parent>computer</parent> + <capability type='ccw'> + <cssid>0x0</cssid> + <ssid>0x0</ssid> + <devno>0x10000</devno> + </capability> +</device> diff --git a/tests/nodedevschemadata/ccw_0_0_ffff.xml b/tests/nodedevschemadata/ccw_0_0_ffff.xml new file mode 100644 index 000000000..5ecd0b0aa --- /dev/null +++ b/tests/nodedevschemadata/ccw_0_0_ffff.xml @@ -0,0 +1,10 @@ +<device> + <name>ccw_0_0_ffff</name> + <path>/sys/devices/css0/0.0.0000/0.0.ffff</path> + <parent>computer</parent> + <capability type='ccw'> + <cssid>0x0</cssid> + <ssid>0x0</ssid> + <devno>0xffff</devno> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index a2aad518d..805deef26 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -103,6 +103,7 @@ mymain(void) DO_TEST("drm_renderD129"); DO_TEST("pci_0000_02_10_7_mdev_types"); DO_TEST("mdev_3627463d_b7f0_4fea_b468_f1da537d301b"); + DO_TEST("ccw_0_0_ffff"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index ad96dda1f..1822d3dce 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -460,6 +460,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) case VIR_NODE_DEV_CAP_MDEV: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV; break; + case VIR_NODE_DEV_CAP_CCW_DEV: + /* enable next patch */ case VIR_NODE_DEV_CAP_LAST: break; } -- 2.11.2
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
Make CCW devices available to the node_device driver. The devices are already seen by udev so let's implement necessary code for detecting them properly.
Topologically, CCW devices are similar to PCI devices, e.g.:
+- ccw_0_0_1a2b | +- scsi_host0 | +- scsi_target0_0_0 | +- scsi_0_0_0_0
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/schemas/basictypes.rng | 31 ++++++++++ docs/schemas/domaincommon.rng | 30 --------- docs/schemas/nodedev.rng | 16 +++++ src/conf/node_device_conf.c | 75 ++++++++++++++++++++++- src/conf/node_device_conf.h | 10 +++ src/node_device/node_device_driver.c | 1 + src/node_device/node_device_udev.c | 35 ++++++++++- tests/nodedevschemadata/ccw_0_0_10000-invalid.xml | 10 +++ tests/nodedevschemadata/ccw_0_0_ffff.xml | 10 +++ tests/nodedevxml2xmltest.c | 1 + tools/virsh-nodedev.c | 2 + 11 files changed, 188 insertions(+), 33 deletions(-) create mode 100644 tests/nodedevschemadata/ccw_0_0_10000-invalid.xml create mode 100644 tests/nodedevschemadata/ccw_0_0_ffff.xml
...
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 9cb63860f..28d4907f5 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c
...
static int +virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def, + xmlNodePtr node, + virNodeDevCapCCWPtr ccw_dev) +{ + xmlNodePtr orignode; + int ret = -1; + char *cssid = NULL, *ssid = NULL, *devno = NULL; + + orignode = ctxt->node; + ctxt->node = node; + + if (!(cssid = virXPathString("string(./cssid[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing cssid value for '%s'"), def->name); + goto out; + } + + if (virStrToLong_uip(cssid, NULL, 0, &ccw_dev->cssid) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid cssid value '%s' for '%s'"), + cssid, def->name); + goto out; + } + + if (!(ssid = virXPathString("string(./ssid[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing ssid value for '%s'"), def->name); + goto out; + } + + if (virStrToLong_uip(ssid, NULL, 0, &ccw_dev->ssid) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid ssid value '%s' for '%s'"), + cssid, def->name); + goto out; + } + + if (!(devno = virXPathString("string(./devno[1])", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing devno value for '%s'"), def->name); + goto out; + } + + if (virStrToLong_uip(devno, NULL, 16, &ccw_dev->devno) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid devno value '%s' for '%s'"), + devno, def->name); + goto out; + }
One would hope they're in range, but since the rng had ranges should you check here similar to what virDomainDeviceCCWAddressIsValid does? It's fine this way since this is essentially reporting from udev which one can only assume (haha) would do validation...
+ + ret = 0; + + out: + ctxt->node = orignode; + return ret; +} + +
...
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 4ecb0b18f..7744c2637 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1105,6 +1105,33 @@ udevProcessMediatedDevice(struct udev_device *dev, }
static int +udevProcessCCW(struct udev_device *device, virNodeDeviceDefPtr def)
Although you're following the module syntax, this should follow current practices for multilines and spacing before/after function... I can adjust that before pushing if this is all that's necessary though.
+{ + int online; + char *p; + virNodeDevCapDataPtr data = &def->caps->data; + + /* process only online devices to keep the list sane */ + if (udevGetIntSysfsAttr(device, "online", &online, 0) < 0 || online != 1) + return -1; + + if ((p = strrchr(def->sysfs_path, '/')) == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.cssid) < 0 || p == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.ssid) < 0 || p == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.devno) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse the CCW address from sysfs path: '%s'"), + def->sysfs_path); + return -1; + } + + if (udevGenerateDeviceName(device, def, NULL) != 0) + return -1; + + return 0; +} + +static int
...
diff --git a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml new file mode 100644 index 000000000..d840555c0 --- /dev/null +++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml @@ -0,0 +1,10 @@ +<device> + <name>ccw_0_0_10000</name> + <path>/sys/devices/css0/0.0.0000/0.0.10000</path> + <parent>computer</parent> + <capability type='ccw'> + <cssid>0x0</cssid> + <ssid>0x0</ssid> + <devno>0x10000</devno> + </capability> +</device>
I assume you planned to use this, but either forgot or didn't want to write the EXPECT_FAIL test? Should it be removed from the patch? Reviewed-by: John Ferlan <jferlan@redhat.com> FWIW: I can make adjustments if you'd like or you can provide a v2 of this patch. Just let me know John ...
John Ferlan <jferlan@redhat.com> [2017-05-25, 03:05PM -0400]:
One would hope they're in range, but since the rng had ranges should you check here similar to what virDomainDeviceCCWAddressIsValid does?
It's fine this way since this is essentially reporting from udev which one can only assume (haha) would do validation...
True, yes, for validation we only rely on the rng. I actually wanted to avoid any complicated error checking here for much easier code.
+ + ret = 0; + + out: + ctxt->node = orignode; + return ret; +} + +
...
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 4ecb0b18f..7744c2637 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1105,6 +1105,33 @@ udevProcessMediatedDevice(struct udev_device *dev, }
static int +udevProcessCCW(struct udev_device *device, virNodeDeviceDefPtr def)
Although you're following the module syntax, this should follow current practices for multilines and spacing before/after function... I can adjust that before pushing if this is all that's necessary though.
Ok, will remember next time.
+{ + int online; + char *p; + virNodeDevCapDataPtr data = &def->caps->data; + + /* process only online devices to keep the list sane */ + if (udevGetIntSysfsAttr(device, "online", &online, 0) < 0 || online != 1) + return -1; + + if ((p = strrchr(def->sysfs_path, '/')) == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.cssid) < 0 || p == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.ssid) < 0 || p == NULL || + virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.devno) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse the CCW address from sysfs path: '%s'"), + def->sysfs_path); + return -1; + } + + if (udevGenerateDeviceName(device, def, NULL) != 0) + return -1; + + return 0; +} + +static int
...
diff --git a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml new file mode 100644 index 000000000..d840555c0 --- /dev/null +++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml @@ -0,0 +1,10 @@ +<device> + <name>ccw_0_0_10000</name> + <path>/sys/devices/css0/0.0.0000/0.0.10000</path> + <parent>computer</parent> + <capability type='ccw'> + <cssid>0x0</cssid> + <ssid>0x0</ssid> + <devno>0x10000</devno> + </capability> +</device>
I assume you planned to use this, but either forgot or didn't want to write the EXPECT_FAIL test?
Since we don't perform any validation in the code, a test would never actually fail. But this XML is implicitly tested by virschematest so I thought at least this is covered.
Should it be removed from the patch?
Depends on if we actually want the validation and/or if the test against the RNG schema is enough.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks anyways.
FWIW: I can make adjustments if you'd like or you can provide a v2 of this patch. Just 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
...
...
diff --git a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml new file mode 100644 index 000000000..d840555c0 --- /dev/null +++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml @@ -0,0 +1,10 @@ +<device> + <name>ccw_0_0_10000</name> + <path>/sys/devices/css0/0.0.0000/0.0.10000</path> + <parent>computer</parent> + <capability type='ccw'> + <cssid>0x0</cssid> + <ssid>0x0</ssid> + <devno>0x10000</devno> + </capability> +</device>
I assume you planned to use this, but either forgot or didn't want to write the EXPECT_FAIL test?
Since we don't perform any validation in the code, a test would never actually fail. But this XML is implicitly tested by virschematest so I thought at least this is covered.
Should it be removed from the patch?
Depends on if we actually want the validation and/or if the test against the RNG schema is enough.
oh right - for some reason I had xml2xml type checking - I'll leave it though. John
Now that the node_device driver is aware of CCW devices, let's hook up virsh so that we can filter them properly. Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/formatnode.html.in | 12 ++++++++++++ include/libvirt/libvirt-nodedev.h | 1 + src/conf/node_device_conf.h | 3 ++- src/conf/virnodedeviceobj.c | 3 ++- src/libvirt-nodedev.c | 1 + tools/virsh-nodedev.c | 3 ++- tools/virsh.pod | 2 +- 7 files changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index a368ffc07..32451d557 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -324,6 +324,18 @@ <code>render</code>.</dd> </dl> </dd> + <dt><code>ccw</code></dt> + <dd>Describes a Command Channel Word (CCW) device commonly found on + the S390 architecture. Sub-elements include: + <dl> + <dt><code>cssid</code></dt> + <dd>The channel subsystem identifier.</dd> + <dt><code>ssid</code></dt> + <dd>The subchannel-set identifier.</dd> + <dt><code>devno</code></dt> + <dd>The device number.</dd> + </dl> + </dd> </dl> </dd> </dl> diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index 1e3043787..cd3f2372b 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -81,6 +81,7 @@ typedef enum { VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM = 1 << 12, /* DRM device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES = 1 << 13, /* Capable of mediated devices */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV = 1 << 14, /* Mediated device */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV = 1 << 15, /* CCW device */ } virConnectListAllNodeDeviceFlags; int virConnectListAllNodeDevices (virConnectPtr conn, diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index bf9d5fce5..285841edc 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -388,7 +388,8 @@ virNodeDevCapMdevTypeFree(virNodeDevCapMdevTypePtr type); VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES | \ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV) + VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV | \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV) char * virNodeDeviceGetParentName(virConnectPtr conn, diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 02ac54437..d460f26ec 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -568,7 +568,8 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj, MATCH(SCSI_GENERIC) || MATCH(DRM) || MATCH(MDEV_TYPES) || - MATCH(MDEV))) + MATCH(MDEV) || + MATCH(CCW_DEV))) return false; } diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index 44e2b4efd..f85c16bbc 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -100,6 +100,7 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags) * VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM * VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES * VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV + * VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV * * Returns the number of node devices found or -1 and sets @devices to NULL in * case of error. On success, the array stored into @devices is guaranteed to diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 1822d3dce..c7ef6bfde 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -461,7 +461,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV; break; case VIR_NODE_DEV_CAP_CCW_DEV: - /* enable next patch */ + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV; + break; case VIR_NODE_DEV_CAP_LAST: break; } diff --git a/tools/virsh.pod b/tools/virsh.pod index e7c513b98..aee964689 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3181,7 +3181,7 @@ I<cap> is used to filter the list by capability types, the types must be separated by comma, e.g. --cap pci,scsi. Valid capability types include 'system', 'pci', 'usb_device', 'usb', 'net', 'scsi_host', 'scsi_target', 'scsi', 'storage', 'fc_host', 'vports', 'scsi_generic', 'drm', 'mdev', -'mdev_types'. +'mdev_types', 'ccw'. If I<--tree> is used, the output is formatted in a tree representing parents of each node. I<cap> and I<--tree> are mutually exclusive. -- 2.11.2
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
Now that the node_device driver is aware of CCW devices, let's hook up virsh so that we can filter them properly.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/formatnode.html.in | 12 ++++++++++++ include/libvirt/libvirt-nodedev.h | 1 + src/conf/node_device_conf.h | 3 ++- src/conf/virnodedeviceobj.c | 3 ++- src/libvirt-nodedev.c | 1 + tools/virsh-nodedev.c | 3 ++- tools/virsh.pod | 2 +- 7 files changed, 21 insertions(+), 4 deletions(-)
...
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 02ac54437..d460f26ec 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -568,7 +568,8 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj, MATCH(SCSI_GENERIC) || MATCH(DRM) || MATCH(MDEV_TYPES) || - MATCH(MDEV))) + MATCH(MDEV) || + MATCH(CCW_DEV))) return false; }
But you didn't modify virNodeDeviceCapMatch in order perform the match that the MATCH does. Also, another well hidden gem that either Erik Skultety or myself will fix "eventually" is virNodeDeviceObjHasCap. See commit id 'e8fcac8ec' for some context. Essentially, the virNodeDeviceMatch only matches for the virNodeDeviceObjListExport API. If you want to send something to squash in or just a v2 of this patch - I can handle either. Reviewed-by: John Ferlan <jferlan@redhat.com> John
John Ferlan <jferlan@redhat.com> [2017-05-25, 03:21PM -0400]:
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
Now that the node_device driver is aware of CCW devices, let's hook up virsh so that we can filter them properly.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/formatnode.html.in | 12 ++++++++++++ include/libvirt/libvirt-nodedev.h | 1 + src/conf/node_device_conf.h | 3 ++- src/conf/virnodedeviceobj.c | 3 ++- src/libvirt-nodedev.c | 1 + tools/virsh-nodedev.c | 3 ++- tools/virsh.pod | 2 +- 7 files changed, 21 insertions(+), 4 deletions(-)
...
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 02ac54437..d460f26ec 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -568,7 +568,8 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj, MATCH(SCSI_GENERIC) || MATCH(DRM) || MATCH(MDEV_TYPES) || - MATCH(MDEV))) + MATCH(MDEV) || + MATCH(CCW_DEV))) return false; }
But you didn't modify virNodeDeviceCapMatch in order perform the match that the MATCH does.
Damn, there's a lot that you can miss with these capabilities.
Also, another well hidden gem that either Erik Skultety or myself will fix "eventually" is virNodeDeviceObjHasCap. See commit id 'e8fcac8ec' for some context. Essentially, the virNodeDeviceMatch only matches for the virNodeDeviceObjListExport API.
And even more. Thanks for the pointers.
If you want to send something to squash in or just a v2 of this patch - I can handle either.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks.
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
On 05/26/2017 03:00 AM, Bjoern Walk wrote:
John Ferlan <jferlan@redhat.com> [2017-05-25, 03:21PM -0400]:
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
Now that the node_device driver is aware of CCW devices, let's hook up virsh so that we can filter them properly.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/formatnode.html.in | 12 ++++++++++++ include/libvirt/libvirt-nodedev.h | 1 + src/conf/node_device_conf.h | 3 ++- src/conf/virnodedeviceobj.c | 3 ++- src/libvirt-nodedev.c | 1 + tools/virsh-nodedev.c | 3 ++- tools/virsh.pod | 2 +- 7 files changed, 21 insertions(+), 4 deletions(-)
...
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 02ac54437..d460f26ec 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -568,7 +568,8 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj, MATCH(SCSI_GENERIC) || MATCH(DRM) || MATCH(MDEV_TYPES) || - MATCH(MDEV))) + MATCH(MDEV) || + MATCH(CCW_DEV))) return false; }
But you didn't modify virNodeDeviceCapMatch in order perform the match that the MATCH does.
Damn, there's a lot that you can miss with these capabilities.
I sent a patch which should cover this for the future: https://www.redhat.com/archives/libvir-list/2017-May/msg01003.html
Also, another well hidden gem that either Erik Skultety or myself will fix "eventually" is virNodeDeviceObjHasCap. See commit id 'e8fcac8ec' for some context. Essentially, the virNodeDeviceMatch only matches for the virNodeDeviceObjListExport API.
And even more. Thanks for the pointers.
In rethinking this - it doesn't necessarily seem as though this device would have the same issues as NPIV (a SCSI_HOST device parent that has capabilities of either "fc_host" or "vports" to describe a capable child) or MDEV (a PCI device that a parent mediated device with "mdev" capabilities to describe the children). As long as you're comfortable that the match can be done - I'm good. The mdev changes were recently reviewed by me so it was fresh in my mind. John
If you want to send something to squash in or just a v2 of this patch - I can handle either.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks.
John
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
We will need some convenient helper functions for managing sysfs-entries for fibre channel-backed devices. Let's implement them and make them available in the private API. Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 5 +++ src/util/virfcp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfcp.h | 34 +++++++++++++++++ 5 files changed, 139 insertions(+) create mode 100644 src/util/virfcp.c create mode 100644 src/util/virfcp.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 5077857b0..142b4d337 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -206,6 +206,7 @@ src/util/virdnsmasq.c src/util/virerror.c src/util/virerror.h src/util/vireventpoll.c +src/util/virfcp.c src/util/virfdstream.c src/util/virfile.c src/util/virfirewall.c diff --git a/src/Makefile.am b/src/Makefile.am index f95f30e2f..0b7cfc89a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -117,6 +117,7 @@ UTIL_SOURCES = \ util/virerror.c util/virerror.h \ util/virevent.c util/virevent.h \ util/vireventpoll.c util/vireventpoll.h \ + util/virfcp.c util/virfcp.h \ util/virfdstream.c util/virfdstream.h \ util/virfile.c util/virfile.h \ util/virfirewall.c util/virfirewall.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d361454d5..429b095bb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1586,6 +1586,11 @@ virEventPollUpdateHandle; virEventPollUpdateTimeout; +# util/virfcp.h +virFCIsCapableRport; +virFCReadRportValue; + + # util/virfdstream.h virFDStreamConnectUNIX; virFDStreamCreateFile; diff --git a/src/util/virfcp.c b/src/util/virfcp.c new file mode 100644 index 000000000..3c7fab917 --- /dev/null +++ b/src/util/virfcp.c @@ -0,0 +1,98 @@ +/* + * virfcp.c: Utility functions for the Fibre Channel Protocol + * + * Copyright (C) 2017 IBM Corporation + * + * 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/>. + * + * Author: Bjoern Walk <bwalk@linux.vnet.ibm.com> + */ + +#include <config.h> + +#include "internal.h" + +#include "viralloc.h" +#include "virfile.h" +#include "virstring.h" + +#include "virfcp.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +#ifdef __linux__ + +# define SYSFS_FC_RPORT_PATH "/sys/class/fc_remote_ports" + +bool +virFCIsCapableRport(const char *rport) +{ + bool ret = false; + char *path = NULL; + + if (virBuildPath(&path, SYSFS_FC_RPORT_PATH, rport) < 0) + return false; + + ret = virFileExists(path); + VIR_FREE(path); + + return ret; +} + +int +virFCReadRportValue(const char *rport, + const char *entry, + char **result) +{ + int ret = -1; + char *buf = NULL, *p = NULL; + + if (virFileReadValueString(&buf, "%s/%s/%s", + SYSFS_FC_RPORT_PATH, rport, entry) < 0) { + return -1; + } + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + + if (VIR_STRDUP(*result, buf) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(buf); + return ret; +} + +#else + +bool +virSysfsIsCapableFCRport(const char *rport ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return false; +} + +int +virSysfsReadFCRport(const char *rport ATTRIBUTE_UNUSED, + const char *entry ATTRIBUTE_UNUSED, + char **result ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + +#endif diff --git a/src/util/virfcp.h b/src/util/virfcp.h new file mode 100644 index 000000000..2bc05387a --- /dev/null +++ b/src/util/virfcp.h @@ -0,0 +1,34 @@ +/* + * virfcp.h: Utility functions for the Fibre Channel Protocol + * + * Copyright (C) 2017 IBM Corporation + * + * 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/>. + * + * Author: Bjoern Walk <bwalk@linux.vnet.ibm.com> + */ + +#ifndef __VIR_FCP_H__ +# define __VIR_FCP_H__ + +bool +virFCIsCapableRport(const char *rport); + +int +virFCReadRportValue(const char *rport, + const char *entry, + char **result); + +#endif -- 2.11.2
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
We will need some convenient helper functions for managing sysfs-entries for fibre channel-backed devices. Let's implement them and make them available in the private API.
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 5 +++ src/util/virfcp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfcp.h | 34 +++++++++++++++++ 5 files changed, 139 insertions(+) create mode 100644 src/util/virfcp.c create mode 100644 src/util/virfcp.h
Reviewed-by: John Ferlan <jferlan@redhat.com> John
John Ferlan <jferlan@redhat.com> [2017-05-25, 03:23PM -0400]:
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
We will need some convenient helper functions for managing sysfs-entries for fibre channel-backed devices. Let's implement them and make them available in the private API.
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 5 +++ src/util/virfcp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfcp.h | 34 +++++++++++++++++ 5 files changed, 139 insertions(+) create mode 100644 src/util/virfcp.c create mode 100644 src/util/virfcp.h
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks.
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
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
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
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
Mention CCW and fc_remote_port capablities in the news.xml file. Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 52915ee2e..d8c9794f9 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -25,6 +25,17 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + Additional capabilities for the node_device module + </summary> + <description> + Introduce two new capabilities to the node_device module. The first + is for CCW devices, most common on the S390 architecture. The second + is for fibre channel-backed SCSI devices and exposes the + fc_remote_port sub-capability to SCSI target devices. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.11.2
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
Mention CCW and fc_remote_port capablities in the news.xml file.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
John Ferlan <jferlan@redhat.com> [2017-05-25, 03:26PM -0400]:
On 05/22/2017 02:38 AM, Bjoern Walk wrote:
Mention CCW and fc_remote_port capablities in the news.xml file.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks.
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
Thanks a lot for your review John. If you don't mind, you can make the adjustments you mentioned and push, as I can only prepare a v2 on Monday and I wanted to get this in v3.4.0. Best, Bjoern -- 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
On 05/26/2017 03:15 AM, Bjoern Walk wrote:
Thanks a lot for your review John.
If you don't mind, you can make the adjustments you mentioned and push, as I can only prepare a v2 on Monday and I wanted to get this in v3.4.0.
Best, Bjoern
I've made the adjustments and will push this sometime today... John
On Fri, May 26, 2017 at 07:11:53AM -0400, John Ferlan wrote:
On 05/26/2017 03:15 AM, Bjoern Walk wrote:
Thanks a lot for your review John.
If you don't mind, you can make the adjustments you mentioned and push, as I can only prepare a v2 on Monday and I wanted to get this in v3.4.0.
Best, Bjoern
I've made the adjustments and will push this sometime today...
FYI this broke the build on OS-X util/virfcp.c:85:5: error: implicit declaration of function 'virReportSystemError' is invalid in C99 [-Werror,-Wimplicit-function-declaration] virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); util/virfcp.c:83:1: error: no previous prototype for function 'virSysfsIsCapableFCRport' [-Werror,-Wmissing-prototypes] virSysfsIsCapableFCRport(const char *rport ATTRIBUTE_UNUSED) util/virfcp.c:90:1: error: no previous prototype for function 'virSysfsReadFCRport' [-Werror,-Wmissing-prototypes] virSysfsReadFCRport(const char *rport ATTRIBUTE_UNUSED, https://travis-ci.org/libvirt/libvirt/jobs/236407391 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrange wrote:
On Fri, May 26, 2017 at 07:11:53AM -0400, John Ferlan wrote:
On 05/26/2017 03:15 AM, Bjoern Walk wrote:
Thanks a lot for your review John.
If you don't mind, you can make the adjustments you mentioned and push, as I can only prepare a v2 on Monday and I wanted to get this in v3.4.0.
Best, Bjoern
I've made the adjustments and will push this sometime today...
FYI this broke the build on OS-X
util/virfcp.c:85:5: error: implicit declaration of function 'virReportSystemError' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
util/virfcp.c:83:1: error: no previous prototype for function 'virSysfsIsCapableFCRport' [-Werror,-Wmissing-prototypes]
virSysfsIsCapableFCRport(const char *rport ATTRIBUTE_UNUSED)
util/virfcp.c:90:1: error: no previous prototype for function 'virSysfsReadFCRport' [-Werror,-Wmissing-prototypes]
virSysfsReadFCRport(const char *rport ATTRIBUTE_UNUSED,
https://travis-ci.org/libvirt/libvirt/jobs/236407391
Regards, Daniel
This breaks build on FreeBSD as well. I have a fix that I plan to push shortly. Roman Bogorodskiy
participants (5)
-
Bjoern Walk -
Daniel P. Berrange -
John Ferlan -
Marc Hartmayer -
Roman Bogorodskiy