[PATCH libvirt v4 00/12] Support AP card, AP queues and AP matrix

Shalini Chellathurai Saroja posted 12 patches 3 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201203175944.3867894-1-shalini@linux.ibm.com
NEWS.rst                                      |   6 +
docs/formatnode.html.in                       |  48 +++-
docs/manpages/virsh.rst                       |   2 +-
docs/schemas/nodedev.rng                      |  46 ++++
include/libvirt/libvirt-nodedev.h             |   3 +
src/conf/node_device_conf.c                   | 213 ++++++++++++++++++
src/conf/node_device_conf.h                   |  41 +++-
src/conf/virnodedeviceobj.c                   |  13 +-
src/libvirt-nodedev.c                         |   3 +
src/libvirt_private.syms                      |   1 +
src/node_device/node_device_driver.c          |  33 ++-
src/node_device/node_device_udev.c            |  80 +++++++
tests/nodedevschemadata/ap_07_0038.xml        |   9 +
tests/nodedevschemadata/ap_card07.xml         |   8 +
tests/nodedevschemadata/ap_matrix.xml         |   7 +
.../ap_matrix_mdev_types.xml                  |  14 ++
...v_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml |   9 +
tests/nodedevxml2xmltest.c                    |   5 +
tools/virsh-nodedev.c                         |   9 +
19 files changed, 544 insertions(+), 6 deletions(-)
create mode 100644 tests/nodedevschemadata/ap_07_0038.xml
create mode 100644 tests/nodedevschemadata/ap_card07.xml
create mode 100644 tests/nodedevschemadata/ap_matrix.xml
create mode 100644 tests/nodedevschemadata/ap_matrix_mdev_types.xml
create mode 100644 tests/nodedevschemadata/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml
[PATCH libvirt v4 00/12] Support AP card, AP queues and AP matrix
Posted by Shalini Chellathurai Saroja 3 years, 5 months ago
Add support for AP card devices, AP queues and AP matrix devices in
libvirt node device driver.
---
v4:
 - Added virNodeDevAPAdapterParseXML function to extract the adapter
   parsing logic.
 - Modified according to review comments.
 - New patch to mention support for AP devices in NEWS.rst. 

v3:
 - Modify schema definition of ap-adapter to support hex values alone.
 - Modify schema definition of ap-domain to support hex values alone.
 - Verify domain value of AP queue device to be within the supported
   range during parsing.
 - Re-organized patches 6 and 10 according to review comment.
 - Minor changes according to review comments.

v2:
 - The tests included in the patches to detect the node devices (AP
   card, AP queues and AP matrix) are moved to separate patches.
   These patches are not divided further due to the following reasons:
    - The previous patches that add support for node devices in
      libvirt are not divided further into smaller patches (eg:
      53aec799fa31711ffaeacc7ec17ec6d3c2e3cadf).
    - It would be easier to identify the patches relevant to support
      AP node devices.
    - The number of lines in each of these patches are around 125,
      which is not too much.
 - Modified according to review comments.
 - New patch to detect mdev_types capabilty of AP matrix device is
   added to this patch series.

Boris Fiuczynski (1):
  node_device: detecting mdev_types capability on ap_matrix device

Farhan Ali (1):
  virsh: nodedev: Filter by AP card and AP queue capabilities

Shalini Chellathurai Saroja (10):
  nodedev: detect AP card device
  tests: AP card node device
  nodedev: detect AP queues
  tests: AP queue node device
  nodedev: detect AP matrix device
  tests: AP matrix node device
  virsh: nodedev: filter by AP Matrix capability
  node_device: refactor address retrieval of node device
  node_device: mdev matrix support
  NEWS: mention node device driver support for AP devices

 NEWS.rst                                      |   6 +
 docs/formatnode.html.in                       |  48 +++-
 docs/manpages/virsh.rst                       |   2 +-
 docs/schemas/nodedev.rng                      |  46 ++++
 include/libvirt/libvirt-nodedev.h             |   3 +
 src/conf/node_device_conf.c                   | 213 ++++++++++++++++++
 src/conf/node_device_conf.h                   |  41 +++-
 src/conf/virnodedeviceobj.c                   |  13 +-
 src/libvirt-nodedev.c                         |   3 +
 src/libvirt_private.syms                      |   1 +
 src/node_device/node_device_driver.c          |  33 ++-
 src/node_device/node_device_udev.c            |  80 +++++++
 tests/nodedevschemadata/ap_07_0038.xml        |   9 +
 tests/nodedevschemadata/ap_card07.xml         |   8 +
 tests/nodedevschemadata/ap_matrix.xml         |   7 +
 .../ap_matrix_mdev_types.xml                  |  14 ++
 ...v_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml |   9 +
 tests/nodedevxml2xmltest.c                    |   5 +
 tools/virsh-nodedev.c                         |   9 +
 19 files changed, 544 insertions(+), 6 deletions(-)
 create mode 100644 tests/nodedevschemadata/ap_07_0038.xml
 create mode 100644 tests/nodedevschemadata/ap_card07.xml
 create mode 100644 tests/nodedevschemadata/ap_matrix.xml
 create mode 100644 tests/nodedevschemadata/ap_matrix_mdev_types.xml
 create mode 100644 tests/nodedevschemadata/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml

