Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-14 Thread Boris Fiuczynski 

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

Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-13 Thread Erik Skultety 
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

Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-11 Thread Boris Fiuczynski 

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

Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-09 Thread Erik Skultety 
On Wed, Sep 09, 2020 at 03:01:50PM +0200, Cornelia Huck wrote:
> On Wed, 9 Sep 2020 14:46:00 +0200
> Erik Skultety  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.znetwork/znetwork_59.htm
> > [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_SA22-7832-03.pdf
> > > >
> > > > 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

Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-09 Thread Cornelia Huck 
On Wed, 9 Sep 2020 14:46:00 +0200
Erik Skultety  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.znetwork/znetwork_59.htm
> [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_SA22-7832-03.pdf
> > >
> > > 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...

Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-09 Thread Erik Skultety 
...
> > > > 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 @@
> > > >  
> > > >  
> > > >  
> > > > +
> > > >
> > > >  
> > > >
> > > > @@ -659,6 +660,21 @@
> > > >  
> > > >
> > > >
> > > > +  
> > > > +
> > > > +  css
> > > > +
> > > > +
> > >
> > > 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]
>
> >
> > >
> > > > +  
> > > > +
> > > > +
> > > > +  
> > > > +
> > > > +
> > > > +  
> > > > +
> > > > +  
> > > > +
> > >
> > > 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.znetwork/znetwork_59.htm
[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_SA22-7832-03.pdf
> >
> > 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_1-invalid.xml 
> > > > b/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> > > > index d840555c..f3cf0c1c 100644
> > > > --- a/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> > > > +++ b/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> > > > @@ -1,7 +1,7 @@
> > > >  
> > > >ccw_0_0_1
> > > > -  /sys/devices/css0/0.0./0.0.1
> > > > -  computer
> > > > +  /sys/devices/css0/0.0.0070/0.0.1
> > > > +  css_0_0_0070
> > >
> > > Looking at

Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-09 Thread Cornelia Huck 
On Wed, 9 Sep 2020 08:18:08 +0200
Bjoern Walk  wrote:

> Erik Skultety  [2020-09-08, 05:46PM +0200]:
> > On Mon, Aug 24, 2020 at 01:59:12PM +0200, Bjoern Walk wrote:  
> > > From: Boris Fiuczynski 
> > >
> > > 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 
> > > Signed-off-by: Boris Fiuczynski 
> > > ---
> > >  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_1-invalid.xml |  4 ++--
> > >  tests/nodedevschemadata/ccw_0_0_.xml  |  4 ++--
> > >  tests/nodedevschemadata/css_0_0_.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_.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 @@
> > >  
> > >  
> > >  
> > > +
> > >
> > >  
> > >
> > > @@ -659,6 +660,21 @@
> > >  
> > >
> > >
> > > +  
> > > +
> > > +  css
> > > +
> > > +  
> > 
> > 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]

> 
> >   
> > > +  
> > > +
> > > +
> > > +  
> > > +
> > > +
> > > +  
> > > +
> > > +  
> > > +  
> > 
> > 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_SA22-7832-03.pdf
> 
> 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_D

Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-08 Thread Bjoern Walk 
Erik Skultety  [2020-09-08, 05:46PM +0200]:
> On Mon, Aug 24, 2020 at 01:59:12PM +0200, Bjoern Walk wrote:
> > From: Boris Fiuczynski 
> >
> > 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 
> > Signed-off-by: Boris Fiuczynski 
> > ---
> >  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_1-invalid.xml |  4 ++--
> >  tests/nodedevschemadata/ccw_0_0_.xml  |  4 ++--
> >  tests/nodedevschemadata/css_0_0_.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_.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 @@
> >  
> >  
> >  
> > +
> >
> >  
> >
> > @@ -659,6 +660,21 @@
> >  
> >
> >
> > +  
> > +
> > +  css
> > +
> > +
> 
> 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.

> 
> > +  
> > +
> > +
> > +  
> > +
> > +
> > +  
> > +
> > +  
> > +
> 
> 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_SA22-7832-03.pdf

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_1-invalid.xml 
> > b/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> > index d840555c..f3cf0c1c 100644
> > --- a/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> > +++ b/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> > @@ -1,7 +1,7 @@
> >  
> >ccw_0_0_1
> > -  /sys/devices/css0/0.0./0.0.1
> > -  computer
> > +  /sys/devices/css0/0.0.0070/0.0.1
> > +  css_0_0_0070
> 
> Looking at the architecture diagram at [1], I'm wondering why haven't we add

Re: [PATCH 2/5] node_device: detect CSS devices

2020-09-08 Thread Erik Skultety 
On Mon, Aug 24, 2020 at 01:59:12PM +0200, Bjoern Walk wrote:
> From: Boris Fiuczynski 
>
> 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 
> Signed-off-by: Boris Fiuczynski 
> ---
>  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_1-invalid.xml |  4 ++--
>  tests/nodedevschemadata/ccw_0_0_.xml  |  4 ++--
>  tests/nodedevschemadata/css_0_0_.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_.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 @@
>  
>  
>  
> +
>
>  
>
> @@ -659,6 +660,21 @@
>  
>
>
> +  
> +
> +  css
> +
> +

Is ^this one just a different name for CHPID or those are completely unrelated?

> +  
> +
> +
> +  
> +
> +
> +  
> +
> +  
> +

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_1-invalid.xml 
> b/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> index d840555c..f3cf0c1c 100644
> --- a/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> +++ b/tests/nodedevschemadata/ccw_0_0_1-invalid.xml
> @@ -1,7 +1,7 @@
>  
>ccw_0_0_1
> -  /sys/devices/css0/0.0./0.0.1
> -  computer
> +  /sys/devices/css0/0.0.0070/0.0.1
> +  css_0_0_0070

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

[PATCH 2/5] node_device: detect CSS devices

2020-08-24 Thread Bjoern Walk 
From: Boris Fiuczynski 

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 
Signed-off-by: Boris Fiuczynski 
---
 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_1-invalid.xml |  4 ++--
 tests/nodedevschemadata/ccw_0_0_.xml  |  4 ++--
 tests/nodedevschemadata/css_0_0_.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_.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 @@
 
 
 
+
   
 
   
@@ -659,6 +660,21 @@
 
   
 
+  
+
+  css
+
+
+  
+
+
+  
+
+
+  
+
+  
+
   
 
   
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, "0x%x\n",
   data->ccw_dev.cssid);
 virBufferAsprintf(&buf, "0x%x\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