[PATCH 00/10] Make mdev_types generic and support it on CSS devices

Since channel subsystem devices can have the capability to create an mdev device they should expose this by listing the mdev_types capability. Many patches of this series is concerned with refactoring existing PCI code. Boris Fiuczynski (10): conf: node_device: fix mdev_types format and XML parsing code to match schema util: refactor mdev_types method from PCI to mdev util: refactor mdev_types methods return code usage conf: node_devive: refactor GetPCIMdevTypesCaps into GetMdevTypeCapes conf: node_device: refactor capability mdev_types formating conf: node_device: refactor mdev_types XML parsing conf: node_device: refactor CSS formating schema: refactor mdev_types out of PCI nodedev schema conf: node_device: cleanup virNodeDevCapCCWParseXML node_device: detecting mdev_types capability on CSS devices docs/drvnodedev.html.in | 5 +- docs/formatnode.html.in | 39 ++ docs/schemas/nodedev.rng | 52 ++- src/conf/node_device_conf.c | 365 ++++++++++++------ src/conf/node_device_conf.h | 11 + src/conf/virnodedeviceobj.c | 7 +- src/libvirt_private.syms | 3 +- src/node_device/node_device_udev.c | 3 + src/util/virmdev.c | 67 ++++ src/util/virmdev.h | 5 + src/util/virpci.c | 60 --- src/util/virpci.h | 3 - .../css_0_0_fffe_mdev_types.xml | 17 + tests/nodedevxml2xmltest.c | 1 + 14 files changed, 428 insertions(+), 210 deletions(-) create mode 100644 tests/nodedevschemadata/css_0_0_fffe_mdev_types.xml -- 2.25.1