-- 
2.26.2

Re: [PATCH libvirt v4 00/12] Support AP card, AP queues and AP matrix
Posted by Erik Skultety 3 years, 4 months ago
On Thu, Dec 03, 2020 at 06:59:32PM +0100, Shalini Chellathurai Saroja wrote:
> Add support for AP card devices, AP queues and AP matrix devices in
> libvirt node device driver.
> ---
> v4:
>  - Added virNodeDevAPAdapterParseXML function to extract the adapter
>    parsing logic.
>  - Modified according to review comments.
>  - New patch to mention support for AP devices in NEWS.rst. 

Reviewed-by: Erik Skultety <eskultet@redhat.com>

I had 2 nitpicks which I can fix before merging, but I'd like to give other
people a couple more days to express their "final" opinions and if there are no
more comments, then sometime next week I'll merge this.
There's one more little thing...Boris linked the s390 AP facility kernel
documentation which really helped me during the review, so I think we should
link it somewhere too - usually we're not so keen on doing that because 3rd
party documentation URLs tend to die or migrate, but in this case it linked
directly to the github repo (I think even the generated HTML on kernel.org
would be just fine), but I don't know what the right place for this actually is
as it describes the whole facility which we modelled in 3 capabilities. We
could put it into the NEWS file, but then again, not sure how often anyone
developing libvirt reads the NEWS file.

Regards,
Erik

Re: [PATCH libvirt v4 00/12] Support AP card, AP queues and AP matrix
Posted by Boris Fiuczynski 3 years, 4 months ago
On 12/4/20 11:24 AM, Erik Skultety wrote:
> On Thu, Dec 03, 2020 at 06:59:32PM +0100, Shalini Chellathurai Saroja wrote:
>> Add support for AP card devices, AP queues and AP matrix devices in
>> libvirt node device driver.
>> ---
>> v4:
>>   - Added virNodeDevAPAdapterParseXML function to extract the adapter
>>     parsing logic.
>>   - Modified according to review comments.
>>   - New patch to mention support for AP devices in NEWS.rst.
> 
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
> 
> I had 2 nitpicks which I can fix before merging, but I'd like to give other
> people a couple more days to express their "final" opinions and if there are no
> more comments, then sometime next week I'll merge this.
> There's one more little thing...Boris linked the s390 AP facility kernel
> documentation which really helped me during the review, so I think we should
> link it somewhere too - usually we're not so keen on doing that because 3rd
> party documentation URLs tend to die or migrate, but in this case it linked
> directly to the github repo (I think even the generated HTML on kernel.org
> would be just fine), but I don't know what the right place for this actually is
> as it describes the whole facility which we modelled in 3 capabilities. We
> could put it into the NEWS file, but then again, not sure how often anyone
> developing libvirt reads the NEWS file.
> 
> Regards,
> Erik
> 

Erik,
thanks for your review.
How about we put for developers the links into the device originating 
commit messages:
Patch 1+3: 
https://github.com/torvalds/linux/blob/master/Documentation/s390/vfio-ap.rst#ap-architectural-overview
Patch 6: 
https://github.com/torvalds/linux/blob/master/Documentation/s390/vfio-ap.rst#the-design

Another alternative could be to add the links into 
docs/formatnode.html.in but that might be too much information for the 
normal libvirt users not interested in these details.

-- 
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 libvirt v4 00/12] Support AP card, AP queues and AP matrix
Posted by Cornelia Huck 3 years, 4 months ago
On Fri, 4 Dec 2020 16:31:56 +0100
Boris Fiuczynski <fiuczy@linux.ibm.com> wrote:

