6:59 a.m.
This series enables support for channel subsystem (CSS) devices,
Direct Access Storage Devices (DASDs) in the node_device driver and
adds support to create vfio-ccw mdev devices in the node_device driver.
Boris Fiuczynski (5):
node_device: refactor udevProcessCCW
node_device: detect CSS devices
virsh: nodedev: ability to filter CSS capabilities
node_device: detect DASD devices
node_device: mdev vfio-ccw support
docs/formatnode.html.in | 12 +++
docs/manpages/virsh.rst | 2 +-
docs/schemas/nodedev.rng | 16 ++++
include/libvirt/libvirt-nodedev.h | 1 +
src/conf/domain_addr.c | 2 +-
src/conf/domain_addr.h | 3 +
src/conf/node_device_conf.c | 5 ++
src/conf/node_device_conf.h | 4 +-
src/conf/virnodedeviceobj.c | 4 +-
src/libvirt-nodedev.c | 1 +
src/libvirt_private.syms | 1 +
src/node_device/node_device_driver.c | 49 +++++++++---
src/node_device/node_device_udev.c | 77 ++++++++++++++++---
.../ccw_0_0_10000-invalid.xml | 4 +-
tests/nodedevschemadata/ccw_0_0_ffff.xml | 4 +-
tests/nodedevschemadata/css_0_0_ffff.xml | 10 +++
tests/nodedevxml2xmltest.c | 1 +
tools/virsh-nodedev.c | 3 +
18 files changed, 172 insertions(+), 27 deletions(-)
create mode 100644 tests/nodedevschemadata/css_0_0_ffff.xml
--
2.24.1
6:59 a.m.
New subject: [PATCH 1/5] node_device: refactor udevProcessCCW
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Refactor out CCW address parsing for later reuse.
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/node_device/node_device_udev.c | 31 ++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index ff558efb..d478a673 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1058,27 +1058,38 @@ udevProcessMediatedDevice(struct udev_device *dev,
static int
-udevProcessCCW(struct udev_device *device,
- virNodeDeviceDefPtr def)
+udevGetCCWAddress(const char *sysfs_path,
+ virNodeDevCapDataPtr data)
{
- 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 ||
+ if ((p = strrchr(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);
+ sysfs_path);
return -1;
}
+ return 0;
+}
+
+
+static int
+udevProcessCCW(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+ int online;
+
+ /* process only online devices to keep the list sane */
+ if (udevGetIntSysfsAttr(device, "online", &online, 0) < 0 || online
!= 1)
+ return -1;
+
+ if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) < 0)
+ return -1;
+
if (udevGenerateDeviceName(device, def, NULL) != 0)
return -1;
--
2.24.1
8:58 a.m.
New subject: [PATCH 1/5] node_device: refactor udevProcessCCW
On Mon, Aug 24, 2020 at 01:59:11PM +0200, Bjoern Walk wrote:
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Refactor out CCW address parsing for later reuse.
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Reviewed-by: Erik
Skultety <eskultet(a)redhat.com>
6:59 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Make channel subsystem (CSS) devices available in the node_device driver.
The CCS devices reside in the computer system and provide CCW devices, e.g.:
+- css_0_0_003a
|
+- ccw_0_0_1a2b
|
+- scsi_host0
|
+- scsi_target0_0_0
|
+- scsi_0_0_0_0
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
docs/schemas/nodedev.rng | 16 ++++++++++++++
src/conf/node_device_conf.c | 5 +++++
src/conf/node_device_conf.h | 1 +
src/conf/virnodedeviceobj.c | 1 +
src/node_device/node_device_udev.c | 22 +++++++++++++++++++
.../ccw_0_0_10000-invalid.xml | 4 ++--
tests/nodedevschemadata/ccw_0_0_ffff.xml | 4 ++--
tests/nodedevschemadata/css_0_0_ffff.xml | 10 +++++++++
tests/nodedevxml2xmltest.c | 1 +
tools/virsh-nodedev.c | 1 +
10 files changed, 61 insertions(+), 4 deletions(-)
create mode 100644 tests/nodedevschemadata/css_0_0_ffff.xml
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 4b2b350f..f7f517b5 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -85,6 +85,7 @@
<ref name="capdrm"/>
<ref name="capmdev"/>
<ref name="capccwdev"/>
+ <ref name="capcssdev"/>
</choice>
</element>
</define>
@@ -659,6 +660,21 @@
</element>
</define>
+ <define name='capcssdev'>
+ <attribute name='type'>
+ <value>css</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 851ef0a8..8694df4b 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -65,6 +65,7 @@ VIR_ENUM_IMPL(virNodeDevCap,
"mdev_types",
"mdev",
"ccw",
+ "css",
);
VIR_ENUM_IMPL(virNodeDevNetCap,
@@ -602,6 +603,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
virNodeDeviceCapMdevDefFormat(&buf, data);
break;
case VIR_NODE_DEV_CAP_CCW_DEV:
+ case VIR_NODE_DEV_CAP_CSS_DEV:
virBufferAsprintf(&buf, "<cssid>0x%x</cssid>\n",
data->ccw_dev.cssid);
virBufferAsprintf(&buf, "<ssid>0x%x</ssid>\n",
@@ -1904,6 +1906,7 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
ret = virNodeDevCapMdevParseXML(ctxt, def, node, &caps->data.mdev);
break;
case VIR_NODE_DEV_CAP_CCW_DEV:
+ case VIR_NODE_DEV_CAP_CSS_DEV:
ret = virNodeDevCapCCWParseXML(ctxt, def, node, &caps->data.ccw_dev);
break;
case VIR_NODE_DEV_CAP_MDEV_TYPES:
@@ -2228,6 +2231,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
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_CSS_DEV:
case VIR_NODE_DEV_CAP_LAST:
/* This case is here to shutup the compiler */
break;
@@ -2281,6 +2285,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def)
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_CSS_DEV:
case VIR_NODE_DEV_CAP_LAST:
break;
}
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 9b8c7aad..47669d42 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -64,6 +64,7 @@ typedef enum {
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_CSS_DEV, /* s390 channel subsystem device */
VIR_NODE_DEV_CAP_LAST
} virNodeDevCapType;
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index bfd52412..e234432b 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -710,6 +710,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
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_CSS_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 d478a673..1c4df709 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1097,6 +1097,24 @@ udevProcessCCW(struct udev_device *device,
}
+static int
+udevProcessCSS(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+ /* do not process EADM and CHSC devices to keep the list sane */
+ if (STREQ(def->driver, "eadm_subchannel") ||
+ STREQ(def->driver, "chsc_subchannel"))
+ return -1;
+
+ if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) < 0)
+ return -1;
+
+ if (udevGenerateDeviceName(device, def, NULL) != 0)
+ return -1;
+
+ return 0;
+}
+
static int
udevGetDeviceNodes(struct udev_device *device,
virNodeDeviceDefPtr def)
@@ -1175,6 +1193,8 @@ udevGetDeviceType(struct udev_device *device,
*type = VIR_NODE_DEV_CAP_MDEV;
else if (STREQ_NULLABLE(subsystem, "ccw"))
*type = VIR_NODE_DEV_CAP_CCW_DEV;
+ else if (STREQ_NULLABLE(subsystem, "css"))
+ *type = VIR_NODE_DEV_CAP_CSS_DEV;
VIR_FREE(subsystem);
}
@@ -1219,6 +1239,8 @@ udevGetDeviceDetails(struct udev_device *device,
return udevProcessMediatedDevice(device, def);
case VIR_NODE_DEV_CAP_CCW_DEV:
return udevProcessCCW(device, def);
+ case VIR_NODE_DEV_CAP_CSS_DEV:
+ return udevProcessCSS(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
index d840555c..f3cf0c1c 100644
--- a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
+++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
@@ -1,7 +1,7 @@
<device>
<name>ccw_0_0_10000</name>
- <path>/sys/devices/css0/0.0.0000/0.0.10000</path>
- <parent>computer</parent>
+ <path>/sys/devices/css0/0.0.0070/0.0.10000</path>
+ <parent>css_0_0_0070</parent>
<capability type='ccw'>
<cssid>0x0</cssid>
<ssid>0x0</ssid>
diff --git a/tests/nodedevschemadata/ccw_0_0_ffff.xml
b/tests/nodedevschemadata/ccw_0_0_ffff.xml
index 5ecd0b0a..3b8ea46e 100644
--- a/tests/nodedevschemadata/ccw_0_0_ffff.xml
+++ b/tests/nodedevschemadata/ccw_0_0_ffff.xml
@@ -1,7 +1,7 @@
<device>
<name>ccw_0_0_ffff</name>
- <path>/sys/devices/css0/0.0.0000/0.0.ffff</path>
- <parent>computer</parent>
+ <path>/sys/devices/css0/0.0.0070/0.0.ffff</path>
+ <parent>css_0_0_0070</parent>
<capability type='ccw'>
<cssid>0x0</cssid>
<ssid>0x0</ssid>
diff --git a/tests/nodedevschemadata/css_0_0_ffff.xml
b/tests/nodedevschemadata/css_0_0_ffff.xml
new file mode 100644
index 00000000..312e07fe
--- /dev/null
+++ b/tests/nodedevschemadata/css_0_0_ffff.xml
@@ -0,0 +1,10 @@
+<device>
+ <name>css_0_0_ffff</name>
+ <path>/sys/devices/css0/0.0.ffff</path>
+ <parent>computer</parent>
+ <capability type='css'>
+ <cssid>0x0</cssid>
+ <ssid>0x0</ssid>
+ <devno>0xffff</devno>
+ </capability>
+</device>
diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
index 6168c29c..3cb23b1d 100644
--- a/tests/nodedevxml2xmltest.c
+++ b/tests/nodedevxml2xmltest.c
@@ -123,6 +123,7 @@ mymain(void)
DO_TEST("pci_0000_02_10_7_mdev_types");
DO_TEST("mdev_3627463d_b7f0_4fea_b468_f1da537d301b");
DO_TEST("ccw_0_0_ffff");
+ DO_TEST("css_0_0_ffff");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index e576b3c8..ff4b7441 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -461,6 +461,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
case VIR_NODE_DEV_CAP_CCW_DEV:
flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV;
break;
+ case VIR_NODE_DEV_CAP_CSS_DEV:
case VIR_NODE_DEV_CAP_LAST:
break;
}
--
2.24.1
10:46 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
On Mon, Aug 24, 2020 at 01:59:12PM +0200, Bjoern Walk wrote:
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Make channel subsystem (CSS) devices available in the node_device driver.o
Can there be more CSS devices per CPC?
The CCS devices reside in the computer system and provide CCW
devices, e.g.:
+- css_0_0_003a
|
+- ccw_0_0_1a2b
|
+- scsi_host0
|
+- scsi_target0_0_0
|
+- scsi_0_0_0_0
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
docs/schemas/nodedev.rng | 16 ++++++++++++++
src/conf/node_device_conf.c | 5 +++++
src/conf/node_device_conf.h | 1 +
src/conf/virnodedeviceobj.c | 1 +
src/node_device/node_device_udev.c | 22 +++++++++++++++++++
.../ccw_0_0_10000-invalid.xml | 4 ++--
tests/nodedevschemadata/ccw_0_0_ffff.xml | 4 ++--
tests/nodedevschemadata/css_0_0_ffff.xml | 10 +++++++++
tests/nodedevxml2xmltest.c | 1 +
tools/virsh-nodedev.c | 1 +
10 files changed, 61 insertions(+), 4 deletions(-)
create mode 100644 tests/nodedevschemadata/css_0_0_ffff.xml
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 4b2b350f..f7f517b5 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -85,6 +85,7 @@
<ref name="capdrm"/>
<ref name="capmdev"/>
<ref name="capccwdev"/>
+ <ref name="capcssdev"/>
</choice>
</element>
</define>
@@ -659,6 +660,21 @@
</element>
</define>
+ <define name='capcssdev'>
+ <attribute name='type'>
+ <value>css</value>
+ </attribute>
+ <element name='cssid'>
Is ^this one just a different name for CHPID or those are completely unrelated?
+ <ref name='ccwCssidRange'/>
+ </element>
+ <element name='ssid'>
+ <ref name='ccwSsidRange'/>
+ </element>
+ <element name='devno'>
+ <ref name='ccwDevnoRange'/>
+ </element>
+ </define>
+
Are ^these attributes documented a little more somewhere? I didn't find those
in [1]. I suppose it is available in the z/Architecture: Principles of
Operation publication for which I'd have to get an IBM account.
[snip]
+static int
+udevProcessCSS(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+ /* do not process EADM and CHSC devices to keep the list sane */
+ if (STREQ(def->driver, "eadm_subchannel") ||
+ STREQ(def->driver, "chsc_subchannel"))
[2] Also mentions message subchannel, although apparently there are no Linux
kernel drivers for that yet, IIUC that one would have to be added here as well
as some point, is that correct?
+ return -1;
+
+ if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) < 0)
+ return -1;
+
+ if (udevGenerateDeviceName(device, def, NULL) != 0)
+ return -1;
+
+ return 0;
+}
+
static int
udevGetDeviceNodes(struct udev_device *device,
virNodeDeviceDefPtr def)
@@ -1175,6 +1193,8 @@ udevGetDeviceType(struct udev_device *device,
*type = VIR_NODE_DEV_CAP_MDEV;
else if (STREQ_NULLABLE(subsystem, "ccw"))
*type = VIR_NODE_DEV_CAP_CCW_DEV;
+ else if (STREQ_NULLABLE(subsystem, "css"))
+ *type = VIR_NODE_DEV_CAP_CSS_DEV;
VIR_FREE(subsystem);
}
@@ -1219,6 +1239,8 @@ udevGetDeviceDetails(struct udev_device *device,
return udevProcessMediatedDevice(device, def);
case VIR_NODE_DEV_CAP_CCW_DEV:
return udevProcessCCW(device, def);
+ case VIR_NODE_DEV_CAP_CSS_DEV:
+ return udevProcessCSS(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
index d840555c..f3cf0c1c 100644
--- a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
+++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
@@ -1,7 +1,7 @@
<device>
<name>ccw_0_0_10000</name>
- <path>/sys/devices/css0/0.0.0000/0.0.10000</path>
- <parent>computer</parent>
+ <path>/sys/devices/css0/0.0.0070/0.0.10000</path>
+ <parent>css_0_0_0070</parent>
Looking at the architecture diagram at [1], I'm wondering why haven't we add
the CSS channel right away and instead only added CCW devices, which are
basically just end points on the CSS subsystem.
The changes otherwise look good, so I'm inclined to give it an ACK, but I'd
like to understand CSS a bit more before that (since I don't have HW to try
this on)
Erik
1:18 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
Erik Skultety <eskultet(a)redhat.com> [2020-09-08, 05:46PM +0200]:
On Mon, Aug 24, 2020 at 01:59:12PM +0200, Bjoern Walk wrote:
> From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
>
> Make channel subsystem (CSS) devices available in the node_device driver.o
Can there be more CSS devices per CPC?
Yes, several typically. One for each I/O device attached to the LPAR.
> The CCS devices reside in the computer system and provide CCW devices, e.g.:
>
> +- css_0_0_003a
> |
> +- ccw_0_0_1a2b
> |
> +- scsi_host0
> |
> +- scsi_target0_0_0
> |
> +- scsi_0_0_0_0
>
> Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
> Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> ---
> docs/schemas/nodedev.rng | 16 ++++++++++++++
> src/conf/node_device_conf.c | 5 +++++
> src/conf/node_device_conf.h | 1 +
> src/conf/virnodedeviceobj.c | 1 +
> src/node_device/node_device_udev.c | 22 +++++++++++++++++++
> .../ccw_0_0_10000-invalid.xml | 4 ++--
> tests/nodedevschemadata/ccw_0_0_ffff.xml | 4 ++--
> tests/nodedevschemadata/css_0_0_ffff.xml | 10 +++++++++
> tests/nodedevxml2xmltest.c | 1 +
> tools/virsh-nodedev.c | 1 +
> 10 files changed, 61 insertions(+), 4 deletions(-)
> create mode 100644 tests/nodedevschemadata/css_0_0_ffff.xml
>
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 4b2b350f..f7f517b5 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -85,6 +85,7 @@
> <ref name="capdrm"/>
> <ref name="capmdev"/>
> <ref name="capccwdev"/>
> + <ref name="capcssdev"/>
> </choice>
> </element>
> </define>
> @@ -659,6 +660,21 @@
> </element>
> </define>
>
> + <define name='capcssdev'>
> + <attribute name='type'>
> + <value>css</value>
> + </attribute>
> + <element name='cssid'>
Is ^this one just a different name for CHPID or those are completely unrelated?
As far as I understood, there is a 1-to-1 relation between CHPIDs and
channel paths, but they are not the same. For example, there are only
256 CHPIDs per system available, compared to 2^16 channel paths.
> + <ref name='ccwCssidRange'/>
> + </element>
> + <element name='ssid'>
> + <ref name='ccwSsidRange'/>
> + </element>
> + <element name='devno'>
> + <ref name='ccwDevnoRange'/>
> + </element>
> + </define>
> +
Are ^these attributes documented a little more somewhere? I didn't find those
in [1]. I suppose it is available in the z/Architecture: Principles of
Operation publication for which I'd have to get an IBM account.
Here's a freely available version of the PoP:
https://www.ibm.com/support/pages/sites/default/files/inline-files/690450...
I/O is described in chapter 13.
[snip]
>
>
> +static int
> +udevProcessCSS(struct udev_device *device,
> + virNodeDeviceDefPtr def)
> +{
> + /* do not process EADM and CHSC devices to keep the list sane */
> + if (STREQ(def->driver, "eadm_subchannel") ||
> + STREQ(def->driver, "chsc_subchannel"))
[2] Also mentions message subchannel, although apparently there are no Linux
kernel drivers for that yet, IIUC that one would have to be added here as well
as some point, is that correct?
We have never seen those, but we would want to add them here if they
have no relevance for the user.
> + return -1;
> +
> + if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) < 0)
> + return -1;
> +
> + if (udevGenerateDeviceName(device, def, NULL) != 0)
> + return -1;
> +
> + return 0;
> +}
> +
> static int
> udevGetDeviceNodes(struct udev_device *device,
> virNodeDeviceDefPtr def)
> @@ -1175,6 +1193,8 @@ udevGetDeviceType(struct udev_device *device,
> *type = VIR_NODE_DEV_CAP_MDEV;
> else if (STREQ_NULLABLE(subsystem, "ccw"))
> *type = VIR_NODE_DEV_CAP_CCW_DEV;
> + else if (STREQ_NULLABLE(subsystem, "css"))
> + *type = VIR_NODE_DEV_CAP_CSS_DEV;
>
> VIR_FREE(subsystem);
> }
> @@ -1219,6 +1239,8 @@ udevGetDeviceDetails(struct udev_device *device,
> return udevProcessMediatedDevice(device, def);
> case VIR_NODE_DEV_CAP_CCW_DEV:
> return udevProcessCCW(device, def);
> + case VIR_NODE_DEV_CAP_CSS_DEV:
> + return udevProcessCSS(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
> index d840555c..f3cf0c1c 100644
> --- a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> +++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> @@ -1,7 +1,7 @@
> <device>
> <name>ccw_0_0_10000</name>
> - <path>/sys/devices/css0/0.0.0000/0.0.10000</path>
> - <parent>computer</parent>
> + <path>/sys/devices/css0/0.0.0070/0.0.10000</path>
> + <parent>css_0_0_0070</parent>
Looking at the architecture diagram at [1], I'm wondering why haven't we add
the CSS channel right away and instead only added CCW devices, which are
basically just end points on the CSS subsystem.
Simply because they were not relevant for the user at that time. For the
typical Linux user, the channel subsystem was an implementaion detail of
I/O and every interaction with I/O devices happenend via the
corresponding CCW device. That's why I didn't implement them in the
first place. This changes now (unfortunately) with mdevs, so we might
want this information in libvirt.
The changes otherwise look good, so I'm inclined to give it an ACK, but I'd
like to understand CSS a bit more before that (since I don't have HW to try
this on)
Thanks a bunch. Understandably, I am not too deeply into I/O and I am
also confused more times than I'd like to admit. Maybe Boris can give
some more details.
Erik
--
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
2:03 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
On Wed, 9 Sep 2020 08:18:08 +0200
Bjoern Walk <bwalk(a)linux.ibm.com> wrote:
Erik Skultety <eskultet(a)redhat.com> [2020-09-08, 05:46PM
+0200]:
> On Mon, Aug 24, 2020 at 01:59:12PM +0200, Bjoern Walk wrote:
> > From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> >
> > Make channel subsystem (CSS) devices available in the node_device driver.o
>
> Can there be more CSS devices per CPC?
Yes, several typically. One for each I/O device attached to the LPAR.
>
> > The CCS devices reside in the computer system and provide CCW devices, e.g.:
> >
> > +- css_0_0_003a
> > |
> > +- ccw_0_0_1a2b
> > |
> > +- scsi_host0
> > |
> > +- scsi_target0_0_0
> > |
> > +- scsi_0_0_0_0
> >
> > Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
> > Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> > ---
> > docs/schemas/nodedev.rng | 16 ++++++++++++++
> > src/conf/node_device_conf.c | 5 +++++
> > src/conf/node_device_conf.h | 1 +
> > src/conf/virnodedeviceobj.c | 1 +
> > src/node_device/node_device_udev.c | 22 +++++++++++++++++++
> > .../ccw_0_0_10000-invalid.xml | 4 ++--
> > tests/nodedevschemadata/ccw_0_0_ffff.xml | 4 ++--
> > tests/nodedevschemadata/css_0_0_ffff.xml | 10 +++++++++
> > tests/nodedevxml2xmltest.c | 1 +
> > tools/virsh-nodedev.c | 1 +
> > 10 files changed, 61 insertions(+), 4 deletions(-)
> > create mode 100644 tests/nodedevschemadata/css_0_0_ffff.xml
> >
> > diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> > index 4b2b350f..f7f517b5 100644
> > --- a/docs/schemas/nodedev.rng
> > +++ b/docs/schemas/nodedev.rng
> > @@ -85,6 +85,7 @@
> > <ref name="capdrm"/>
> > <ref name="capmdev"/>
> > <ref name="capccwdev"/>
> > + <ref name="capcssdev"/>
> > </choice>
> > </element>
> > </define>
> > @@ -659,6 +660,21 @@
> > </element>
> > </define>
> >
> > + <define name='capcssdev'>
> > + <attribute name='type'>
> > + <value>css</value>
> > + </attribute>
> > + <element name='cssid'>
>
> Is ^this one just a different name for CHPID or those are completely unrelated?
As far as I understood, there is a 1-to-1 relation between CHPIDs and
channel paths, but they are not the same. For example, there are only
256 CHPIDs per system available, compared to 2^16 channel paths.
'CHPID' is short for 'channel path identifier'. You can have up to 256
channel paths per channel subsystem image (a given LPAR only sees one
channel subsystem image[a]). Any subchannel can use up to 8 channel
paths, and a given channel path is usually used by many subchannels.
The 'cssid' is the number of the channel subsystem image for the
subchannel.[b]
>
> > + <ref name='ccwCssidRange'/>
> > + </element>
> > + <element name='ssid'>
> > + <ref name='ccwSsidRange'/>
> > + </element>
> > + <element name='devno'>
> > + <ref name='ccwDevnoRange'/>
> > + </element>
> > + </define>
> > +
>
> Are ^these attributes documented a little more somewhere? I didn't find those
> in [1]. I suppose it is available in the z/Architecture: Principles of
[BTW: what are [1] and [2] referring to?]
> Operation publication for which I'd have to get an IBM
account.
>
Here's a freely available version of the PoP:
https://www.ibm.com/support/pages/sites/default/files/inline-files/690450...
I/O is described in chapter 13.
Let me also point to a series of articles I did on my blog:
https://virtualpenguins.blogspot.com/2017/02/channel-io-demystified.html
> [snip]
>
> >
> >
> > +static int
> > +udevProcessCSS(struct udev_device *device,
> > + virNodeDeviceDefPtr def)
> > +{
> > + /* do not process EADM and CHSC devices to keep the list sane */
> > + if (STREQ(def->driver, "eadm_subchannel") ||
> > + STREQ(def->driver, "chsc_subchannel"))
>
> [2] Also mentions message subchannel, although apparently there are no Linux
> kernel drivers for that yet, IIUC that one would have to be added here as well
> as some point, is that correct?
We have never seen those, but we would want to add them here if they
have no relevance for the user.
I think you want to filter message subchannels as well, my assumption
is that libvirt will only care about I/O subchannels.
>
> > + return -1;
> > +
> > + if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) <
0)
> > + return -1;
> > +
> > + if (udevGenerateDeviceName(device, def, NULL) != 0)
> > + return -1;
> > +
> > + return 0;
> > +}
> > +
> > static int
> > udevGetDeviceNodes(struct udev_device *device,
> > virNodeDeviceDefPtr def)
> > @@ -1175,6 +1193,8 @@ udevGetDeviceType(struct udev_device *device,
> > *type = VIR_NODE_DEV_CAP_MDEV;
> > else if (STREQ_NULLABLE(subsystem, "ccw"))
> > *type = VIR_NODE_DEV_CAP_CCW_DEV;
> > + else if (STREQ_NULLABLE(subsystem, "css"))
> > + *type = VIR_NODE_DEV_CAP_CSS_DEV;
> >
> > VIR_FREE(subsystem);
> > }
> > @@ -1219,6 +1239,8 @@ udevGetDeviceDetails(struct udev_device *device,
> > return udevProcessMediatedDevice(device, def);
> > case VIR_NODE_DEV_CAP_CCW_DEV:
> > return udevProcessCCW(device, def);
> > + case VIR_NODE_DEV_CAP_CSS_DEV:
> > + return udevProcessCSS(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
> > index d840555c..f3cf0c1c 100644
> > --- a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> > +++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> > @@ -1,7 +1,7 @@
> > <device>
> > <name>ccw_0_0_10000</name>
> > - <path>/sys/devices/css0/0.0.0000/0.0.10000</path>
> > - <parent>computer</parent>
> > + <path>/sys/devices/css0/0.0.0070/0.0.10000</path>
> > + <parent>css_0_0_0070</parent>
>
> Looking at the architecture diagram at [1], I'm wondering why haven't we
add
> the CSS channel right away and instead only added CCW devices, which are
> basically just end points on the CSS subsystem.
Simply because they were not relevant for the user at that time. For the
typical Linux user, the channel subsystem was an implementaion detail of
I/O and every interaction with I/O devices happenend via the
corresponding CCW device. That's why I didn't implement them in the
first place. This changes now (unfortunately) with mdevs, so we might
want this information in libvirt.
>
> The changes otherwise look good, so I'm inclined to give it an ACK, but I'd
> like to understand CSS a bit more before that (since I don't have HW to try
> this on)
Thanks a bunch. Understandably, I am not too deeply into I/O and I am
also confused more times than I'd like to admit. Maybe Boris can give
some more details.
I know I/O, but I'm not that deeply into libvirt; I hope I could help a
bit nevertheless :)
[a] You could have more than one channel subsystem image visible in
theory, if something called MCSS-E is available and enabled by the
operating system. AFAIK, the only implementation of MCSS-E is the one
in QEMU, and there has never been any operating system that supported
it, so it's probably best to just ignore this.
[b] Always 0 on LPARs, and within guests. We wanted to use 0xfe for
virtio devices (see QEMU device definitions), but as MCSS-E turned out
to be never properly implemented, they show up as 0 as well, which
makes it a bit useless.
7:46 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
...
> > > diff --git a/docs/schemas/nodedev.rng
b/docs/schemas/nodedev.rng
> > > index 4b2b350f..f7f517b5 100644
> > > --- a/docs/schemas/nodedev.rng
> > > +++ b/docs/schemas/nodedev.rng
> > > @@ -85,6 +85,7 @@
> > > <ref name="capdrm"/>
> > > <ref name="capmdev"/>
> > > <ref name="capccwdev"/>
> > > + <ref name="capcssdev"/>
> > > </choice>
> > > </element>
> > > </define>
> > > @@ -659,6 +660,21 @@
> > > </element>
> > > </define>
> > >
> > > + <define name='capcssdev'>
> > > + <attribute name='type'>
> > > + <value>css</value>
> > > + </attribute>
> > > + <element name='cssid'>
> >
> > Is ^this one just a different name for CHPID or those are completely
unrelated?
>
> As far as I understood, there is a 1-to-1 relation between CHPIDs and
> channel paths, but they are not the same. For example, there are only
> 256 CHPIDs per system available, compared to 2^16 channel paths.
'CHPID' is short for 'channel path identifier'. You can have up to 256
channel paths per channel subsystem image (a given LPAR only sees one
channel subsystem image[a]). Any subchannel can use up to 8 channel
paths, and a given channel path is usually used by many subchannels.
The 'cssid' is the number of the channel subsystem image for the
subchannel.[b]
>
> >
> > > + <ref name='ccwCssidRange'/>
> > > + </element>
> > > + <element name='ssid'>
> > > + <ref name='ccwSsidRange'/>
> > > + </element>
> > > + <element name='devno'>
> > > + <ref name='ccwDevnoRange'/>
> > > + </element>
> > > + </define>
> > > +
> >
> > Are ^these attributes documented a little more somewhere? I didn't find
those
> > in [1]. I suppose it is available in the z/Architecture: Principles of
[BTW: what are [1] and [2] referring to?]
Urgh...not again...
[1]
https://www.ibm.com/support/knowledgecenter/zosbasics/com.ibm.zos.znetwor...
[2] https://www.kernel.org/doc/html/latest/driver-api/s390-drivers.html
Sorry...
> > Operation publication for which I'd have to get an IBM account.
> >
>
> Here's a freely available version of the PoP:
>
>
https://www.ibm.com/support/pages/sites/default/files/inline-files/690450...
>
> I/O is described in chapter 13.
Let me also point to a series of articles I did on my blog:
https://virtualpenguins.blogspot.com/2017/02/channel-io-demystified.html
Thanks, I've looked at both, it's still fairly cloudy though.
>
> > [snip]
> >
> > >
> > >
> > > +static int
> > > +udevProcessCSS(struct udev_device *device,
> > > + virNodeDeviceDefPtr def)
> > > +{
> > > + /* do not process EADM and CHSC devices to keep the list sane */
> > > + if (STREQ(def->driver, "eadm_subchannel") ||
> > > + STREQ(def->driver, "chsc_subchannel"))
> >
> > [2] Also mentions message subchannel, although apparently there are no Linux
> > kernel drivers for that yet, IIUC that one would have to be added here as well
> > as some point, is that correct?
>
> We have never seen those, but we would want to add them here if they
> have no relevance for the user.
I think you want to filter message subchannels as well, my assumption
is that libvirt will only care about I/O subchannels.
Yes, from my limited understanding of [2] and the architecture overview, we
should filter those too.
>
> >
> > > + return -1;
> > > +
> > > + if (udevGetCCWAddress(def->sysfs_path, &def->caps->data)
< 0)
> > > + return -1;
> > > +
> > > + if (udevGenerateDeviceName(device, def, NULL) != 0)
> > > + return -1;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int
> > > udevGetDeviceNodes(struct udev_device *device,
> > > virNodeDeviceDefPtr def)
> > > @@ -1175,6 +1193,8 @@ udevGetDeviceType(struct udev_device *device,
> > > *type = VIR_NODE_DEV_CAP_MDEV;
> > > else if (STREQ_NULLABLE(subsystem, "ccw"))
> > > *type = VIR_NODE_DEV_CAP_CCW_DEV;
> > > + else if (STREQ_NULLABLE(subsystem, "css"))
> > > + *type = VIR_NODE_DEV_CAP_CSS_DEV;
> > >
> > > VIR_FREE(subsystem);
> > > }
> > > @@ -1219,6 +1239,8 @@ udevGetDeviceDetails(struct udev_device *device,
> > > return udevProcessMediatedDevice(device, def);
> > > case VIR_NODE_DEV_CAP_CCW_DEV:
> > > return udevProcessCCW(device, def);
> > > + case VIR_NODE_DEV_CAP_CSS_DEV:
> > > + return udevProcessCSS(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
> > > index d840555c..f3cf0c1c 100644
> > > --- a/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> > > +++ b/tests/nodedevschemadata/ccw_0_0_10000-invalid.xml
> > > @@ -1,7 +1,7 @@
> > > <device>
> > > <name>ccw_0_0_10000</name>
> > > - <path>/sys/devices/css0/0.0.0000/0.0.10000</path>
> > > - <parent>computer</parent>
> > > + <path>/sys/devices/css0/0.0.0070/0.0.10000</path>
> > > + <parent>css_0_0_0070</parent>
> >
> > Looking at the architecture diagram at [1], I'm wondering why haven't
we add
> > the CSS channel right away and instead only added CCW devices, which are
> > basically just end points on the CSS subsystem.
>
> Simply because they were not relevant for the user at that time. For the
> typical Linux user, the channel subsystem was an implementaion detail of
> I/O and every interaction with I/O devices happenend via the
> corresponding CCW device. That's why I didn't implement them in the
> first place. This changes now (unfortunately) with mdevs, so we might
> want this information in libvirt.
Hmm, I think having a proper representation of the architecture in the nodedev
driver is desirable regardless of mdev.
>
> >
> > The changes otherwise look good, so I'm inclined to give it an ACK, but
I'd
> > like to understand CSS a bit more before that (since I don't have HW to
try
> > this on)
>
> Thanks a bunch. Understandably, I am not too deeply into I/O and I am
> also confused more times than I'd like to admit. Maybe Boris can give
> some more details.
I know I/O, but I'm not that deeply into libvirt; I hope I could help a
bit nevertheless :)
Appreciated Connie, thanks.
Erik
[a] You could have more than one channel subsystem image visible in
theory, if something called MCSS-E is available and enabled by the
operating system. AFAIK, the only implementation of MCSS-E is the one
in QEMU, and there has never been any operating system that supported
it, so it's probably best to just ignore this.
[b] Always 0 on LPARs, and within guests. We wanted to use 0xfe for
virtio devices (see QEMU device definitions), but as MCSS-E turned out
to be never properly implemented, they show up as 0 as well, which
makes it a bit useless.
8:01 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
On Wed, 9 Sep 2020 14:46:00 +0200
Erik Skultety <eskultet(a)redhat.com> wrote:
> > > Are ^these attributes documented a little more
somewhere? I didn't find those
> > > in [1]. I suppose it is available in the z/Architecture: Principles of
>
> [BTW: what are [1] and [2] referring to?]
Urgh...not again...
[1]
https://www.ibm.com/support/knowledgecenter/zosbasics/com.ibm.zos.znetwor...
[2] https://www.kernel.org/doc/html/latest/driver-api/s390-drivers.html
TBH, I'm not sure how useful [2] is; it's mostly a vehicle for pulling
in kerneldoc documentation of structs and functions. There's also
https://www.kernel.org/doc/html/latest/s390/driver-model.html, not sure
if that is helpful.
Sorry...
>
> > > Operation publication for which I'd have to get an IBM account.
> > >
> >
> > Here's a freely available version of the PoP:
> >
> >
https://www.ibm.com/support/pages/sites/default/files/inline-files/690450...
> >
> > I/O is described in chapter 13.
>
> Let me also point to a series of articles I did on my blog:
> https://virtualpenguins.blogspot.com/2017/02/channel-io-demystified.html
Thanks, I've looked at both, it's still fairly cloudy though.
Let me know if I can do anything to make it clearer. This channel I/O
stuff probably can be a bit daunting...
8:16 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
On Wed, Sep 09, 2020 at 03:01:50PM +0200, Cornelia Huck wrote:
On Wed, 9 Sep 2020 14:46:00 +0200
Erik Skultety <eskultet(a)redhat.com> wrote:
> > > > Are ^these attributes documented a little more somewhere? I
didn't find those
> > > > in [1]. I suppose it is available in the z/Architecture: Principles
of
> >
> > [BTW: what are [1] and [2] referring to?]
>
> Urgh...not again...
> [1]
https://www.ibm.com/support/knowledgecenter/zosbasics/com.ibm.zos.znetwor...
> [2] https://www.kernel.org/doc/html/latest/driver-api/s390-drivers.html
TBH, I'm not sure how useful [2] is; it's mostly a vehicle for pulling
It wasn't, but it was the only resource mentioning something about the ccw bus
which was clearly relevant to patch 2/5 (udevProcessCSS).
in kerneldoc documentation of structs and functions. There's
also
https://www.kernel.org/doc/html/latest/s390/driver-model.html, not sure
if that is helpful.
>
> Sorry...
>
> >
> > > > Operation publication for which I'd have to get an IBM account.
> > > >
> > >
> > > Here's a freely available version of the PoP:
> > >
> > >
https://www.ibm.com/support/pages/sites/default/files/inline-files/690450...
> > >
> > > I/O is described in chapter 13.
> >
> > Let me also point to a series of articles I did on my blog:
> > https://virtualpenguins.blogspot.com/2017/02/channel-io-demystified.html
>
> Thanks, I've looked at both, it's still fairly cloudy though.
Let me know if I can do anything to make it clearer. This channel I/O
stuff probably can be a bit daunting...
Sure, you could shed a bit more light on this one:
https://www.redhat.com/archives/libvir-list/2020-September/msg00492.html
Erik
11:11 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
On 9/9/20 9:03 AM, Cornelia Huck wrote:
>> [snip]
>>
>>>
>>> +static int
>>> +udevProcessCSS(struct udev_device *device,
>>> + virNodeDeviceDefPtr def)
>>> +{
>>> + /* do not process EADM and CHSC devices to keep the list sane */
>>> + if (STREQ(def->driver, "eadm_subchannel") ||
>>> + STREQ(def->driver, "chsc_subchannel"))
>> [2] Also mentions message subchannel, although apparently there are no Linux
>> kernel drivers for that yet, IIUC that one would have to be added here as well
>> as some point, is that correct?
> We have never seen those, but we would want to add them here if they
> have no relevance for the user.
I think you want to filter message subchannels as well, my assumption
is that libvirt will only care about I/O subchannels.
In
https://www.kernel.org/doc/html/latest/driver-api/s390-drivers.html
it is stated that for
Message subchannels. No Linux driver currently exists.
Therefore I do not know how I could currently filter message subchannels
out. Due you want me to reverse the logic and just filter out everything
but "io_subchannel" instead (which I would regard as fencing the unknown)?
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
12:41 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
On Fri, Sep 11, 2020 at 06:11:49PM +0200, Boris Fiuczynski wrote:
On 9/9/20 9:03 AM, Cornelia Huck wrote:
> > > [snip]
> > > >
> > > > +static int
> > > > +udevProcessCSS(struct udev_device *device,
> > > > + virNodeDeviceDefPtr def)
> > > > +{
> > > > + /* do not process EADM and CHSC devices to keep the list sane
*/
> > > > + if (STREQ(def->driver, "eadm_subchannel") ||
> > > > + STREQ(def->driver, "chsc_subchannel"))
> > > [2] Also mentions message subchannel, although apparently there are no
Linux
> > > kernel drivers for that yet, IIUC that one would have to be added here as
well
> > > as some point, is that correct?
> > We have never seen those, but we would want to add them here if they
> > have no relevance for the user.
> I think you want to filter message subchannels as well, my assumption
> is that libvirt will only care about I/O subchannels.
>
In
https://www.kernel.org/doc/html/latest/driver-api/s390-drivers.html
it is stated that for
Message subchannels. No Linux driver currently exists.
Therefore I do not know how I could currently filter message subchannels
out. Due you want me to reverse the logic and just filter out everything but
"io_subchannel" instead (which I would regard as fencing the unknown)?
Well, I don't see a problem filtering out everything but the single thing you
actually care about, rather than enumerating the things you don't care about
and won't care about in the future, so yeah, I'd go with that.
Erik
6:52 a.m.
New subject: [PATCH 2/5] node_device: detect CSS devices
On 9/14/20 7:41 AM, Erik Skultety wrote:
On Fri, Sep 11, 2020 at 06:11:49PM +0200, Boris Fiuczynski wrote:
> On 9/9/20 9:03 AM, Cornelia Huck wrote:
>>>> [snip]
>>>>>
>>>>> +static int
>>>>> +udevProcessCSS(struct udev_device *device,
>>>>> + virNodeDeviceDefPtr def)
>>>>> +{
>>>>> + /* do not process EADM and CHSC devices to keep the list sane
*/
>>>>> + if (STREQ(def->driver, "eadm_subchannel") ||
>>>>> + STREQ(def->driver, "chsc_subchannel"))
>>>> [2] Also mentions message subchannel, although apparently there are no
Linux
>>>> kernel drivers for that yet, IIUC that one would have to be added here as
well
>>>> as some point, is that correct?
>>> We have never seen those, but we would want to add them here if they
>>> have no relevance for the user.
>> I think you want to filter message subchannels as well, my assumption
>> is that libvirt will only care about I/O subchannels.
>>
> In
> https://www.kernel.org/doc/html/latest/driver-api/s390-drivers.html
> it is stated that for
> Message subchannels. No Linux driver currently exists.
>
> Therefore I do not know how I could currently filter message subchannels
> out. Due you want me to reverse the logic and just filter out everything but
> "io_subchannel" instead (which I would regard as fencing the unknown)?
Well, I don't see a problem filtering out everything but the single thing you
actually care about, rather than enumerating the things you don't care about
and won't care about in the future, so yeah, I'd go with that.
Erik
OK, I will do that.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
6:59 a.m.
New subject: [PATCH 3/5] virsh: nodedev: ability to filter CSS capabilities
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Allow to filter for CSS devices.
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
docs/formatnode.html.in | 12 ++++++++++++
docs/manpages/virsh.rst | 2 +-
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 | 2 ++
7 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 8a51c4da..4c7de50a 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -420,6 +420,18 @@
<dd>The device number.</dd>
</dl>
</dd>
+ <dt><code>css</code></dt>
+ <dd>Describes a Channel SubSystem (CSS) 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/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 5731656b..ee287ac5 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -4908,7 +4908,7 @@ List all of the devices available on the node that are known by
libvirt.
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', 'ccw'.
+'mdev_types', 'ccw', 'css'.
If *--tree* is used, the output is formatted in a tree representing parents of each
node. *cap* and *--tree* are mutually exclusive.
diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h
index a2ad61ac..dd2ffd57 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_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 */
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV = 1 << 16, /* CSS device */
} virConnectListAllNodeDeviceFlags;
int virConnectListAllNodeDevices (virConnectPtr conn,
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 47669d42..5484bc34 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -368,7 +368,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps);
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)
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV | \
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV)
int
virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host);
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index e234432b..8aefd15e 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -861,7 +861,8 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj,
MATCH(DRM) ||
MATCH(MDEV_TYPES) ||
MATCH(MDEV) ||
- MATCH(CCW_DEV)))
+ MATCH(CCW_DEV) ||
+ MATCH(CSS_DEV)))
return false;
}
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index ca6c2bb0..28765b9c 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -101,6 +101,7 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int
flags)
* VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES
* VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV
* VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV
+ * VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_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 ff4b7441..9f29181a 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -462,6 +462,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV;
break;
case VIR_NODE_DEV_CAP_CSS_DEV:
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV;
+ break;
case VIR_NODE_DEV_CAP_LAST:
break;
}
--
2.24.1
10:48 a.m.
New subject: [PATCH 3/5] virsh: nodedev: ability to filter CSS capabilities
On Mon, Aug 24, 2020 at 01:59:13PM +0200, Bjoern Walk wrote:
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Allow to filter for CSS devices.
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
6:59 a.m.
New subject: [PATCH 4/5] node_device: detect DASD devices
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Make Direct Access Storage Devices (DASDs) available in the node_device driver.
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/node_device/node_device_udev.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 1c4df709..5f9d67cc 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -871,6 +871,19 @@ udevProcessSD(struct udev_device *device,
}
+static int
+udevProcessDasd(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+ virNodeDevCapStoragePtr storage = &def->caps->data.storage;
+
+ if (udevGetStringSysfsAttr(device, "device/uid", &storage->serial)
< 0)
+ return -1;
+
+ return udevProcessDisk(device, def);
+}
+
+
/* This function exists to deal with the case in which a driver does
* not provide a device type in the usual place, but udev told us it's
* a storage device, and we can make a good guess at what kind of
@@ -891,6 +904,15 @@ udevKludgeStorageType(virNodeDeviceDefPtr def)
def->sysfs_path);
return 0;
}
+ /* dasd disk */
+ if (STRPREFIX(def->caps->data.storage.block, "/dev/dasd")) {
+ def->caps->data.storage.drive_type = g_strdup("dasd");
+ VIR_DEBUG("Found storage type '%s' for device "
+ "with sysfs path '%s'",
+ def->caps->data.storage.drive_type,
+ def->sysfs_path);
+ return 0;
+ }
VIR_DEBUG("Could not determine storage type "
"for device with sysfs path '%s'", def->sysfs_path);
return -1;
@@ -978,6 +1000,8 @@ udevProcessStorage(struct udev_device *device,
ret = udevProcessFloppy(device, def);
} else if (STREQ(def->caps->data.storage.drive_type, "sd")) {
ret = udevProcessSD(device, def);
+ } else if (STREQ(def->caps->data.storage.drive_type, "dasd")) {
+ ret = udevProcessDasd(device, def);
} else {
VIR_DEBUG("Unsupported storage type '%s'",
def->caps->data.storage.drive_type);
--
2.24.1
10:50 a.m.
New subject: [PATCH 4/5] node_device: detect DASD devices
On Mon, Aug 24, 2020 at 01:59:14PM +0200, Bjoern Walk wrote:
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Make Direct Access Storage Devices (DASDs) available in the node_device driver.
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/node_device/node_device_udev.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 1c4df709..5f9d67cc 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -871,6 +871,19 @@ udevProcessSD(struct udev_device *device,
}
+static int
+udevProcessDasd(struct udev_device *device,
+ virNodeDeviceDefPtr def)
s/Dasd/DASD
+{
+ virNodeDevCapStoragePtr storage = &def->caps->data.storage;
+
+ if (udevGetStringSysfsAttr(device, "device/uid", &storage->serial)
< 0)
+ return -1;
+
+ return udevProcessDisk(device, def);
+}
+
+
/* This function exists to deal with the case in which a driver does
* not provide a device type in the usual place, but udev told us it's
* a storage device, and we can make a good guess at what kind of
@@ -891,6 +904,15 @@ udevKludgeStorageType(virNodeDeviceDefPtr def)
def->sysfs_path);
return 0;
}
+ /* dasd disk */
+ if (STRPREFIX(def->caps->data.storage.block, "/dev/dasd")) {
+ def->caps->data.storage.drive_type = g_strdup("dasd");
+ VIR_DEBUG("Found storage type '%s' for device "
+ "with sysfs path '%s'",
+ def->caps->data.storage.drive_type,
+ def->sysfs_path);
I understand why we would need it for /dev/vdX, but can udev not know the
drive_type from kernel? IOW Do we really need ^this hunk?
+ return 0;
+ }
VIR_DEBUG("Could not determine storage type "
"for device with sysfs path '%s'", def->sysfs_path);
return -1;
@@ -978,6 +1000,8 @@ udevProcessStorage(struct udev_device *device,
ret = udevProcessFloppy(device, def);
} else if (STREQ(def->caps->data.storage.drive_type, "sd")) {
ret = udevProcessSD(device, def);
+ } else if (STREQ(def->caps->data.storage.drive_type, "dasd")) {
+ ret = udevProcessDasd(device, def);
} else {
VIR_DEBUG("Unsupported storage type '%s'",
def->caps->data.storage.drive_type);
The rest looks good:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
10:58 a.m.
New subject: [PATCH 4/5] node_device: detect DASD devices
On 9/8/20 5:50 PM, Erik Skultety wrote:
On Mon, Aug 24, 2020 at 01:59:14PM +0200, Bjoern Walk wrote:
> From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
>
> Make Direct Access Storage Devices (DASDs) available in the node_device driver.
>
> Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
> Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> ---
> src/node_device/node_device_udev.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 1c4df709..5f9d67cc 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -871,6 +871,19 @@ udevProcessSD(struct udev_device *device,
> }
>
>
> +static int
> +udevProcessDasd(struct udev_device *device,
> + virNodeDeviceDefPtr def)
s/Dasd/DASD
Changed it
> +{
> + virNodeDevCapStoragePtr storage = &def->caps->data.storage;
> +
> + if (udevGetStringSysfsAttr(device, "device/uid",
&storage->serial) < 0)
> + return -1;
> +
> + return udevProcessDisk(device, def);
> +}
> +
> +
> /* This function exists to deal with the case in which a driver does
> * not provide a device type in the usual place, but udev told us it's
> * a storage device, and we can make a good guess at what kind of
> @@ -891,6 +904,15 @@ udevKludgeStorageType(virNodeDeviceDefPtr def)
> def->sysfs_path);
> return 0;
> }
> + /* dasd disk */
> + if (STRPREFIX(def->caps->data.storage.block, "/dev/dasd")) {
> + def->caps->data.storage.drive_type = g_strdup("dasd");
> + VIR_DEBUG("Found storage type '%s' for device "
> + "with sysfs path '%s'",
> + def->caps->data.storage.drive_type,
> + def->sysfs_path);
I understand why we would need it for /dev/vdX, but can udev not know the
drive_type from kernel? IOW Do we really need ^this hunk?
For DASDs there are currently no identifies in udev besides ID_PATH.
ID_TYPE=disk does not exist. That's why the DASDs fall through the
udevProcessStorage detection logic. Without this hunk the dasd devices
are being detected as storage devices but than end up as "Unsupported
storage type".
The short answer is yes. :-)
> + return 0;
> + }
> VIR_DEBUG("Could not determine storage type "
> "for device with sysfs path '%s'",
def->sysfs_path);
> return -1;
> @@ -978,6 +1000,8 @@ udevProcessStorage(struct udev_device *device,
> ret = udevProcessFloppy(device, def);
> } else if (STREQ(def->caps->data.storage.drive_type, "sd")) {
> ret = udevProcessSD(device, def);
> + } else if (STREQ(def->caps->data.storage.drive_type, "dasd")) {
> + ret = udevProcessDasd(device, def);
> } else {
> VIR_DEBUG("Unsupported storage type '%s'",
> def->caps->data.storage.drive_type);
The rest looks good:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
Thanks
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
12:17 a.m.
New subject: [PATCH 4/5] node_device: detect DASD devices
...
> > }
> > + /* dasd disk */
> > + if (STRPREFIX(def->caps->data.storage.block, "/dev/dasd"))
{
> > + def->caps->data.storage.drive_type =
g_strdup("dasd");
> > + VIR_DEBUG("Found storage type '%s' for device "
> > + "with sysfs path '%s'",
> > + def->caps->data.storage.drive_type,
> > + def->sysfs_path);
>
> I understand why we would need it for /dev/vdX, but can udev not know the
> drive_type from kernel? IOW Do we really need ^this hunk?
>
For DASDs there are currently no identifies in udev besides ID_PATH.
ID_TYPE=disk does not exist. That's why the DASDs fall through the
udevProcessStorage detection logic. Without this hunk the dasd devices are
being detected as storage devices but than end up as "Unsupported storage
type".
The short answer is yes. :-)
Okay, can you put a concise version of ^this in a commentary then?
Erik
6:53 a.m.
New subject: [PATCH 4/5] node_device: detect DASD devices
On 9/14/20 7:17 AM, Erik Skultety wrote:
...
>>> }
>>> + /* dasd disk */
>>> + if (STRPREFIX(def->caps->data.storage.block,
"/dev/dasd")) {
>>> + def->caps->data.storage.drive_type =
g_strdup("dasd");
>>> + VIR_DEBUG("Found storage type '%s' for device "
>>> + "with sysfs path '%s'",
>>> + def->caps->data.storage.drive_type,
>>> + def->sysfs_path);
>>
>> I understand why we would need it for /dev/vdX, but can udev not know the
>> drive_type from kernel? IOW Do we really need ^this hunk?
>>
>
> For DASDs there are currently no identifies in udev besides ID_PATH.
> ID_TYPE=disk does not exist. That's why the DASDs fall through the
> udevProcessStorage detection logic. Without this hunk the dasd devices are
> being detected as storage devices but than end up as "Unsupported storage
> type".
> The short answer is yes. :-)
Okay, can you put a concise version of ^this in a commentary then?
Erik
Done, I am going to send a v2 later.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
6:59 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Allow vfio-ccw mdev devices to be created besides vfio-pci mdev devices
as well.
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/conf/domain_addr.c | 2 +-
src/conf/domain_addr.h | 3 ++
src/libvirt_private.syms | 1 +
src/node_device/node_device_driver.c | 49 +++++++++++++++++++++++-----
4 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 1068cbf1..1bfa164a 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1365,7 +1365,7 @@ virDomainPCIAddressSetAllMulti(virDomainDefPtr def)
}
-static char*
+char*
virDomainCCWAddressAsString(virDomainDeviceCCWAddressPtr addr)
{
return g_strdup_printf("%x.%x.%04x", addr->cssid, addr->ssid,
addr->devno);
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index c1363c14..a0460b40 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -208,6 +208,9 @@ int virDomainCCWAddressAssign(virDomainDeviceInfoPtr dev,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
void virDomainCCWAddressSetFree(virDomainCCWAddressSetPtr addrs);
+char* virDomainCCWAddressAsString(virDomainDeviceCCWAddressPtr addr)
+ ATTRIBUTE_NONNULL(1);
+
virDomainCCWAddressSetPtr
virDomainCCWAddressSetCreateFromDomain(virDomainDefPtr def)
ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 01c2e710..e1342569 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -143,6 +143,7 @@ virPCIDeviceAddressParseXML;
# conf/domain_addr.h
virDomainCCWAddressAssign;
+virDomainCCWAddressAsString;
virDomainCCWAddressSetCreateFromDomain;
virDomainCCWAddressSetFree;
virDomainPCIAddressBusIsFullyReserved;
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index e89c8b0e..bee2ef93 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -28,6 +28,7 @@
#include "virerror.h"
#include "datatypes.h"
+#include "domain_addr.h"
#include "viralloc.h"
#include "virfile.h"
#include "virjson.h"
@@ -628,7 +629,7 @@ nodeDeviceFindAddressByName(const char *name)
{
virNodeDeviceDefPtr def = NULL;
virNodeDevCapsDefPtr caps = NULL;
- char *pci_addr = NULL;
+ char *addr = NULL;
virNodeDeviceObjPtr dev = virNodeDeviceObjListFindByName(driver->devs, name);
if (!dev) {
@@ -639,22 +640,52 @@ nodeDeviceFindAddressByName(const char *name)
def = virNodeDeviceObjGetDef(dev);
for (caps = def->caps; caps != NULL; caps = caps->next) {
- if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) {
- virPCIDeviceAddress addr = {
+ switch ((virNodeDevCapType) caps->data.type) {
+ case VIR_NODE_DEV_CAP_PCI_DEV: {
+ virPCIDeviceAddress pci_addr = {
.domain = caps->data.pci_dev.domain,
.bus = caps->data.pci_dev.bus,
.slot = caps->data.pci_dev.slot,
.function = caps->data.pci_dev.function
};
- pci_addr = virPCIDeviceAddressAsString(&addr);
+ addr = virPCIDeviceAddressAsString(&pci_addr);
break;
}
+ case VIR_NODE_DEV_CAP_CSS_DEV: {
+ virDomainDeviceCCWAddress ccw_addr = {
+ .cssid = caps->data.ccw_dev.cssid,
+ .ssid = caps->data.ccw_dev.ssid,
+ .devno = caps->data.ccw_dev.devno
+ };
+
+ addr = virDomainCCWAddressAsString(&ccw_addr);
+ break;
+ }
+
+ 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_NET:
+ case VIR_NODE_DEV_CAP_SCSI_HOST:
+ 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:
+ case VIR_NODE_DEV_CAP_VPORTS:
+ case VIR_NODE_DEV_CAP_SCSI_GENERIC:
+ case VIR_NODE_DEV_CAP_DRM:
+ 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:
+ continue;
+ }
}
virNodeDeviceObjEndAPI(&dev);
- return pci_addr;
+ return addr;
}
@@ -664,11 +695,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
{
virCommandPtr cmd;
g_autofree char *json = NULL;
- g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
+ g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
- if (!parent_pci) {
+ if (!parent_addr) {
virReportError(VIR_ERR_NO_NODE_DEVICE,
- _("unable to find PCI address for parent device
'%s'"), def->parent);
+ _("unable to find address for parent device
'%s'"), def->parent);
return NULL;
}
@@ -679,7 +710,7 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
}
cmd = virCommandNewArgList(MDEVCTL, "start",
- "-p", parent_pci,
+ "-p", parent_addr,
"--jsonfile", "/dev/stdin",
NULL);
--
2.24.1
11:01 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
On Mon, Aug 24, 2020 at 01:59:15PM +0200, Bjoern Walk wrote:
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Allow vfio-ccw mdev devices to be created besides vfio-pci mdev devices
as well.
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/conf/domain_addr.c | 2 +-
src/conf/domain_addr.h | 3 ++
src/libvirt_private.syms | 1 +
src/node_device/node_device_driver.c | 49 +++++++++++++++++++++++-----
4 files changed, 45 insertions(+), 10 deletions(-)
[snip]
@@ -639,22 +640,52 @@ nodeDeviceFindAddressByName(const char *name)
def = virNodeDeviceObjGetDef(dev);
for (caps = def->caps; caps != NULL; caps = caps->next) {
- if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) {
- virPCIDeviceAddress addr = {
+ switch ((virNodeDevCapType) caps->data.type) {
+ case VIR_NODE_DEV_CAP_PCI_DEV: {
+ virPCIDeviceAddress pci_addr = {
.domain = caps->data.pci_dev.domain,
.bus = caps->data.pci_dev.bus,
.slot = caps->data.pci_dev.slot,
.function = caps->data.pci_dev.function
};
- pci_addr = virPCIDeviceAddressAsString(&addr);
+ addr = virPCIDeviceAddressAsString(&pci_addr);
break;
}
+ case VIR_NODE_DEV_CAP_CSS_DEV: {
+ virDomainDeviceCCWAddress ccw_addr = {
+ .cssid = caps->data.ccw_dev.cssid,
+ .ssid = caps->data.ccw_dev.ssid,
+ .devno = caps->data.ccw_dev.devno
+ };
+
+ addr = virDomainCCWAddressAsString(&ccw_addr);
+ break;
+ }
+
+ 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_NET:
+ case VIR_NODE_DEV_CAP_SCSI_HOST:
+ 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:
+ case VIR_NODE_DEV_CAP_VPORTS:
+ case VIR_NODE_DEV_CAP_SCSI_GENERIC:
+ case VIR_NODE_DEV_CAP_DRM:
+ case VIR_NODE_DEV_CAP_MDEV_TYPES:
+ case VIR_NODE_DEV_CAP_MDEV:
+ case VIR_NODE_DEV_CAP_CCW_DEV:
What about ccw mdevs? I would think that it's CCW end point device that
we would want to assign concurrently, but we're only "slicing" the root
channel
subsystem device. If you have and CSS mdev, are the CCW subchannels separate
for each VM or does it behave kind of like NPIV with its vHBAs? OR I'm
completely off here?
+ case VIR_NODE_DEV_CAP_LAST:
+ continue;
+ }
}
virNodeDeviceObjEndAPI(&dev);
- return pci_addr;
+ return addr;
}
@@ -664,11 +695,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
{
virCommandPtr cmd;
g_autofree char *json = NULL;
- g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
+ g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
- if (!parent_pci) {
+ if (!parent_addr) {
virReportError(VIR_ERR_NO_NODE_DEVICE,
- _("unable to find PCI address for parent device
'%s'"), def->parent);
+ _("unable to find address for parent device
'%s'"), def->parent);
I'm wondering whether "unable to find parent device '%s'" would not
suffice,
since we're not specifying what type of address we were not able to find - I'm
not even sure the address information is important at all.
Erik
return NULL;
}
@@ -679,7 +710,7 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
}
cmd = virCommandNewArgList(MDEVCTL, "start",
- "-p", parent_pci,
+ "-p", parent_addr,
"--jsonfile", "/dev/stdin",
NULL);
--
2.24.1
1:23 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
Erik Skultety <eskultet(a)redhat.com> [2020-09-08, 06:01PM +0200]:
On Mon, Aug 24, 2020 at 01:59:15PM +0200, Bjoern Walk wrote:
> From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> + 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_NET:
> + case VIR_NODE_DEV_CAP_SCSI_HOST:
> + 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:
> + case VIR_NODE_DEV_CAP_VPORTS:
> + case VIR_NODE_DEV_CAP_SCSI_GENERIC:
> + case VIR_NODE_DEV_CAP_DRM:
> + case VIR_NODE_DEV_CAP_MDEV_TYPES:
> + case VIR_NODE_DEV_CAP_MDEV:
> + case VIR_NODE_DEV_CAP_CCW_DEV:
What about ccw mdevs? I would think that it's CCW end point device that
we would want to assign concurrently, but we're only "slicing" the root
channel
subsystem device. If you have and CSS mdev, are the CCW subchannels separate
for each VM or does it behave kind of like NPIV with its vHBAs? OR I'm
completely off here?
You are correct. This is an unfortunate architectural decision on our
platform. For vfio-ccw, we instatiate the mdevs on the CSS layer, which
has a 1-to-1 relation to the underlying CCW device. That's why we need
the information about the CSS devices in libvirt, to give the user a
chance to easily find this relation when he wants to do passthrough with
a DASD for example. It's a bit unintuitive and even silly for the
end user, but it's architecturally correct and hence it was implemented
like this in the vfio-ccw kernel driver. We don't even get the benefits
of something like NPIV because for every subchannel there is only one
CCW device (at least to my knowledge).
> + case VIR_NODE_DEV_CAP_LAST:
> + continue;
> + }
> }
>
> virNodeDeviceObjEndAPI(&dev);
>
> - return pci_addr;
> + return addr;
> }
>
>
> @@ -664,11 +695,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
> {
> virCommandPtr cmd;
> g_autofree char *json = NULL;
> - g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
> + g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
>
> - if (!parent_pci) {
> + if (!parent_addr) {
> virReportError(VIR_ERR_NO_NODE_DEVICE,
> - _("unable to find PCI address for parent device
'%s'"), def->parent);
> + _("unable to find address for parent device
'%s'"), def->parent);
I'm wondering whether "unable to find parent device '%s'" would not
suffice,
since we're not specifying what type of address we were not able to find - I'm
not even sure the address information is important at all.
Erik
> return NULL;
> }
>
> @@ -679,7 +710,7 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
> }
>
> cmd = virCommandNewArgList(MDEVCTL, "start",
> - "-p", parent_pci,
> + "-p", parent_addr,
> "--jsonfile", "/dev/stdin",
> NULL);
>
> --
> 2.24.1
>
--
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
7:40 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
On Wed, Sep 09, 2020 at 08:23:39AM +0200, Bjoern Walk wrote:
Erik Skultety <eskultet(a)redhat.com> [2020-09-08, 06:01PM
+0200]:
> On Mon, Aug 24, 2020 at 01:59:15PM +0200, Bjoern Walk wrote:
> > From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> > + 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_NET:
> > + case VIR_NODE_DEV_CAP_SCSI_HOST:
> > + 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:
> > + case VIR_NODE_DEV_CAP_VPORTS:
> > + case VIR_NODE_DEV_CAP_SCSI_GENERIC:
> > + case VIR_NODE_DEV_CAP_DRM:
> > + case VIR_NODE_DEV_CAP_MDEV_TYPES:
> > + case VIR_NODE_DEV_CAP_MDEV:
> > + case VIR_NODE_DEV_CAP_CCW_DEV:
>
> What about ccw mdevs? I would think that it's CCW end point device that
> we would want to assign concurrently, but we're only "slicing" the
root channel
> subsystem device. If you have and CSS mdev, are the CCW subchannels separate
> for each VM or does it behave kind of like NPIV with its vHBAs? OR I'm
> completely off here?
You are correct. This is an unfortunate architectural decision on our
platform. For vfio-ccw, we instatiate the mdevs on the CSS layer, which
has a 1-to-1 relation to the underlying CCW device. That's why we need
the information about the CSS devices in libvirt, to give the user a
chance to easily find this relation when he wants to do passthrough with
a DASD for example. It's a bit unintuitive and even silly for the
end user, but it's architecturally correct and hence it was implemented
like this in the vfio-ccw kernel driver. We don't even get the benefits
of something like NPIV because for every subchannel there is only one
CCW device (at least to my knowledge).
Having read both Connie's blog on channels and the I/O chapter in the free
architectural publication you linked in the other reply, I now have a very
vague understating of the architecture. One thing that still
puzzles me though is that if there's a 1:1 subchannel mapping between CSS and an
CCW device which then communicates via a control unit (which there may be more
than 1) with CSS over up to 8 channel paths (this at least to me resembles
NPIV), what does the mdev really solve, can't you assign one of the channel
paths of one of the several control units the device is connected to to
plug into the VM? Well, clearly not, I'm just being curious and trying to
understand the use case here.
Regards,
Erik
10:29 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
On Wed, 9 Sep 2020 14:40:33 +0200
Erik Skultety <eskultet(a)redhat.com> wrote:
On Wed, Sep 09, 2020 at 08:23:39AM +0200, Bjoern Walk wrote:
> Erik Skultety <eskultet(a)redhat.com> [2020-09-08, 06:01PM +0200]:
> > On Mon, Aug 24, 2020 at 01:59:15PM +0200, Bjoern Walk wrote:
> > > From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> > > + 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_NET:
> > > + case VIR_NODE_DEV_CAP_SCSI_HOST:
> > > + 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:
> > > + case VIR_NODE_DEV_CAP_VPORTS:
> > > + case VIR_NODE_DEV_CAP_SCSI_GENERIC:
> > > + case VIR_NODE_DEV_CAP_DRM:
> > > + case VIR_NODE_DEV_CAP_MDEV_TYPES:
> > > + case VIR_NODE_DEV_CAP_MDEV:
> > > + case VIR_NODE_DEV_CAP_CCW_DEV:
> >
> > What about ccw mdevs? I would think that it's CCW end point device that
> > we would want to assign concurrently, but we're only "slicing"
the root channel
> > subsystem device. If you have and CSS mdev, are the CCW subchannels separate
> > for each VM or does it behave kind of like NPIV with its vHBAs? OR I'm
> > completely off here?
>
> You are correct. This is an unfortunate architectural decision on our
> platform. For vfio-ccw, we instatiate the mdevs on the CSS layer, which
> has a 1-to-1 relation to the underlying CCW device. That's why we need
> the information about the CSS devices in libvirt, to give the user a
> chance to easily find this relation when he wants to do passthrough with
> a DASD for example. It's a bit unintuitive and even silly for the
> end user, but it's architecturally correct and hence it was implemented
> like this in the vfio-ccw kernel driver. We don't even get the benefits
> of something like NPIV because for every subchannel there is only one
> CCW device (at least to my knowledge).
Having read both Connie's blog on channels and the I/O chapter in the free
architectural publication you linked in the other reply, I now have a very
vague understating of the architecture. One thing that still
puzzles me though is that if there's a 1:1 subchannel mapping between CSS and an
CCW device which then communicates via a control unit (which there may be more
than 1) with CSS over up to 8 channel paths (this at least to me resembles
NPIV), what does the mdev really solve, can't you assign one of the channel
paths of one of the several control units the device is connected to to
plug into the VM? Well, clearly not, I'm just being curious and trying to
understand the use case here.
I think it's easier to leave control units mostly out of the picture,
as they are not really relevant for the modelling here. And yes, there
is always exactly one subchannel for an I/O device.
Subchannels (mostly a concept describing how to interact with a device)
and channel paths (mostly corresponding to the way data flows to/from
the device) are probably best understood as different ways to describe
how devices are accessed. You don't really want to pass channel paths
(as they are shared between many devices); the subchannel is a more
natural unit to pass. The operating system also has way more control
over subchannels than over individual channel paths.
Regarding mdev, the idea here is less to be able to slice up a device
as you do with GPUs, but more to be able to add a vendor driver that
does the actual interesting work (channel program translations etc.)
As to why we use the subchannel instead of the device, that's mostly
following from how the low-level channel I/O layer in the Linux kernel
is organized: The default I/O subchannel driver does a lot of things
that a normal device driver should not need to deal with (path
grouping, verification, status accumulation, etc.), but which would get
into the way if you wanted to assign the device via vfio. Therefore,
vfio-ccw implements an alternate I/O subchannel driver.
HTH (at least a little bit).
3:41 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
On Wed, Sep 09, 2020 at 05:29:20PM +0200, Cornelia Huck wrote:
On Wed, 9 Sep 2020 14:40:33 +0200
Erik Skultety <eskultet(a)redhat.com> wrote:
> On Wed, Sep 09, 2020 at 08:23:39AM +0200, Bjoern Walk wrote:
> > Erik Skultety <eskultet(a)redhat.com> [2020-09-08, 06:01PM +0200]:
> > > On Mon, Aug 24, 2020 at 01:59:15PM +0200, Bjoern Walk wrote:
> > > > From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> > > > + 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_NET:
> > > > + case VIR_NODE_DEV_CAP_SCSI_HOST:
> > > > + 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:
> > > > + case VIR_NODE_DEV_CAP_VPORTS:
> > > > + case VIR_NODE_DEV_CAP_SCSI_GENERIC:
> > > > + case VIR_NODE_DEV_CAP_DRM:
> > > > + case VIR_NODE_DEV_CAP_MDEV_TYPES:
> > > > + case VIR_NODE_DEV_CAP_MDEV:
> > > > + case VIR_NODE_DEV_CAP_CCW_DEV:
> > >
> > > What about ccw mdevs? I would think that it's CCW end point device
that
> > > we would want to assign concurrently, but we're only
"slicing" the root channel
> > > subsystem device. If you have and CSS mdev, are the CCW subchannels
separate
> > > for each VM or does it behave kind of like NPIV with its vHBAs? OR
I'm
> > > completely off here?
> >
> > You are correct. This is an unfortunate architectural decision on our
> > platform. For vfio-ccw, we instatiate the mdevs on the CSS layer, which
> > has a 1-to-1 relation to the underlying CCW device. That's why we need
> > the information about the CSS devices in libvirt, to give the user a
> > chance to easily find this relation when he wants to do passthrough with
> > a DASD for example. It's a bit unintuitive and even silly for the
> > end user, but it's architecturally correct and hence it was implemented
> > like this in the vfio-ccw kernel driver. We don't even get the benefits
> > of something like NPIV because for every subchannel there is only one
> > CCW device (at least to my knowledge).
>
> Having read both Connie's blog on channels and the I/O chapter in the free
> architectural publication you linked in the other reply, I now have a very
> vague understating of the architecture. One thing that still
> puzzles me though is that if there's a 1:1 subchannel mapping between CSS and
an
> CCW device which then communicates via a control unit (which there may be more
> than 1) with CSS over up to 8 channel paths (this at least to me resembles
> NPIV), what does the mdev really solve, can't you assign one of the channel
> paths of one of the several control units the device is connected to to
> plug into the VM? Well, clearly not, I'm just being curious and trying to
> understand the use case here.
I think it's easier to leave control units mostly out of the picture,
as they are not really relevant for the modelling here. And yes, there
is always exactly one subchannel for an I/O device.
Subchannels (mostly a concept describing how to interact with a device)
and channel paths (mostly corresponding to the way data flows to/from
the device) are probably best understood as different ways to describe
how devices are accessed. You don't really want to pass channel paths
(as they are shared between many devices); the subchannel is a more
natural unit to pass. The operating system also has way more control
over subchannels than over individual channel paths.
Gotcha.
Regarding mdev, the idea here is less to be able to slice up a device
as you do with GPUs, but more to be able to add a vendor driver that
does the actual interesting work (channel program translations etc.)
As to why we use the subchannel instead of the device, that's mostly
following from how the low-level channel I/O layer in the Linux kernel
is organized: The default I/O subchannel driver does a lot of things
that a normal device driver should not need to deal with (path
grouping, verification, status accumulation, etc.), but which would get
into the way if you wanted to assign the device via vfio. Therefore,
vfio-ccw implements an alternate I/O subchannel driver.
Okay, now it's much clearer what the mdev use case is.
Thanks Connie.
Erik
10:34 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
On 9/8/20 6:01 PM, Erik Skultety wrote:
> @@ -664,11 +695,11 @@
nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
> {
> virCommandPtr cmd;
> g_autofree char *json = NULL;
> - g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
> + g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
>
> - if (!parent_pci) {
> + if (!parent_addr) {
> virReportError(VIR_ERR_NO_NODE_DEVICE,
> - _("unable to find PCI address for parent device
'%s'"), def->parent);
> + _("unable to find address for parent device
'%s'"), def->parent);
I'm wondering whether "unable to find parent device '%s'" would not
suffice,
since we're not specifying what type of address we were not able to find - I'm
not even sure the address information is important at all.
Erik
Erik,
how about
_("unable to find parent device '%s' by its address"),
def->parent);
just to indicate the search criteria but I could also agree to a simple
_("unable to find parent device '%s'"), def->parent);
Your choice.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
12:34 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
On Fri, Sep 11, 2020 at 05:34:24PM +0200, Boris Fiuczynski wrote:
On 9/8/20 6:01 PM, Erik Skultety wrote:
> > @@ -664,11 +695,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr
def,
> > {
> > virCommandPtr cmd;
> > g_autofree char *json = NULL;
> > - g_autofree char *parent_pci =
nodeDeviceFindAddressByName(def->parent);
> > + g_autofree char *parent_addr =
nodeDeviceFindAddressByName(def->parent);
> >
> > - if (!parent_pci) {
> > + if (!parent_addr) {
> > virReportError(VIR_ERR_NO_NODE_DEVICE,
> > - _("unable to find PCI address for parent device
'%s'"), def->parent);
> > + _("unable to find address for parent device
'%s'"), def->parent);
> I'm wondering whether "unable to find parent device '%s'"
would not suffice,
> since we're not specifying what type of address we were not able to find -
I'm
> not even sure the address information is important at all.
>
> Erik
>
Erik,
how about
_("unable to find parent device '%s' by its address"),
def->parent);
just to indicate the search criteria but I could also agree to a simple
_("unable to find parent device '%s'"), def->parent);
I'd still go with the latter.
Erik
6:52 a.m.
New subject: [PATCH 5/5] node_device: mdev vfio-ccw support
On 9/14/20 7:34 AM, Erik Skultety wrote:
On Fri, Sep 11, 2020 at 05:34:24PM +0200, Boris Fiuczynski wrote:
> On 9/8/20 6:01 PM, Erik Skultety wrote:
>>> @@ -664,11 +695,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr
def,
>>> {
>>> virCommandPtr cmd;
>>> g_autofree char *json = NULL;
>>> - g_autofree char *parent_pci =
nodeDeviceFindAddressByName(def->parent);
>>> + g_autofree char *parent_addr =
nodeDeviceFindAddressByName(def->parent);
>>>
>>> - if (!parent_pci) {
>>> + if (!parent_addr) {
>>> virReportError(VIR_ERR_NO_NODE_DEVICE,
>>> - _("unable to find PCI address for parent device
'%s'"), def->parent);
>>> + _("unable to find address for parent device
'%s'"), def->parent);
>> I'm wondering whether "unable to find parent device '%s'"
would not suffice,
>> since we're not specifying what type of address we were not able to find -
I'm
>> not even sure the address information is important at all.
>>
>> Erik
>>
>
> Erik,
> how about
>
> _("unable to find parent device '%s' by its address"),
def->parent);
>
> just to indicate the search criteria but I could also agree to a simple
>
> _("unable to find parent device '%s'"), def->parent);
I'd still go with the latter.
Erik
OK, changed.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
2:49 a.m.
Bjoern Walk <bwalk(a)linux.ibm.com> [2020-08-24, 01:59PM +0200]:
This series enables support for channel subsystem (CSS) devices,
Direct Access Storage Devices (DASDs) in the node_device driver and
adds support to create vfio-ccw mdev devices in the node_device driver.
Boris Fiuczynski (5):
node_device: refactor udevProcessCCW
node_device: detect CSS devices
virsh: nodedev: ability to filter CSS capabilities
node_device: detect DASD devices
node_device: mdev vfio-ccw support
docs/formatnode.html.in | 12 +++
docs/manpages/virsh.rst | 2 +-
docs/schemas/nodedev.rng | 16 ++++
include/libvirt/libvirt-nodedev.h | 1 +
src/conf/domain_addr.c | 2 +-
src/conf/domain_addr.h | 3 +
src/conf/node_device_conf.c | 5 ++
src/conf/node_device_conf.h | 4 +-
src/conf/virnodedeviceobj.c | 4 +-
src/libvirt-nodedev.c | 1 +
src/libvirt_private.syms | 1 +
src/node_device/node_device_driver.c | 49 +++++++++---
src/node_device/node_device_udev.c | 77 ++++++++++++++++---
.../ccw_0_0_10000-invalid.xml | 4 +-
tests/nodedevschemadata/ccw_0_0_ffff.xml | 4 +-
tests/nodedevschemadata/css_0_0_ffff.xml | 10 +++
tests/nodedevxml2xmltest.c | 1 +
tools/virsh-nodedev.c | 3 +
18 files changed, 172 insertions(+), 27 deletions(-)
create mode 100644 tests/nodedevschemadata/css_0_0_ffff.xml
--
2.24.1
ping
--
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
6:59 a.m.
On Tue, Sep 01, 2020 at 09:49:46AM +0200, Bjoern Walk wrote:
Bjoern Walk <bwalk(a)linux.ibm.com> [2020-08-24, 01:59PM +0200]:
> This series enables support for channel subsystem (CSS) devices,
> Direct Access Storage Devices (DASDs) in the node_device driver and
> adds support to create vfio-ccw mdev devices in the node_device driver.
From a quick look I don't really have any design comments. I'm
leaving for PTO
tomorrow and be back on Monday, so I'll do a proper review next
week unless
someone else beats me to it.
Regards,
Erik
1561
days inactive
1582
days old
30 comments
4 participants
participants (4)
-
Bjoern Walk -
Boris Fiuczynski -
Cornelia Huck -
Erik Skultety