The nodedev schema defines that a mdev_types capability must have one or more type elements. The XML parsing and the format allows to accept and to write mdev_types capability without any type element. This patches fixes this. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 40 ++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4adfdef572..3fbe9338ee 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -276,25 +276,27 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf, virPCIHeaderTypeToString(data->pci_dev.hdrType)); } if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) { - virBufferAddLit(buf, "<capability type='mdev_types'>\n"); - virBufferAdjustIndent(buf, 2); - for (i = 0; i < data->pci_dev.nmdev_types; i++) { - virMediatedDeviceTypePtr type = data->pci_dev.mdev_types[i]; - virBufferEscapeString(buf, "<type id='%s'>\n", type->id); + if (data->pci_dev.nmdev_types > 0) { + virBufferAddLit(buf, "<capability type='mdev_types'>\n"); virBufferAdjustIndent(buf, 2); - if (type->name) - virBufferEscapeString(buf, "<name>%s</name>\n", - type->name); - virBufferEscapeString(buf, "<deviceAPI>%s</deviceAPI>\n", - type->device_api); - virBufferAsprintf(buf, - "<availableInstances>%u</availableInstances>\n", - type->available_instances); + for (i = 0; i < data->pci_dev.nmdev_types; i++) { + virMediatedDeviceTypePtr type = data->pci_dev.mdev_types[i]; + virBufferEscapeString(buf, "<type id='%s'>\n", type->id); + virBufferAdjustIndent(buf, 2); + if (type->name) + virBufferEscapeString(buf, "<name>%s</name>\n", + type->name); + virBufferEscapeString(buf, "<deviceAPI>%s</deviceAPI>\n", + type->device_api); + virBufferAsprintf(buf, + "<availableInstances>%u</availableInstances>\n", + type->available_instances); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</type>\n"); + } virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</type>\n"); + virBufferAddLit(buf, "</capability>\n"); } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</capability>\n"); } if (data->pci_dev.nIommuGroupDevices) { virBufferAsprintf(buf, "<iommuGroup number='%d'>\n", @@ -1520,6 +1522,12 @@ virNodeDevPCICapMdevTypesParseXML(xmlXPathContextPtr ctxt, if ((nmdev_types = virXPathNodeSet("./type", ctxt, &nodes)) < 0) goto cleanup; + if (nmdev_types == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing <type> element in <capability> element")); + goto cleanup; + } + orignode = ctxt->node; for (i = 0; i < nmdev_types; i++) { ctxt->node = nodes[i]; -- 2.25.1

On a Friday in 2020, Boris Fiuczynski wrote:
The nodedev schema defines that a mdev_types capability must have one or more type elements. The XML parsing and the format allows to accept and to write mdev_types capability without any type element. This patches fixes this.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 40 ++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Extract virPCIGetMdevTypes from PCI as virMediatedDeviceGetMdevTypes into mdev for later reuse. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virmdev.c | 65 +++++++++++++++++++++++++++++++++++++ src/util/virmdev.h | 4 +++ src/util/virpci.c | 60 ---------------------------------- src/util/virpci.h | 3 -- 6 files changed, 71 insertions(+), 65 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 3fbe9338ee..db1258436a 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2584,7 +2584,7 @@ virNodeDeviceGetPCIMdevTypesCaps(const char *sysfspath, pci_dev->nmdev_types = 0; pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; - rc = virPCIGetMdevTypes(sysfspath, &types); + rc = virMediatedDeviceGetMdevTypes(sysfspath, &types); if (rc <= 0) return rc; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 95e50835ad..9029ea4fa2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2509,6 +2509,7 @@ virMediatedDeviceAttrNew; virMediatedDeviceFree; virMediatedDeviceGetIOMMUGroupDev; virMediatedDeviceGetIOMMUGroupNum; +virMediatedDeviceGetMdevTypes; virMediatedDeviceGetSysfsPath; virMediatedDeviceGetUsedBy; virMediatedDeviceIsUsed; @@ -2845,7 +2846,6 @@ virPCIELinkSpeedTypeFromString; virPCIELinkSpeedTypeToString; virPCIGetDeviceAddressFromSysfsLink; virPCIGetHeaderType; -virPCIGetMdevTypes; virPCIGetNetName; virPCIGetPhysicalFunction; virPCIGetVirtualFunctionIndex; diff --git a/src/util/virmdev.c b/src/util/virmdev.c index 31994631ed..80f5f2a767 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -522,3 +522,68 @@ void virMediatedDeviceAttrFree(virMediatedDeviceAttrPtr attr) g_free(attr->value); g_free(attr); } + + +#ifdef __linux__ + +ssize_t +virMediatedDeviceGetMdevTypes(const char *sysfspath, + virMediatedDeviceTypePtr **types) +{ + ssize_t ret = -1; + int dirret = -1; + DIR *dir = NULL; + struct dirent *entry; + g_autofree char *types_path = NULL; + g_autoptr(virMediatedDeviceType) mdev_type = NULL; + virMediatedDeviceTypePtr *mdev_types = NULL; + size_t ntypes = 0; + size_t i; + + types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath); + + if ((dirret = virDirOpenIfExists(&dir, types_path)) < 0) + goto cleanup; + + if (dirret == 0) { + ret = 0; + goto cleanup; + } + + while ((dirret = virDirRead(dir, &entry, types_path)) > 0) { + g_autofree char *tmppath = NULL; + /* append the type id to the path and read the attributes from there */ + tmppath = g_strdup_printf("%s/%s", types_path, entry->d_name); + + if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0) + goto cleanup; + + if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0) + goto cleanup; + } + + if (dirret < 0) + goto cleanup; + + *types = g_steal_pointer(&mdev_types); + ret = ntypes; + ntypes = 0; + cleanup: + for (i = 0; i < ntypes; i++) + virMediatedDeviceTypeFree(mdev_types[i]); + VIR_FREE(mdev_types); + VIR_DIR_CLOSE(dir); + return ret; +} + +#else + +ssize_t +virMediatedDeviceGetMdevTypes(const char *sysfspath G_GNUC_UNUSED, + virMediatedDeviceTypePtr **types G_GNUC_UNUSED) +{ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); + return -1; +} + +#endif /* __linux__ */ diff --git a/src/util/virmdev.h b/src/util/virmdev.h index eb167ccb48..846e1662e7 100644 --- a/src/util/virmdev.h +++ b/src/util/virmdev.h @@ -149,5 +149,9 @@ int virMediatedDeviceTypeReadAttrs(const char *sysfspath, virMediatedDeviceTypePtr *type); +ssize_t +virMediatedDeviceGetMdevTypes(const char *sysfspath, + virMediatedDeviceTypePtr **types); + G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDevice, virMediatedDeviceFree); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDeviceType, virMediatedDeviceTypeFree); diff --git a/src/util/virpci.c b/src/util/virpci.c index 1f679a7b45..2086a4270b 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2528,57 +2528,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, return 0; } - -ssize_t -virPCIGetMdevTypes(const char *sysfspath, - virMediatedDeviceTypePtr **types) -{ - ssize_t ret = -1; - int dirret = -1; - DIR *dir = NULL; - struct dirent *entry; - g_autofree char *types_path = NULL; - g_autoptr(virMediatedDeviceType) mdev_type = NULL; - virMediatedDeviceTypePtr *mdev_types = NULL; - size_t ntypes = 0; - size_t i; - - types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath); - - if ((dirret = virDirOpenIfExists(&dir, types_path)) < 0) - goto cleanup; - - if (dirret == 0) { - ret = 0; - goto cleanup; - } - - while ((dirret = virDirRead(dir, &entry, types_path)) > 0) { - g_autofree char *tmppath = NULL; - /* append the type id to the path and read the attributes from there */ - tmppath = g_strdup_printf("%s/%s", types_path, entry->d_name); - - if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0) - goto cleanup; - - if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0) - goto cleanup; - } - - if (dirret < 0) - goto cleanup; - - *types = g_steal_pointer(&mdev_types); - ret = ntypes; - ntypes = 0; - cleanup: - for (i = 0; i < ntypes; i++) - virMediatedDeviceTypeFree(mdev_types[i]); - VIR_FREE(mdev_types); - VIR_DIR_CLOSE(dir); - return ret; -} - #else static const char *unsupported = N_("not supported on non-linux platforms"); @@ -2653,15 +2602,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path G_GNUC_UNUSED, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); return -1; } - - -ssize_t -virPCIGetMdevTypes(const char *sysfspath G_GNUC_UNUSED, - virMediatedDeviceTypePtr **types G_GNUC_UNUSED) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); - return -1; -} #endif /* __linux__ */ int diff --git a/src/util/virpci.h b/src/util/virpci.h index 1f896ca481..43828b0a8a 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -275,9 +275,6 @@ int virPCIGetHeaderType(virPCIDevicePtr dev, int *hdrType); void virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev); -ssize_t virPCIGetMdevTypes(const char *sysfspath, - virMediatedDeviceType ***types); - void virPCIDeviceAddressFree(virPCIDeviceAddressPtr address); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIDevice, virPCIDeviceFree); -- 2.25.1

On a Friday in 2020, Boris Fiuczynski wrote:
Extract virPCIGetMdevTypes from PCI as virMediatedDeviceGetMdevTypes into mdev for later reuse.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virmdev.c | 65 +++++++++++++++++++++++++++++++++++++ src/util/virmdev.h | 4 +++ src/util/virpci.c | 60 ---------------------------------- src/util/virpci.h | 3 -- 6 files changed, 71 insertions(+), 65 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c index 1f679a7b45..2086a4270b 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2528,57 +2528,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, return 0; }
- -ssize_t -virPCIGetMdevTypes(const char *sysfspath, - virMediatedDeviceTypePtr **types) -{ - ssize_t ret = -1; - int dirret = -1; - DIR *dir = NULL;
commit c0ae4919e386cda6e21d3ba022ee187e8b09793b change DIR* int g_autoptr(DIR) where appropriate touched this code so it no longer applies cleanly.
- struct dirent *entry; - g_autofree char *types_path = NULL; - g_autoptr(virMediatedDeviceType) mdev_type = NULL; - virMediatedDeviceTypePtr *mdev_types = NULL; - size_t ntypes = 0; - size_t i; - - types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath); - - if ((dirret = virDirOpenIfExists(&dir, types_path)) < 0) - goto cleanup; - - if (dirret == 0) { - ret = 0; - goto cleanup; - } - - while ((dirret = virDirRead(dir, &entry, types_path)) > 0) { - g_autofree char *tmppath = NULL; - /* append the type id to the path and read the attributes from there */ - tmppath = g_strdup_printf("%s/%s", types_path, entry->d_name); - - if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0) - goto cleanup; - - if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0) - goto cleanup; - } - - if (dirret < 0) - goto cleanup; - - *types = g_steal_pointer(&mdev_types); - ret = ntypes; - ntypes = 0; - cleanup: - for (i = 0; i < ntypes; i++) - virMediatedDeviceTypeFree(mdev_types[i]); - VIR_FREE(mdev_types); - VIR_DIR_CLOSE(dir); - return ret; -} - #else static const char *unsupported = N_("not supported on non-linux platforms");
This also needs to be copied to the new file...
@@ -2653,15 +2602,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path G_GNUC_UNUSED, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); return -1; } - - -ssize_t -virPCIGetMdevTypes(const char *sysfspath G_GNUC_UNUSED, - virMediatedDeviceTypePtr **types G_GNUC_UNUSED) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
otherwise this won't compile.
- return -1; -} #endif /* __linux__ */
With that fixed (no need to resend - this is a refactor so I'll push it regardless of the rest of the series): Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On 11/4/20 6:38 PM, Ján Tomko wrote:
On a Friday in 2020, Boris Fiuczynski wrote:
Extract virPCIGetMdevTypes from PCI as virMediatedDeviceGetMdevTypes into mdev for later reuse.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virmdev.c | 65 +++++++++++++++++++++++++++++++++++++ src/util/virmdev.h | 4 +++ src/util/virpci.c | 60 ---------------------------------- src/util/virpci.h | 3 -- 6 files changed, 71 insertions(+), 65 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c index 1f679a7b45..2086a4270b 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2528,57 +2528,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, return 0; }
- -ssize_t -virPCIGetMdevTypes(const char *sysfspath, - virMediatedDeviceTypePtr **types) -{ - ssize_t ret = -1; - int dirret = -1; - DIR *dir = NULL;
commit c0ae4919e386cda6e21d3ba022ee187e8b09793b change DIR* int g_autoptr(DIR) where appropriate
touched this code so it no longer applies cleanly.
yes, correct. Laine's change was commited after I sent this series.
- struct dirent *entry; - g_autofree char *types_path = NULL; - g_autoptr(virMediatedDeviceType) mdev_type = NULL; - virMediatedDeviceTypePtr *mdev_types = NULL; - size_t ntypes = 0; - size_t i; - - types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath); - - if ((dirret = virDirOpenIfExists(&dir, types_path)) < 0) - goto cleanup; - - if (dirret == 0) { - ret = 0; - goto cleanup; - } - - while ((dirret = virDirRead(dir, &entry, types_path)) > 0) { - g_autofree char *tmppath = NULL; - /* append the type id to the path and read the attributes from there */ - tmppath = g_strdup_printf("%s/%s", types_path, entry->d_name); - - if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0) - goto cleanup; - - if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0) - goto cleanup; - } - - if (dirret < 0) - goto cleanup; - - *types = g_steal_pointer(&mdev_types); - ret = ntypes; - ntypes = 0; - cleanup: - for (i = 0; i < ntypes; i++) - virMediatedDeviceTypeFree(mdev_types[i]); - VIR_FREE(mdev_types); - VIR_DIR_CLOSE(dir); - return ret; -} - #else static const char *unsupported = N_("not supported on non-linux platforms");
This also needs to be copied to the new file...
Sorry for missing this.
@@ -2653,15 +2602,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path G_GNUC_UNUSED, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); return -1; } - - -ssize_t -virPCIGetMdevTypes(const char *sysfspath G_GNUC_UNUSED, - virMediatedDeviceTypePtr **types G_GNUC_UNUSED) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
otherwise this won't compile.
Yes, that is correct. I did my checks on a supported platform only and therefore did not catch this.
- return -1; -} #endif /* __linux__ */
With that fixed (no need to resend - this is a refactor so I'll push it regardless of the rest of the series):
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Thanks for fixing this and also for your review of this and the other refactoring patches ... and for pushing them. :-) -- 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