> On 12/4/20 11:24 AM, Erik Skultety wrote:
> > On Thu, Dec 03, 2020 at 06:59:32PM +0100, Shalini Chellathurai Saroja wrote:  
> >> Add support for AP card devices, AP queues and AP matrix devices in
> >> libvirt node device driver.
> >> ---
> >> v4:
> >>   - Added virNodeDevAPAdapterParseXML function to extract the adapter
> >>     parsing logic.
> >>   - Modified according to review comments.
> >>   - New patch to mention support for AP devices in NEWS.rst.  
> > 
> > Reviewed-by: Erik Skultety <eskultet@redhat.com>
> > 
> > I had 2 nitpicks which I can fix before merging, but I'd like to give other
> > people a couple more days to express their "final" opinions and if there are no
> > more comments, then sometime next week I'll merge this.
> > There's one more little thing...Boris linked the s390 AP facility kernel
> > documentation which really helped me during the review, so I think we should
> > link it somewhere too - usually we're not so keen on doing that because 3rd
> > party documentation URLs tend to die or migrate, but in this case it linked
> > directly to the github repo (I think even the generated HTML on kernel.org
> > would be just fine), but I don't know what the right place for this actually is
> > as it describes the whole facility which we modelled in 3 capabilities. We
> > could put it into the NEWS file, but then again, not sure how often anyone
> > developing libvirt reads the NEWS file.
> > 
> > Regards,
> > Erik
> >   
> 
> Erik,
> thanks for your review.
> How about we put for developers the links into the device originating 
> commit messages:

<comment from the side line>

> Patch 1+3: 
> https://github.com/torvalds/linux/blob/master/Documentation/s390/vfio-ap.rst#ap-architectural-overview

For the link, I would use

https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#ap-architectural-overview

> Patch 6: 
> https://github.com/torvalds/linux/blob/master/Documentation/s390/vfio-ap.rst#the-design

and

https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#the-design

</comment from the side line>

> 
> Another alternative could be to add the links into 
> docs/formatnode.html.in but that might be too much information for the 
> normal libvirt users not interested in these details.
> 

Re: [PATCH libvirt v4 00/12] Support AP card, AP queues and AP matrix
Posted by Erik Skultety 3 years, 4 months ago
On Fri, Dec 04, 2020 at 05:30:04PM +0100, Cornelia Huck wrote:
> On Fri, 4 Dec 2020 16:31:56 +0100
> Boris Fiuczynski <fiuczy@linux.ibm.com> wrote:
> 
> > On 12/4/20 11:24 AM, Erik Skultety wrote:
> > > On Thu, Dec 03, 2020 at 06:59:32PM +0100, Shalini Chellathurai Saroja wrote:  
> > >> Add support for AP card devices, AP queues and AP matrix devices in
> > >> libvirt node device driver.
> > >> ---
> > >> v4:
> > >>   - Added virNodeDevAPAdapterParseXML function to extract the adapter
> > >>     parsing logic.
> > >>   - Modified according to review comments.
> > >>   - New patch to mention support for AP devices in NEWS.rst.  
> > > 
> > > Reviewed-by: Erik Skultety <eskultet@redhat.com>
> > > 
> > > I had 2 nitpicks which I can fix before merging, but I'd like to give other
> > > people a couple more days to express their "final" opinions and if there are no
> > > more comments, then sometime next week I'll merge this.
> > > There's one more little thing...Boris linked the s390 AP facility kernel
> > > documentation which really helped me during the review, so I think we should
> > > link it somewhere too - usually we're not so keen on doing that because 3rd
> > > party documentation URLs tend to die or migrate, but in this case it linked
> > > directly to the github repo (I think even the generated HTML on kernel.org
> > > would be just fine), but I don't know what the right place for this actually is
> > > as it describes the whole facility which we modelled in 3 capabilities. We
> > > could put it into the NEWS file, but then again, not sure how often anyone
> > > developing libvirt reads the NEWS file.
> > > 
> > > Regards,
> > > Erik
> > >   
> > 
> > Erik,
> > thanks for your review.
> > How about we put for developers the links into the device originating 
> > commit messages:
> 
> <comment from the side line>
> 
> > Patch 1+3: 
> > https://github.com/torvalds/linux/blob/master/Documentation/s390/vfio-ap.rst#ap-architectural-overview
> 
> For the link, I would use
> 
> https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#ap-architectural-overview
> 
> > Patch 6: 
> > https://github.com/torvalds/linux/blob/master/Documentation/s390/vfio-ap.rst#the-design
> 
> and
> 
> https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#the-design
> 
> </comment from the side line>

Sounds good, I'll tweak the commit messages before merging.

Regards,
Erik

