[PATCH libvirt v2 01/11] nodedev: detect AP card device
Shalini Chellathurai Saroja
shalini at linux.ibm.com
Mon Nov 16 13:07:27 UTC 2020
On 11/13/20 4:28 PM, Jonathon Jongsma wrote:
> On Fri, 13 Nov 2020 12:09:26 +0100
> Shalini Chellathurai Saroja <shalini at linux.ibm.com> wrote:
>
>> Hi Jonathon,
>>
>> Thank you for the quick review:-)
>>
>> On 11/12/20 5:27 PM, Jonathon Jongsma wrote:
>>>> + <define name='address'>
>>>> + <element name='address'>
>>>> + <attribute name='domain'><ref name='hexuint'/></attribute>
>>>> + <attribute name='bus'><ref name='hexuint'/></attribute>
>>>> + <attribute name='slot'><ref name='hexuint'/></attribute>
>>>> + <attribute name='function'><ref
>>>> name='hexuint'/></attribute>
>>> It seems that you're unnecessarily changing double-quotes to
>>> single-quotes here, which adds spurious changes to the diff. The
>>> rest of the file uses double-quotes. Let's stick with that.
>> Sure.
>>>
>>>> </element>
>>>> </define>
>>>>
>>>> @@ -716,4 +726,16 @@
>>>> </element>
>>>> </define>
>>>>
>>>> + <define name="apAdapterRange">
>>>> + <choice>
>>>> + <data type="string">
>>>> + <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param>
>>>> + </data>
>>>> + <data type="int">
>>>> + <param name="minInclusive">0</param>
>>>> + <param name="maxInclusive">255</param>
>>>> + </data>
>>>> + </choice>
>>>> + </define>
>>> As far as I can tell, this is identical to the definition of the
>>> 'uint8' type in basictypes.rng. Is there a reason for defining a
>>> different type?
>>>
>> Yes, In definition apAdapterRange, the prefix '0x' is optional.
>>
> Oh, I guess I missed that part. Now that I look at basictypes.rng, I
> see that the '0x' is required for uint8 and uint24, but it is optional
> for uint16 and uint32. Does anybody know the justification for these
> differences?
>
> That said, I don't believe that your parsing code actually supports an
> optional '0x' prefix. In virNodeDevCapAPCardParseXML(), you call
>
> virStrToLong_uip(adapter, NULL, 0, &ap_card->ap_adapter)
>
> But I'm quite sure that passing a value of e.g. 'ff' for adapter will
> result in a parsing failure. Try changing the ap-adapter value in
> tests/nodedevschemadata/ap_card07.xml to some different values and see
> what happens.
>
> Jonathon
Hi Jonathon,
That's true, I will use the uint8 definition instead, thank you:-)
--
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
More information about the libvir-list
mailing list