Remove mix of array length and error code in the return code. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 10 ++++------ src/util/virmdev.c | 14 ++++++++------ src/util/virmdev.h | 3 ++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index db1258436a..af8edded3c 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2574,7 +2574,7 @@ virNodeDeviceGetPCIMdevTypesCaps(const char *sysfspath, virNodeDevCapPCIDevPtr pci_dev) { virMediatedDeviceTypePtr *types = NULL; - int rc = 0; + size_t ntypes = 0; size_t i; /* this could be a refresh, so clear out the old data */ @@ -2584,13 +2584,11 @@ virNodeDeviceGetPCIMdevTypesCaps(const char *sysfspath, pci_dev->nmdev_types = 0; pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; - rc = virMediatedDeviceGetMdevTypes(sysfspath, &types); - - if (rc <= 0) - return rc; + if (virMediatedDeviceGetMdevTypes(sysfspath, &types, &ntypes) < 0) + return -1; pci_dev->mdev_types = g_steal_pointer(&types); - pci_dev->nmdev_types = rc; + pci_dev->nmdev_types = ntypes; pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; return 0; diff --git a/src/util/virmdev.c b/src/util/virmdev.c index 80f5f2a767..b02005bd1a 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -528,7 +528,8 @@ void virMediatedDeviceAttrFree(virMediatedDeviceAttrPtr attr) ssize_t virMediatedDeviceGetMdevTypes(const char *sysfspath, - virMediatedDeviceTypePtr **types) + virMediatedDeviceTypePtr **types, + size_t *ntypes) { ssize_t ret = -1; int dirret = -1; @@ -537,7 +538,7 @@ virMediatedDeviceGetMdevTypes(const char *sysfspath, g_autofree char *types_path = NULL; g_autoptr(virMediatedDeviceType) mdev_type = NULL; virMediatedDeviceTypePtr *mdev_types = NULL; - size_t ntypes = 0; + size_t nmdev_types = 0; size_t i; types_path = g_strdup_printf("%s/mdev_supported_types", sysfspath); @@ -558,7 +559,7 @@ virMediatedDeviceGetMdevTypes(const char *sysfspath, if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0) + if (VIR_APPEND_ELEMENT(mdev_types, nmdev_types, mdev_type) < 0) goto cleanup; } @@ -566,10 +567,11 @@ virMediatedDeviceGetMdevTypes(const char *sysfspath, goto cleanup; *types = g_steal_pointer(&mdev_types); - ret = ntypes; - ntypes = 0; + *ntypes = nmdev_types; + nmdev_types = 0; + ret = 0; cleanup: - for (i = 0; i < ntypes; i++) + for (i = 0; i < nmdev_types; i++) virMediatedDeviceTypeFree(mdev_types[i]); VIR_FREE(mdev_types); VIR_DIR_CLOSE(dir); diff --git a/src/util/virmdev.h b/src/util/virmdev.h index 846e1662e7..b6563a94fc 100644 --- a/src/util/virmdev.h +++ b/src/util/virmdev.h @@ -151,7 +151,8 @@ virMediatedDeviceTypeReadAttrs(const char *sysfspath, ssize_t virMediatedDeviceGetMdevTypes(const char *sysfspath, - virMediatedDeviceTypePtr **types); + virMediatedDeviceTypePtr **types, + size_t *ntypes); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDevice, virMediatedDeviceFree); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMediatedDeviceType, virMediatedDeviceTypeFree); -- 2.25.1

On a Friday in 2020, Boris Fiuczynski wrote:
Remove mix of array length and error code in the return code.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 10 ++++------ src/util/virmdev.c | 14 ++++++++------ src/util/virmdev.h | 3 ++- 3 files changed, 14 insertions(+), 13 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Extracting PCI from virNodeDeviceGetPCIMdevTypesCaps creating virNodeDeviceGetMdevTypesCaps to make later reuse possible. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index af8edded3c..811d102208 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2570,26 +2570,25 @@ virNodeDeviceGetPCIIOMMUGroupCaps(virNodeDevCapPCIDevPtr pci_dev) static int -virNodeDeviceGetPCIMdevTypesCaps(const char *sysfspath, - virNodeDevCapPCIDevPtr pci_dev) +virNodeDeviceGetMdevTypesCaps(const char *sysfspath, + virMediatedDeviceTypePtr **mdev_types, + size_t *nmdev_types) { virMediatedDeviceTypePtr *types = NULL; size_t ntypes = 0; size_t i; /* this could be a refresh, so clear out the old data */ - for (i = 0; i < pci_dev->nmdev_types; i++) - virMediatedDeviceTypeFree(pci_dev->mdev_types[i]); - VIR_FREE(pci_dev->mdev_types); - pci_dev->nmdev_types = 0; - pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; + for (i = 0; i < *nmdev_types; i++) + virMediatedDeviceTypeFree(*mdev_types[i]); + VIR_FREE(*mdev_types); + *nmdev_types = 0; if (virMediatedDeviceGetMdevTypes(sysfspath, &types, &ntypes) < 0) return -1; - pci_dev->mdev_types = g_steal_pointer(&types); - pci_dev->nmdev_types = ntypes; - pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; + *mdev_types = g_steal_pointer(&types); + *nmdev_types = ntypes; return 0; } @@ -2606,9 +2605,17 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath, virNodeDevCapPCIDevPtr pci_dev) { if (virNodeDeviceGetPCISRIOVCaps(sysfsPath, pci_dev) < 0 || - virNodeDeviceGetPCIIOMMUGroupCaps(pci_dev) < 0 || - virNodeDeviceGetPCIMdevTypesCaps(sysfsPath, pci_dev) < 0) + virNodeDeviceGetPCIIOMMUGroupCaps(pci_dev) < 0) + return -1; + + pci_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; + if (virNodeDeviceGetMdevTypesCaps(sysfsPath, + &pci_dev->mdev_types, + &pci_dev->nmdev_types) < 0) return -1; + if (pci_dev->nmdev_types > 0) + pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; + return 0; } -- 2.25.1