Re: [PATCH libvirt v4 00/12] Support AP card, AP queues and AP matrix
Posted by Erik Skultety 3 years, 4 months ago
On Fri, Dec 04, 2020 at 11:24:12AM +0100, Erik Skultety wrote:
> On Thu, Dec 03, 2020 at 06:59:32PM +0100, Shalini Chellathurai Saroja wrote:
> > Add support for AP card devices, AP queues and AP matrix devices in
> > libvirt node device driver.
> > ---
> > v4:
> >  - Added virNodeDevAPAdapterParseXML function to extract the adapter
> >    parsing logic.
> >  - Modified according to review comments.
> >  - New patch to mention support for AP devices in NEWS.rst. 
> 
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
> 
> I had 2 nitpicks which I can fix before merging, but I'd like to give other
> people a couple more days to express their "final" opinions and if there are no
> more comments, then sometime next week I'll merge this.
> There's one more little thing...Boris linked the s390 AP facility kernel
> documentation which really helped me during the review, so I think we should
> link it somewhere too - usually we're not so keen on doing that because 3rd
> party documentation URLs tend to die or migrate, but in this case it linked
> directly to the github repo (I think even the generated HTML on kernel.org
> would be just fine), but I don't know what the right place for this actually is
> as it describes the whole facility which we modelled in 3 capabilities. We
> could put it into the NEWS file, but then again, not sure how often anyone
> developing libvirt reads the NEWS file.
> 
> Regards,
> Erik
> 

Pushed now.

Regards,
Erik

Re: [PATCH libvirt v4 00/12] Support AP card, AP queues and AP matrix
Posted by Shalini Chellathurai Saroja 3 years, 4 months ago
On 12/9/20 2:03 PM, Erik Skultety wrote:
> On Fri, Dec 04, 2020 at 11:24:12AM +0100, Erik Skultety wrote:
>> On Thu, Dec 03, 2020 at 06:59:32PM +0100, Shalini Chellathurai Saroja wrote:
>>> Add support for AP card devices, AP queues and AP matrix devices in
>>> libvirt node device driver.
>>> ---
>>> v4:
>>>   - Added virNodeDevAPAdapterParseXML function to extract the adapter
>>>     parsing logic.
>>>   - Modified according to review comments.
>>>   - New patch to mention support for AP devices in NEWS.rst.
>> Reviewed-by: Erik Skultety <eskultet@redhat.com>
>>
>> I had 2 nitpicks which I can fix before merging, but I'd like to give other
>> people a couple more days to express their "final" opinions and if there are no
>> more comments, then sometime next week I'll merge this.
>> There's one more little thing...Boris linked the s390 AP facility kernel
>> documentation which really helped me during the review, so I think we should
>> link it somewhere too - usually we're not so keen on doing that because 3rd
>> party documentation URLs tend to die or migrate, but in this case it linked
>> directly to the github repo (I think even the generated HTML on kernel.org
>> would be just fine), but I don't know what the right place for this actually is
>> as it describes the whole facility which we modelled in 3 capabilities. We
>> could put it into the NEWS file, but then again, not sure how often anyone
>> developing libvirt reads the NEWS file.
>>
>> Regards,
>> Erik
>>
> Pushed now.

Hello Erik,

Thank you.

>
> Regards,
> Erik
>
-- 
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


Re: [PATCH libvirt v4 00/12] Support AP card, AP queues and AP matrix
Posted by Shalini Chellathurai Saroja 3 years, 4 months ago
On 12/4/20 11:24 AM, Erik Skultety wrote:
> On Thu, Dec 03, 2020 at 06:59:32PM +0100, Shalini Chellathurai Saroja wrote:
>> Add support for AP card devices, AP queues and AP matrix devices in
>> libvirt node device driver.
>> ---
>> v4:
>>   - Added virNodeDevAPAdapterParseXML function to extract the adapter
>>     parsing logic.
>>   - Modified according to review comments.
>>   - New patch to mention support for AP devices in NEWS.rst.
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
>
> I had 2 nitpicks which I can fix before merging, but I'd like to give other
> people a couple more days to express their "final" opinions and if there are no
> more comments, then sometime next week I'll merge this.
> There's one more little thing...Boris linked the s390 AP facility kernel
> documentation which really helped me during the review, so I think we should
> link it somewhere too - usually we're not so keen on doing that because 3rd
> party documentation URLs tend to die or migrate, but in this case it linked
> directly to the github repo (I think even the generated HTML on kernel.org
> would be just fine), but I don't know what the right place for this actually is
> as it describes the whole facility which we modelled in 3 capabilities. We
> could put it into the NEWS file, but then again, not sure how often anyone
> developing libvirt reads the NEWS file.
>
> Regards,
> Erik
>
> Hello Erik,
>
> Thank you very much for the review.
>

-- 
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294