Re: [libvirt] [PATCH 08/13] conf: Introduce parser, formatter for uid and fid
On 06/08/2018 03:40 AM, Laine Stump wrote:
On 05/25/2018 07:05 AM, Pino Toscano wrote:
> On Thursday, 24 May 2018 14:24:33 CEST Xiao Feng Ren wrote:
>> From: Yi Min Zhao <zyimin(a)linux.ibm.com>
>>
>> This patch introduces new XML parser/formatter functions. Uid is
>> 16-bit and non-zero. Fid is 32-bit. They are added as two new
>> attributes of PCI address, and parsed/formatted along with PCI
>> address parser/formatter.
>>
>> Signed-off-by: Yi Min Zhao <zyimin(a)linux.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
>> Reviewed-by: Stefan Zimmermann <stzi(a)linux.ibm.com>
>> Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
>> ---
>> docs/schemas/basictypes.rng | 28 ++++++++++++++++
>> docs/schemas/domaincommon.rng | 1 +
>> src/conf/device_conf.c | 74 +++++++++++++++++++++++++++++++++++++++++++
>> src/conf/domain_addr.c | 3 ++
>> src/conf/domain_conf.c | 4 +++
>> src/util/virpci.h | 3 ++
>> 6 files changed, 113 insertions(+)
>>
>> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
>> index 1a18cd31b1..8050a3ebc4 100644
>> --- a/docs/schemas/basictypes.rng
>> +++ b/docs/schemas/basictypes.rng
>> @@ -111,6 +111,34 @@
>> </attribute>
>> </optional>
>> </define>
>> + <define name="zpciaddress">
>> + <optional>
>> + <attribute name="uid">
>> + <choice>
>> + <data type="string">
>> + <param
name="pattern">(0x)?[0-9a-fA-F]{1,4}</param>
>> + </data>
>> + <data type="unsignedInt">
>> + <param name="minInclusive">1</param>
>> + <param name="maxInclusive">65535</param>
>> + </data>
>> + </choice>
>> + </attribute>
> This seems to be the "uint16" type already.
>
>> + </optional>
>> + <optional>
>> + <attribute name="fid">
>> + <choice>
>> + <data type="string">
>> + <param
name="pattern">(0x)?[0-9a-fA-F]{1,8}</param>
>> + </data>
>> + <data type="unsignedLong">
>> + <param name="minInclusive">0</param>
>> + <param name="maxInclusive">4294967295</param>
>> + </data>
>> + </choice>
>> + </attribute>
> This could be a new "uint32" type, changing the 0x prefix as
> non-optional (otherwise the value "10" can be both valid as decimal
> and hexadeciaml).
My personal opinion - if numbers without a leading 0x are consistently
not interpreted as hexadecimal, then there is no ambiguity and no
confusion. If there's a leading 0x, then it is hexadecimal. No leading
0x, it's decimal. Period.
Laine,
so are you saying the pattern for type string should for uid and fid
start with 0x instead of (0x)?. That would certainly work.
The difference is than the error message if e.g. for uid a user specifies 0.
The schema error is than
error: XML document failed to validate against schema: Unable to
validate doc against /usr/share/libvirt/schemas/domain.rng
Extra element devices in interleave
Element domain failed to validate content
and with the optional 0x the value 0 would bypass the schema and the
checks in the code reports the user friendly error
error: XML error: Invalid PCI address uid='0x0', must be > 0x0 and <=
0xffff
I agree that the later is really tricking the schema but with usability
in mind (you hopefully agree that the schema error message su(&s ) I
definitely vote for the later solution.... :-(
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294