s/devive/device/ in the commit summary On a Friday in 2020, Boris Fiuczynski wrote:
Extracting PCI from virNodeDeviceGetPCIMdevTypesCaps creating virNodeDeviceGetMdevTypesCaps to make later reuse possible.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Extract the XML formating for mdev_types from PCI capability into a generic standalone method for later reuse. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 55 +++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 811d102208..dce04ad4f0 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -206,6 +206,37 @@ virNodeDeviceCapSystemDefFormat(virBufferPtr buf, } +static void +virNodeDeviceCapMdevTypesFormat(virBufferPtr buf, + virMediatedDeviceTypePtr *mdev_types, + const size_t nmdev_types) +{ + size_t i; + + if (nmdev_types > 0) { + virBufferAddLit(buf, "<capability type='mdev_types'>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < nmdev_types; i++) { + virMediatedDeviceTypePtr type = mdev_types[i]; + virBufferEscapeString(buf, "<type id='%s'>\n", type->id); + virBufferAdjustIndent(buf, 2); + if (type->name) + virBufferEscapeString(buf, "<name>%s</name>\n", + type->name); + virBufferEscapeString(buf, "<deviceAPI>%s</deviceAPI>\n", + type->device_api); + virBufferAsprintf(buf, + "<availableInstances>%u</availableInstances>\n", + type->available_instances); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</type>\n"); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</capability>\n"); + } +} + + static void virNodeDeviceCapPCIDefFormat(virBufferPtr buf, const virNodeDevCapData *data) @@ -276,27 +307,9 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf, virPCIHeaderTypeToString(data->pci_dev.hdrType)); } if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) { - if (data->pci_dev.nmdev_types > 0) { - virBufferAddLit(buf, "<capability type='mdev_types'>\n"); - virBufferAdjustIndent(buf, 2); - for (i = 0; i < data->pci_dev.nmdev_types; i++) { - virMediatedDeviceTypePtr type = data->pci_dev.mdev_types[i]; - virBufferEscapeString(buf, "<type id='%s'>\n", type->id); - virBufferAdjustIndent(buf, 2); - if (type->name) - virBufferEscapeString(buf, "<name>%s</name>\n", - type->name); - virBufferEscapeString(buf, "<deviceAPI>%s</deviceAPI>\n", - type->device_api); - virBufferAsprintf(buf, - "<availableInstances>%u</availableInstances>\n", - type->available_instances); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</type>\n"); - } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</capability>\n"); - } + virNodeDeviceCapMdevTypesFormat(buf, + data->pci_dev.mdev_types, + data->pci_dev.nmdev_types); } if (data->pci_dev.nIommuGroupDevices) { virBufferAsprintf(buf, "<iommuGroup number='%d'>\n", -- 2.25.1

