Add the new introduced sysfs attribute dev_busid which provides the address
of the device in the subchannel independent from the bound device driver.
It is added if available in the sysfs as optional channel_dev_addr element into
the css device capabilty providing the ccw deivce address attributes cssid,
ssid and devno.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
src/conf/node_device_conf.c | 26 ++++++++++++++++++++++++++
src/conf/node_device_conf.h | 2 ++
src/conf/schemas/nodedev.rng | 5 +++++
src/node_device/node_device_udev.c | 8 ++++++++
4 files changed, 41 insertions(+)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 5aafcbb838..d5ee8da3ee 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -645,6 +645,17 @@ virNodeDeviceCapCSSDefFormat(virBuffer *buf,
virNodeDeviceCapCCWDefFormat(buf, data);
+ if (ccw_dev.channel_dev_addr) {
+ virCCWDeviceAddress *ccw = ccw_dev.channel_dev_addr;
+ virBufferAddLit(buf, "<channel_dev_addr>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<cssid>0x%x</cssid>\n", ccw->cssid);
+ virBufferAsprintf(buf, "<ssid>0x%x</ssid>\n", ccw->ssid);
+ virBufferAsprintf(buf, "<devno>0x%04x</devno>\n", ccw->devno);
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</channel_dev_addr>\n");
+ }
+
if (ccw_dev.flags & VIR_NODE_DEV_CAP_FLAG_CSS_MDEV)
virNodeDeviceCapMdevTypesFormat(buf,
ccw_dev.mdev_types,
@@ -1257,6 +1268,8 @@ virNodeDevCapCSSParseXML(xmlXPathContextPtr ctxt,
g_autofree xmlNodePtr *nodes = NULL;
int n = 0;
size_t i = 0;
+ xmlNodePtr channel_ddno = NULL;
+ g_autofree virCCWDeviceAddress *channel_dev = NULL;
ctxt->node = node;
@@ -1271,6 +1284,19 @@ virNodeDevCapCSSParseXML(xmlXPathContextPtr ctxt,
return -1;
}
+ // channel_dev_addr is optional
+ if ((channel_ddno = virXPathNode("./channel_dev_addr[1]", ctxt))) {
+ channel_dev = g_new0(virCCWDeviceAddress, 1);
+
+ if (virNodeDevCCWDeviceAddressParseXML(ctxt,
+ channel_ddno,
+ def->name,
+ channel_dev) < 0)
+ return -1;
+
+ ccw_dev->channel_dev_addr = g_steal_pointer(&channel_dev);
+ }
+
return 0;
}
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index e4d1f67d53..d1751ed874 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -24,6 +24,7 @@
#include "internal.h"
#include "virbitmap.h"
+#include "virccw.h"
#include "virpcivpd.h"
#include "virscsihost.h"
#include "virpci.h"
@@ -279,6 +280,7 @@ struct _virNodeDevCapCCW {
unsigned int flags; /* enum virNodeDevCCWCapFlags */
virMediatedDeviceType **mdev_types;
size_t nmdev_types;
+ virCCWDeviceAddress *channel_dev_addr;
};
typedef struct _virNodeDevCapVDPA virNodeDevCapVDPA;
diff --git a/src/conf/schemas/nodedev.rng b/src/conf/schemas/nodedev.rng
index a9f83e048c..e40243e257 100644
--- a/src/conf/schemas/nodedev.rng
+++ b/src/conf/schemas/nodedev.rng
@@ -682,6 +682,11 @@
<value>css</value>
</attribute>
<ref name="capccwaddress"/>
+ <optional>
+ <element name="channel_dev_addr">
+ <ref name="capccwaddress"/>
+ </element>
+ </optional>
<optional>
<ref name="mdev_types"/>
</optional>
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 4bb841c66b..16314627ca 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1124,6 +1124,8 @@ static int
udevProcessCSS(struct udev_device *device,
virNodeDeviceDef *def)
{
+ char *dev_busid;
+
/* only process IO subchannel and vfio-ccw devices to keep the list sane */
if (!def->driver ||
(STRNEQ(def->driver, "io_subchannel") &&
@@ -1135,6 +1137,12 @@ udevProcessCSS(struct udev_device *device,
udevGenerateDeviceName(device, def, NULL);
+ /* process optional channel devices information */
+ udevGetStringSysfsAttr(device, "dev_busid", &dev_busid);
+
+ if (dev_busid != NULL)
+ def->caps->data.ccw_dev.channel_dev_addr = virCCWDeviceAddressFromString(dev_busid);
+
if (virNodeDeviceGetCSSDynamicCaps(def->sysfs_path, &def->caps->data.ccw_dev) < 0)
return -1;
--
2.33.1On 5/13/22 12:31, Boris Fiuczynski wrote:
> Add the new introduced sysfs attribute dev_busid which provides the address
> of the device in the subchannel independent from the bound device driver.
> It is added if available in the sysfs as optional channel_dev_addr element into
> the css device capabilty providing the ccw deivce address attributes cssid,
> ssid and devno.
>
> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> ---
> src/conf/node_device_conf.c | 26 ++++++++++++++++++++++++++
> src/conf/node_device_conf.h | 2 ++
> src/conf/schemas/nodedev.rng | 5 +++++
> src/node_device/node_device_udev.c | 8 ++++++++
> 4 files changed, 41 insertions(+)
>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 5aafcbb838..d5ee8da3ee 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -645,6 +645,17 @@ virNodeDeviceCapCSSDefFormat(virBuffer *buf,
>
> virNodeDeviceCapCCWDefFormat(buf, data);
>
> + if (ccw_dev.channel_dev_addr) {
> + virCCWDeviceAddress *ccw = ccw_dev.channel_dev_addr;
> + virBufferAddLit(buf, "<channel_dev_addr>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferAsprintf(buf, "<cssid>0x%x</cssid>\n", ccw->cssid);
> + virBufferAsprintf(buf, "<ssid>0x%x</ssid>\n", ccw->ssid);
> + virBufferAsprintf(buf, "<devno>0x%04x</devno>\n", ccw->devno);
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</channel_dev_addr>\n");
> + }
> +
> if (ccw_dev.flags & VIR_NODE_DEV_CAP_FLAG_CSS_MDEV)
> virNodeDeviceCapMdevTypesFormat(buf,
> ccw_dev.mdev_types,
> @@ -1257,6 +1268,8 @@ virNodeDevCapCSSParseXML(xmlXPathContextPtr ctxt,
> g_autofree xmlNodePtr *nodes = NULL;
> int n = 0;
> size_t i = 0;
> + xmlNodePtr channel_ddno = NULL;
> + g_autofree virCCWDeviceAddress *channel_dev = NULL;
>
> ctxt->node = node;
>
> @@ -1271,6 +1284,19 @@ virNodeDevCapCSSParseXML(xmlXPathContextPtr ctxt,
> return -1;
> }
>
> + // channel_dev_addr is optional
c89 style of comments please :-)
> + if ((channel_ddno = virXPathNode("./channel_dev_addr[1]", ctxt))) {
> + channel_dev = g_new0(virCCWDeviceAddress, 1);
This variable is not used anywhere else but in this block. It may be
worth declaring it within.
> +
> + if (virNodeDevCCWDeviceAddressParseXML(ctxt,
> + channel_ddno,
> + def->name,
> + channel_dev) < 0)
> + return -1;
> +
> + ccw_dev->channel_dev_addr = g_steal_pointer(&channel_dev);
So this steals allocation from passed variable. But corresponding free()
call in virNodeDevCapsDefFree() is missing.
> + }
> +
> return 0;
> }
>
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index e4d1f67d53..d1751ed874 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -24,6 +24,7 @@
>
> #include "internal.h"
> #include "virbitmap.h"
> +#include "virccw.h"
> #include "virpcivpd.h"
> #include "virscsihost.h"
> #include "virpci.h"
> @@ -279,6 +280,7 @@ struct _virNodeDevCapCCW {
> unsigned int flags; /* enum virNodeDevCCWCapFlags */
> virMediatedDeviceType **mdev_types;
> size_t nmdev_types;
> + virCCWDeviceAddress *channel_dev_addr;
> };
>
> typedef struct _virNodeDevCapVDPA virNodeDevCapVDPA;
> diff --git a/src/conf/schemas/nodedev.rng b/src/conf/schemas/nodedev.rng
> index a9f83e048c..e40243e257 100644
> --- a/src/conf/schemas/nodedev.rng
> +++ b/src/conf/schemas/nodedev.rng
> @@ -682,6 +682,11 @@
> <value>css</value>
> </attribute>
> <ref name="capccwaddress"/>
> + <optional>
> + <element name="channel_dev_addr">
> + <ref name="capccwaddress"/>
> + </element>
> + </optional>
> <optional>
> <ref name="mdev_types"/>
> </optional>
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 4bb841c66b..16314627ca 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1124,6 +1124,8 @@ static int
> udevProcessCSS(struct udev_device *device,
> virNodeDeviceDef *def)
> {
> + char *dev_busid;
> +
> /* only process IO subchannel and vfio-ccw devices to keep the list sane */
> if (!def->driver ||
> (STRNEQ(def->driver, "io_subchannel") &&
> @@ -1135,6 +1137,12 @@ udevProcessCSS(struct udev_device *device,
>
> udevGenerateDeviceName(device, def, NULL);
>
> + /* process optional channel devices information */
> + udevGetStringSysfsAttr(device, "dev_busid", &dev_busid);
This allocates dev_busid, which is never freed. it's sufficient to use
g_autofree for the variable.
Looking into the kernel sources, we may either get proper address here
or "none":
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/s390/cio/css.c#n433
Is it worth to bother with "none" string here?
> +
> + if (dev_busid != NULL)
> + def->caps->data.ccw_dev.channel_dev_addr = virCCWDeviceAddressFromString(dev_busid);
> +
> if (virNodeDeviceGetCSSDynamicCaps(def->sysfs_path, &def->caps->data.ccw_dev) < 0)
> return -1;
>
Michal
On 5/23/22 4:40 PM, Michal Prívozník wrote:
> On 5/13/22 12:31, Boris Fiuczynski wrote:
>> Add the new introduced sysfs attribute dev_busid which provides the address
>> of the device in the subchannel independent from the bound device driver.
>> It is added if available in the sysfs as optional channel_dev_addr element into
>> the css device capabilty providing the ccw deivce address attributes cssid,
>> ssid and devno.
>>
>> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
>> ---
>> src/conf/node_device_conf.c | 26 ++++++++++++++++++++++++++
>> src/conf/node_device_conf.h | 2 ++
>> src/conf/schemas/nodedev.rng | 5 +++++
>> src/node_device/node_device_udev.c | 8 ++++++++
>> 4 files changed, 41 insertions(+)
>>
>> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>> index 5aafcbb838..d5ee8da3ee 100644
>> --- a/src/conf/node_device_conf.c
>> +++ b/src/conf/node_device_conf.c
>> @@ -645,6 +645,17 @@ virNodeDeviceCapCSSDefFormat(virBuffer *buf,
>>
>> virNodeDeviceCapCCWDefFormat(buf, data);
>>
>> + if (ccw_dev.channel_dev_addr) {
>> + virCCWDeviceAddress *ccw = ccw_dev.channel_dev_addr;
>> + virBufferAddLit(buf, "<channel_dev_addr>\n");
>> + virBufferAdjustIndent(buf, 2);
>> + virBufferAsprintf(buf, "<cssid>0x%x</cssid>\n", ccw->cssid);
>> + virBufferAsprintf(buf, "<ssid>0x%x</ssid>\n", ccw->ssid);
>> + virBufferAsprintf(buf, "<devno>0x%04x</devno>\n", ccw->devno);
>> + virBufferAdjustIndent(buf, -2);
>> + virBufferAddLit(buf, "</channel_dev_addr>\n");
>> + }
>> +
>> if (ccw_dev.flags & VIR_NODE_DEV_CAP_FLAG_CSS_MDEV)
>> virNodeDeviceCapMdevTypesFormat(buf,
>> ccw_dev.mdev_types,
>> @@ -1257,6 +1268,8 @@ virNodeDevCapCSSParseXML(xmlXPathContextPtr ctxt,
>> g_autofree xmlNodePtr *nodes = NULL;
>> int n = 0;
>> size_t i = 0;
>> + xmlNodePtr channel_ddno = NULL;
>> + g_autofree virCCWDeviceAddress *channel_dev = NULL;
>>
>> ctxt->node = node;
>>
>> @@ -1271,6 +1284,19 @@ virNodeDevCapCSSParseXML(xmlXPathContextPtr ctxt,
>> return -1;
>> }
>>
>> + // channel_dev_addr is optional
>
> c89 style of comments please :-)
>
>> + if ((channel_ddno = virXPathNode("./channel_dev_addr[1]", ctxt))) {
>> + channel_dev = g_new0(virCCWDeviceAddress, 1);
>
> This variable is not used anywhere else but in this block. It may be
> worth declaring it within.
Yes, good idea.
>
>> +
>> + if (virNodeDevCCWDeviceAddressParseXML(ctxt,
>> + channel_ddno,
>> + def->name,
>> + channel_dev) < 0)
>> + return -1;
>> +
>> + ccw_dev->channel_dev_addr = g_steal_pointer(&channel_dev);
>
> So this steals allocation from passed variable. But corresponding free()
> call in virNodeDevCapsDefFree() is missing.
Ups, correct. Thanks for fixing it.
>
>> + }
>> +
>> return 0;
>> }
>>
>> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
>> index e4d1f67d53..d1751ed874 100644
>> --- a/src/conf/node_device_conf.h
>> +++ b/src/conf/node_device_conf.h
>> @@ -24,6 +24,7 @@
>>
>> #include "internal.h"
>> #include "virbitmap.h"
>> +#include "virccw.h"
>> #include "virpcivpd.h"
>> #include "virscsihost.h"
>> #include "virpci.h"
>> @@ -279,6 +280,7 @@ struct _virNodeDevCapCCW {
>> unsigned int flags; /* enum virNodeDevCCWCapFlags */
>> virMediatedDeviceType **mdev_types;
>> size_t nmdev_types;
>> + virCCWDeviceAddress *channel_dev_addr;
>> };
>>
>> typedef struct _virNodeDevCapVDPA virNodeDevCapVDPA;
>> diff --git a/src/conf/schemas/nodedev.rng b/src/conf/schemas/nodedev.rng
>> index a9f83e048c..e40243e257 100644
>> --- a/src/conf/schemas/nodedev.rng
>> +++ b/src/conf/schemas/nodedev.rng
>> @@ -682,6 +682,11 @@
>> <value>css</value>
>> </attribute>
>> <ref name="capccwaddress"/>
>> + <optional>
>> + <element name="channel_dev_addr">
>> + <ref name="capccwaddress"/>
>> + </element>
>> + </optional>
>> <optional>
>> <ref name="mdev_types"/>
>> </optional>
>> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
>> index 4bb841c66b..16314627ca 100644
>> --- a/src/node_device/node_device_udev.c
>> +++ b/src/node_device/node_device_udev.c
>> @@ -1124,6 +1124,8 @@ static int
>> udevProcessCSS(struct udev_device *device,
>> virNodeDeviceDef *def)
>> {
>> + char *dev_busid;
>> +
>> /* only process IO subchannel and vfio-ccw devices to keep the list sane */
>> if (!def->driver ||
>> (STRNEQ(def->driver, "io_subchannel") &&
>> @@ -1135,6 +1137,12 @@ udevProcessCSS(struct udev_device *device,
>>
>> udevGenerateDeviceName(device, def, NULL);
>>
>> + /* process optional channel devices information */
>> + udevGetStringSysfsAttr(device, "dev_busid", &dev_busid);
>
> This allocates dev_busid, which is never freed. it's sufficient to use
> g_autofree for the variable.
>
> Looking into the kernel sources, we may either get proper address here
> or "none":
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/s390/cio/css.c#n433
>
> Is it worth to bother with "none" string here?
OK, it's fair as it currently would result in a libvirt internal error.
I will send another patch with an additional check for "none" shortly.
>
>> +
>> + if (dev_busid != NULL)
>> + def->caps->data.ccw_dev.channel_dev_addr = virCCWDeviceAddressFromString(dev_busid);
>> +
>> if (virNodeDeviceGetCSSDynamicCaps(def->sysfs_path, &def->caps->data.ccw_dev) < 0)
>> return -1;
>>
>
> Michal
>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
© 2016 - 2026 Red Hat, Inc.