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@xxxxxxxxxx>
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