On a Friday in 2020, Boris Fiuczynski wrote:
Extract the XML formating for mdev_types from PCI capability into a generic standalone method for later reuse.
s/formating/formatting/g
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 55 +++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 21 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Extract PCI code from virNodeDevPCICapMdevTypesParseXML to make method virNodeDevCapMdevTypesParseXML generic for later reuse. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 141 ++++++++++++++++++------------------ 1 file changed, 72 insertions(+), 69 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index dce04ad4f0..6fd5b1b038 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -757,6 +757,72 @@ virNodeDevCapDRMParseXML(xmlXPathContextPtr ctxt, } +static int +virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt, + virMediatedDeviceTypePtr **mdev_types, + size_t *nmdev_types) +{ + int ret = -1; + xmlNodePtr orignode = NULL; + xmlNodePtr *nodes = NULL; + int ntypes = -1; + virMediatedDeviceTypePtr type = NULL; + size_t i; + + if ((ntypes = virXPathNodeSet("./type", ctxt, &nodes)) < 0) + goto cleanup; + + if (nmdev_types == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing <type> element in <capability> element")); + goto cleanup; + } + + orignode = ctxt->node; + for (i = 0; i < ntypes; i++) { + ctxt->node = nodes[i]; + + type = g_new0(virMediatedDeviceType, 1); + + if (!(type->id = virXPathString("string(./@id[1])", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'id' attribute for mediated device's " + "<type> element")); + goto cleanup; + } + + if (!(type->device_api = virXPathString("string(./deviceAPI[1])", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing device API for mediated device type '%s'"), + type->id); + goto cleanup; + } + + if (virXPathUInt("number(./availableInstances)", ctxt, + &type->available_instances) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("missing number of available instances for " + "mediated device type '%s'"), + type->id); + goto cleanup; + } + + type->name = virXPathString("string(./name)", ctxt); + + if (VIR_APPEND_ELEMENT(*mdev_types, + *nmdev_types, type) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(nodes); + virMediatedDeviceTypeFree(type); + ctxt->node = orignode; + return ret; +} + + static int virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -1521,72 +1587,6 @@ virNodeDevPCICapSRIOVVirtualParseXML(xmlXPathContextPtr ctxt, } -static int -virNodeDevPCICapMdevTypesParseXML(xmlXPathContextPtr ctxt, - virNodeDevCapPCIDevPtr pci_dev) -{ - int ret = -1; - xmlNodePtr orignode = NULL; - xmlNodePtr *nodes = NULL; - int nmdev_types = -1; - virMediatedDeviceTypePtr type = NULL; - size_t i; - - if ((nmdev_types = virXPathNodeSet("./type", ctxt, &nodes)) < 0) - goto cleanup; - - if (nmdev_types == 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing <type> element in <capability> element")); - goto cleanup; - } - - orignode = ctxt->node; - for (i = 0; i < nmdev_types; i++) { - ctxt->node = nodes[i]; - - type = g_new0(virMediatedDeviceType, 1); - - if (!(type->id = virXPathString("string(./@id[1])", ctxt))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing 'id' attribute for mediated device's " - "<type> element")); - goto cleanup; - } - - if (!(type->device_api = virXPathString("string(./deviceAPI[1])", ctxt))) { - virReportError(VIR_ERR_XML_ERROR, - _("missing device API for mediated device type '%s'"), - type->id); - goto cleanup; - } - - if (virXPathUInt("number(./availableInstances)", ctxt, - &type->available_instances) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("missing number of available instances for " - "mediated device type '%s'"), - type->id); - goto cleanup; - } - - type->name = virXPathString("string(./name)", ctxt); - - if (VIR_APPEND_ELEMENT(pci_dev->mdev_types, - pci_dev->nmdev_types, type) < 0) - goto cleanup; - } - - pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; - ret = 0; - cleanup: - VIR_FREE(nodes); - virMediatedDeviceTypeFree(type); - ctxt->node = orignode; - return ret; -} - - static int virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node, @@ -1609,9 +1609,12 @@ virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt, } else if (STREQ(type, "virt_functions") && virNodeDevPCICapSRIOVVirtualParseXML(ctxt, pci_dev) < 0) { goto cleanup; - } else if (STREQ(type, "mdev_types") && - virNodeDevPCICapMdevTypesParseXML(ctxt, pci_dev) < 0) { - goto cleanup; + } else if (STREQ(type, "mdev_types")) { + if (virNodeDevCapMdevTypesParseXML(ctxt, + &pci_dev->mdev_types, + &pci_dev->nmdev_types) < 0) + goto cleanup; + pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; } else { int hdrType = virPCIHeaderTypeFromString(type); -- 2.25.1

On a Friday in 2020, Boris Fiuczynski wrote:
Extract PCI code from virNodeDevPCICapMdevTypesParseXML to make method virNodeDevCapMdevTypesParseXML generic for later reuse.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 141 ++++++++++++++++++------------------ 1 file changed, 72 insertions(+), 69 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Move XML formating code into a new method. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 6fd5b1b038..ebabf20b67 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -533,6 +533,20 @@ virNodeDeviceCapMdevDefFormat(virBufferPtr buf, } } + +static void +virNodeDeviceCapCCWDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + 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); +} + + char * virNodeDeviceDefFormat(const virNodeDeviceDef *def) { @@ -619,12 +633,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) 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", - data->ccw_dev.ssid); - virBufferAsprintf(&buf, "<devno>0x%04x</devno>\n", - data->ccw_dev.devno); + virNodeDeviceCapCCWDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: -- 2.25.1

