On 11/13/20 4:50 PM, Jonathon Jongsma wrote:
On Fri, 13 Nov 2020 15:19:11 +0100
Shalini Chellathurai Saroja <shal...@linux.ibm.com> wrote:
On 11/12/20 6:01 PM, Jonathon Jongsma wrote:
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index d10a79e3..45281363 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -439,6 +439,17 @@
<dd>AP Card identifier.</dd>
</dl>
</dd>
+ <dt><code>ap_queue</code></dt>
+ <dd>Describes the AP Queue on a S390 host. An AP Queue
is
+ identified by it's ap-adapter and ap-domain id.
"it's" should be "its". Is it worth a very brief description of
what an AP queue actually is?
Sure, I will provide a brief description on AP queue.
...
+ <define name='capapqueue'>
+ <attribute name='type'>
+ <value>ap_queue</value>
+ </attribute>
+ <element name='ap-adapter'>
+ <ref name='apAdapterRange'/>
+ </element>
+ <element name='ap-domain'>
+ <ref name='apDomainRange'/>
+ </element>
+ </define>
Let's use double quotes to keep the file consistent.
Sure:-)
+
<define name='address'>
<element name='address'>
<attribute name='domain'><ref name='hexuint'/></attribute>
@@ -738,4 +751,16 @@
</choice>
</define>
+ <define name="apDomainRange">
+ <choice>
+ <data type="string">
+ <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param>
+ </data>
+ <data type="int">
+ <param name="minInclusive">0</param>
+ <param name="maxInclusive">255</param>
Is 255 correct here? the hex pattern above implies that it is a 16
bit value, but here you're limiting it to 255. If it is a 16 bit
value, can we just use the already-defined 'uint16' type from
basictypes.rng?
Yes, it is. Current architecture limit is 256 (0-255) for domains.
The address schema of domains was created in linux on z, with a
future architectural change in mind.
...
So, this is a bit confusing to me. The xml schema appears to be
trying to prevent me from specifying a value above 255 if I enter
it as an integer, yet it allows me to specify a value of 0xFFFF
if I specify it as a hex value.
Side note: I can also enter plain integers beyond 255 because of the
fact that the '0x' prefix is optional. 9999 will validate according to
the schema because it matches the hex string pattern.
However I note that your parsing code does not enforce this 255 limit
anywhere.
I will enforce the maximum limit of domain value as 255 in the parsing code.
I will modify the apDomainRange definition to have the prefix '0x' as
required instead of optional.
Thank you for pointing it out:-)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c index b4eb4553..6bbff571
100644 --- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1218,6 +1218,29 @@ udevProcessAPCard(struct udev_device
*device, }
+static int
+udevProcessAPQueue(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+ char *c;
+ virNodeDevCapDataPtr data = &def->caps->data;
+
In the previous patch, you added a comment explaining the format of
the sysfs path. I found that helpful. Without knowing the format,
it's a bit difficult to judge whether this code below is correct.
I will add the below comment
/* The sysfs path would be in the format /sys/bus/ap/devices/xx.yyyy,
where xx is ap adapter id and yyyy is ap domain id.
eg:/sys/bus/ap/devices/08.0001 */
--
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
