On 2/5/25 19:37, Peter Krempa wrote:
On Tue, Feb 04, 2025 at 18:11:41 +0100, Boris Fiuczynski wrote:
Add ccwgroup node device type supporting qeth generic driver.
Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
---
docs/manpages/virsh.rst | 6 +-
include/libvirt/libvirt-nodedev.h | 1 +
src/conf/node_device_conf.c | 212 ++++++++++++++++++
src/conf/node_device_conf.h | 29 ++-
src/conf/schemas/nodedev.rng | 43 ++++
src/conf/virnodedeviceobj.c | 4 +-
src/libvirt_private.syms | 2 +
src/node_device/node_device_driver.c | 5 +
src/node_device/node_device_udev.c | 41 ++++
src/util/virccw.c | 102 +++++++++
src/util/virccw.h | 22 ++
tests/nodedevschemadata/ccwgroup_0_0_bd00.xml | 20 ++
tests/nodedevxml2xmlout/ccwgroup_0_0_bd00.xml | 1 +
tests/nodedevxml2xmltest.c | 1 +
tools/virsh-nodedev.c | 3 +
15 files changed, 487 insertions(+), 5 deletions(-)
create mode 100644 tests/nodedevschemadata/ccwgroup_0_0_bd00.xml
create mode 120000 tests/nodedevxml2xmlout/ccwgroup_0_0_bd00.xml
[...]
+static int
+udevProcessCCWGroup(struct udev_device *device,
+ virNodeDeviceDef *def)
+{
+ const char *devtype = udev_device_get_devtype(device);
+ virNodeDevCapData *data = &def->caps->data;
+
+ data->ccwgroup_dev.address = virCCWDeviceAddressFromString(udev_device_get_sysname(device));
+
+ udevCCWGetState(device, &data->ccwgroup_dev.state);
+
+ udevGenerateDeviceName(device, def, NULL);
+
+ if ((data->ccwgroup_dev.type = virNodeDevCCWGroupCapTypeFromString(devtype)) < 0)
+ return -1;
Apart from the broken build where clang complains about assignment to
the unsigned enum which I've fixed, this doesn't report an error ...
Peter, thanks for fixing the clang problem.
I agree that here I should add reporting an internal error as this
should not happen since the devtype was already used before in the udev
event handling to get here and not being able to do the conversion would
be an error. I will send a patch shortly.
+
+ switch (data->ccwgroup_dev.type) {
+ case VIR_NODE_DEV_CAP_CCWGROUP_QETH_GENERIC:
+ {
+ virCCWGroupTypeQeth *qeth = &data->ccwgroup_dev.qeth;
+ /* process qeth device information */
+ udevGetStringSysfsAttr(device, "card_type", &qeth->card_type);
+ udevGetStringSysfsAttr(device, "chpid", &qeth->chpid);
+ }
+ break;
+ case VIR_NODE_DEV_CAP_CCWGROUP_LAST:
+ return -1;
+ }
+
+ if (virNodeDeviceGetCCWGroupDynamicCaps(def->sysfs_path,
+ &data->ccwgroup_dev) < 0)
... and neither this at least on 2 code paths:
1) the dummy impl if not running on linux
2) if the assertion that a group must be non-empty fails in the real
code path inside virCCWGroupDeviceGetMembers.
This is implemented following the common DynamicCaps pattern, e.g. as in
virNodeDeviceGetMdevParentDynamicCaps.
Therefore I would refrain from adding error reporting.
The caller does seem to care about errors as other code paths do set
them, thus this should as well.
+ return -1;
+
+ return 0;
+}
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