On a Friday in 2020, Boris Fiuczynski wrote:
Move XML formating code into a new method.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
s/formating/formatting/g Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Refactor mdev_types into standalone define for later reuse. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- docs/schemas/nodedev.rng | 48 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 166e278cf8..9548412999 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -215,27 +215,7 @@ </optional> <optional> - <element name="capability"> - <attribute name="type"> - <value>mdev_types</value> - </attribute> - <oneOrMore> - <element name="type"> - <attribute name="id"> - <data type="string"/> - </attribute> - <optional> - <element name="name"><text/></element> - </optional> - <element name="deviceAPI"> - <value>vfio-pci</value> - </element> - <element name="availableInstances"> - <ref name="unsignedInt"/> - </element> - </element> - </oneOrMore> - </element> + <ref name="mdev_types"/> </optional> <optional> @@ -696,4 +676,30 @@ </data> </define> + <define name="mdev_types"> + <element name="capability"> + <attribute name="type"> + <value>mdev_types</value> + </attribute> + <oneOrMore> + <element name="type"> + <attribute name="id"> + <data type="string"/> + </attribute> + <optional> + <element name="name"><text/></element> + </optional> + <element name="deviceAPI"> + <choice> + <value>vfio-pci</value> + </choice> + </element> + <element name="availableInstances"> + <ref name="unsignedInt"/> + </element> + </element> + </oneOrMore> + </element> + </define> + </grammar> -- 2.25.1

On a Friday in 2020, Boris Fiuczynski wrote:
Refactor mdev_types into standalone define for later reuse.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- docs/schemas/nodedev.rng | 48 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 21 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Make use of g_autofree Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index ebabf20b67..39950565b5 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -839,57 +839,52 @@ virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, virNodeDevCapCCWPtr ccw_dev) { VIR_XPATH_NODE_AUTORESTORE(ctxt) - int ret = -1; - char *cssid = NULL, *ssid = NULL, *devno = NULL; + g_autofree char *cssid = NULL; + g_autofree char *ssid = NULL; + g_autofree char *devno = NULL; ctxt->node = node; - if (!(cssid = virXPathString("string(./cssid[1])", ctxt))) { + if (!(cssid = virXPathString("string(./cssid[1])", ctxt))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("missing cssid value for '%s'"), def->name); - goto out; + return -1; } 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; + return -1; } if (!(ssid = virXPathString("string(./ssid[1])", ctxt))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("missing ssid value for '%s'"), def->name); - goto out; + return -1; } 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; + return -1; } if (!(devno = virXPathString("string(./devno[1])", ctxt))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("missing devno value for '%s'"), def->name); - goto out; + return -1; } 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; + return -1; } - ret = 0; - - out: - VIR_FREE(cssid); - VIR_FREE(ssid); - VIR_FREE(devno); - return ret; + return 0; } -- 2.25.1

On a Friday in 2020, Boris Fiuczynski wrote:
Make use of g_autofree
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/node_device_conf.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Add detection of mdev_types capability to channel subsystem devices. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- docs/drvnodedev.html.in | 5 +- docs/formatnode.html.in | 39 ++++++++ docs/schemas/nodedev.rng | 4 + src/conf/node_device_conf.c | 92 ++++++++++++++++++- src/conf/node_device_conf.h | 11 +++ src/conf/virnodedeviceobj.c | 7 +- src/libvirt_private.syms | 1 + src/node_device/node_device_udev.c | 3 + .../css_0_0_fffe_mdev_types.xml | 17 ++++ tests/nodedevxml2xmltest.c | 1 + 10 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 tests/nodedevschemadata/css_0_0_fffe_mdev_types.xml diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in index 0823c1888d..d5191d6d93 100644 --- a/docs/drvnodedev.html.in +++ b/docs/drvnodedev.html.in @@ -139,12 +139,13 @@ <h3><a id="MDEVCap">MDEV capability</a></h3> <p> - A PCI device capable of creating mediated devices will include a nested + A device capable of creating mediated devices will include a nested capability <code>mdev_types</code> which enumerates all supported mdev types on the physical device, along with the type attributes available through sysfs. A detailed description of the XML format for the <code>mdev_types</code> capability can be found - <a href="formatnode.html#MDEVCap">here</a>. + <a href="formatnode.html#MDEVCap">here for PCI</a> or + <a href="formatnode.html#MDEVCapCSS">here for CSS</a>. </p> <p> The following example shows how we might represent an NVIDIA GPU device diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 594427468b..7f3c71941d 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -430,6 +430,45 @@ <dd>The subchannel-set identifier.</dd> <dt><code>devno</code></dt> <dd>The device number.</dd> + <dt><code>capability</code></dt> + <dd> + This optional element can occur multiple times. If it + exists, it has a mandatory <code>type</code> attribute + which will be set to: + <dl> + <dt><code><a id="MDEVCapCSS">mdev_types</a></code></dt> + <dd> + This device is capable of creating mediated devices, and + the capability will contain a list of <code>type</code> + elements, which list all mdev types supported on the + physical device. <span class="since">Since 6.9.0</span> + Each <code>type</code> element has a single <code>id</code> + attribute that holds an official vendor-supplied identifier + for the type. It supports the following sub-elements: + <dl> + <dt><code>name</code></dt> + <dd> + The <code>name</code> element holds a vendor-supplied + code name for the given mediated device type. This is + an optional element. + </dd> + <dt><code>deviceAPI</code></dt> + <dd> + The value of this element describes how an instance of + the given type will be presented to the guest by the + VFIO framework. + </dd> + <dt><code>availableInstances</code></dt> + <dd> + This element reports the current state of resource + allocation. In other words, how many instances of the + given type can still be successfully created on the + physical device. + </dd> + </dl> + </dd> + </dl> + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 9548412999..d3248e90a9 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -653,6 +653,9 @@ <element name="devno"> <ref name="ccwDevnoRange"/> </element> + <optional> + <ref name="mdev_types"/> + </optional> </define> <define name="address"> @@ -692,6 +695,7 @@ <element name="deviceAPI"> <choice> <value>vfio-pci</value> + <value>vfio-ccw</value> </choice> </element> <element name="availableInstances"> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 39950565b5..75c033b0e1 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -544,6 +544,10 @@ virNodeDeviceCapCCWDefFormat(virBufferPtr buf, data->ccw_dev.ssid); virBufferAsprintf(buf, "<devno>0x%04x</devno>\n", data->ccw_dev.devno); + if (data->ccw_dev.flags & VIR_NODE_DEV_CAP_FLAG_CSS_MDEV) + virNodeDeviceCapMdevTypesFormat(buf, + data->ccw_dev.mdev_types, + data->ccw_dev.nmdev_types); } @@ -832,6 +836,33 @@ virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt, } +static int +virNodeDevCSSCapabilityParseXML(xmlXPathContextPtr ctxt, + xmlNodePtr node, + virNodeDevCapCCWPtr ccw_dev) +{ + g_autofree char *type = virXMLPropString(node, "type"); + VIR_XPATH_NODE_AUTORESTORE(ctxt) + + ctxt->node = node; + + if (!type) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing capability type")); + return -1; + } + + if (STREQ(type, "mdev_types")) { + if (virNodeDevCapMdevTypesParseXML(ctxt, + &ccw_dev->mdev_types, + &ccw_dev->nmdev_types) < 0) + return -1; + ccw_dev->flags |= VIR_NODE_DEV_CAP_FLAG_CSS_MDEV; + } + + return 0; +} + + static int virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -839,6 +870,9 @@ virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, virNodeDevCapCCWPtr ccw_dev) { VIR_XPATH_NODE_AUTORESTORE(ctxt) + g_autofree xmlNodePtr *nodes = NULL; + int n = 0; + size_t i = 0; g_autofree char *cssid = NULL; g_autofree char *ssid = NULL; g_autofree char *devno = NULL; @@ -884,6 +918,14 @@ virNodeDevCapCCWParseXML(xmlXPathContextPtr ctxt, return -1; } + if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + if (virNodeDevCSSCapabilityParseXML(ctxt, nodes[i], ccw_dev) < 0) + return -1; + } + return 0; } @@ -2241,12 +2283,16 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) virMediatedDeviceAttrFree(data->mdev.attributes[i]); VIR_FREE(data->mdev.attributes); break; + case VIR_NODE_DEV_CAP_CSS_DEV: + for (i = 0; i < data->ccw_dev.nmdev_types; i++) + virMediatedDeviceTypeFree(data->ccw_dev.mdev_types[i]); + VIR_FREE(data->ccw_dev.mdev_types); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: 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_CSS_DEV: case VIR_NODE_DEV_CAP_LAST: /* This case is here to shutup the compiler */ break; @@ -2284,6 +2330,11 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def) &cap->data.pci_dev) < 0) return -1; break; + case VIR_NODE_DEV_CAP_CSS_DEV: + if (virNodeDeviceGetCSSDynamicCaps(def->sysfs_path, + &cap->data.ccw_dev) < 0) + return -1; + break; /* all types that (supposedly) don't require any updates * relative to what's in the cache. @@ -2300,7 +2351,6 @@ 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; } @@ -2374,6 +2424,15 @@ virNodeDeviceCapsListExport(virNodeDeviceDefPtr def, ncaps++; } } + + if (caps->data.type == VIR_NODE_DEV_CAP_CSS_DEV) { + flags = caps->data.ccw_dev.flags; + + if (flags & VIR_NODE_DEV_CAP_FLAG_CSS_MDEV) { + MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_MDEV_TYPES); + ncaps++; + } + } } #undef MAYBE_ADD_CAP @@ -2639,6 +2698,28 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath, return 0; } + +/* virNodeDeviceGetCSSDynamicCaps() get info that is stored in sysfs + * about devices related to this device, i.e. things that can change + * without this device itself changing. These must be refreshed + * anytime full XML of the device is requested, because they can + * change with no corresponding notification from the kernel/udev. + */ +int +virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath, + virNodeDevCapCCWPtr ccw_dev) +{ + ccw_dev->flags &= ~VIR_NODE_DEV_CAP_FLAG_CSS_MDEV; + if (virNodeDeviceGetMdevTypesCaps(sysfsPath, + &ccw_dev->mdev_types, + &ccw_dev->nmdev_types) < 0) + return -1; + if (ccw_dev->nmdev_types > 0) + ccw_dev->flags |= VIR_NODE_DEV_CAP_FLAG_CSS_MDEV; + + return 0; +} + #else int @@ -2661,4 +2742,11 @@ int virNodeDeviceGetSCSITargetCaps(const char *sysfsPath G_GNUC_UNUSED, return -1; } +int +virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath G_GNUC_UNUSED, + virNodeDevCapCCWPtr ccw_dev G_GNUC_UNUSED) +{ + return -1; +} + #endif /* __linux__ */ diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 5484bc340f..c72b943ba3 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -101,6 +101,10 @@ typedef enum { VIR_NODE_DEV_CAP_FLAG_PCI_MDEV = (1 << 3), } virNodeDevPCICapFlags; +typedef enum { + VIR_NODE_DEV_CAP_FLAG_CSS_MDEV = (1 << 0), +} virNodeDevCCWCapFlags; + typedef enum { /* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */ VIR_NODE_DEV_DRM_PRIMARY, @@ -273,6 +277,9 @@ struct _virNodeDevCapCCW { unsigned int cssid; unsigned int ssid; unsigned int devno; + unsigned int flags; /* enum virNodeDevCCWCapFlags */ + virMediatedDeviceTypePtr *mdev_types; + size_t nmdev_types; }; typedef struct _virNodeDevCapData virNodeDevCapData; @@ -382,6 +389,10 @@ int virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath, virNodeDevCapPCIDevPtr pci_dev); +int +virNodeDeviceGetCSSDynamicCaps(const char *sysfsPath, + virNodeDevCapCCWPtr ccw_dev); + int virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def); diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index f240abf315..e5a1af8b3b 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -696,6 +696,12 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, return true; break; + case VIR_NODE_DEV_CAP_CSS_DEV: + if (type == VIR_NODE_DEV_CAP_MDEV_TYPES && + (cap->data.ccw_dev.flags & VIR_NODE_DEV_CAP_FLAG_CSS_MDEV)) + return true; + break; + case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: @@ -710,7 +716,6 @@ 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/libvirt_private.syms b/src/libvirt_private.syms index 9029ea4fa2..d9c8df81a7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -823,6 +823,7 @@ virNodeDeviceDefFree; virNodeDeviceDefParseFile; virNodeDeviceDefParseNode; virNodeDeviceDefParseString; +virNodeDeviceGetCSSDynamicCaps; virNodeDeviceGetPCIDynamicCaps; virNodeDeviceGetSCSIHostCaps; virNodeDeviceGetSCSITargetCaps; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 29a7eaa07c..062c0fc52e 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1139,6 +1139,9 @@ udevProcessCSS(struct udev_device *device, if (udevGenerateDeviceName(device, def, NULL) != 0) return -1; + if (virNodeDeviceGetCSSDynamicCaps(def->sysfs_path, &def->caps->data.ccw_dev) < 0) + return -1; + return 0; } diff --git a/tests/nodedevschemadata/css_0_0_fffe_mdev_types.xml b/tests/nodedevschemadata/css_0_0_fffe_mdev_types.xml new file mode 100644 index 0000000000..5058b6434e --- /dev/null +++ b/tests/nodedevschemadata/css_0_0_fffe_mdev_types.xml @@ -0,0 +1,17 @@ +<device> + <name>css_0_0_fffe</name> + <path>/sys/devices/css0/0.0.fffe</path> + <parent>computer</parent> + <capability type='css'> + <cssid>0x0</cssid> + <ssid>0x0</ssid> + <devno>0xfffe</devno> + <capability type='mdev_types'> + <type id='vfio_ccw-io'> + <name>I/O subchannel (Non-QDIO)</name> + <deviceAPI>vfio-ccw</deviceAPI> + <availableInstances>1</availableInstances> + </type> + </capability> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index 3cb23b1df4..a009ecb343 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -124,6 +124,7 @@ mymain(void) DO_TEST("mdev_3627463d_b7f0_4fea_b468_f1da537d301b"); DO_TEST("ccw_0_0_ffff"); DO_TEST("css_0_0_ffff"); + DO_TEST("css_0_0_fffe_mdev_types"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.25.1

On Fri, Oct 23, 2020 at 07:31:42PM +0200, Boris Fiuczynski wrote:
Since channel subsystem devices can have the capability to create an mdev device they should expose this by listing the mdev_types capability.
Many patches of this series is concerned with refactoring existing PCI code.
Hi Boris, I will start looking at the patches next week. Regards, Erik
participants (3)
-
Boris Fiuczynski
-
Erik Skultety
-
Ján Tomko