[RFC 1/1] virxml: Accept 'default' for virTristate* properties

[Martin Kletzander]

On Wed, Mar 23, 2022 at 07:04:20PM +0100, Andrea Bolognani wrote:
>The _ABSENT value of each enumeration has 'default' as string
>representation, and when that's been formatted to XML we should
>parse it back successfully, so we can't just treat encountering
>it as an error.
>
>Callers of virXMLPropTristate*() can of course still pass
>VIR_XML_PROP_NONZERO explicitly to the helpers if the current
>behavior is the one they want.
>
>After this change, libvirtd no longer logs
>
>  error : virXMLPropEnumInternal:516 : XML error: Invalid value
>  for attribute 'value' in element 'allowReboot': 'default'.
>
>when it gets restarted while there are running guests.
>

I had the same issue, but did not get around sending a patch for it.
What I assume is that allowReboot is one of the few, if not the only
exception where we format the default zero value.

>Fixes: 8861d96c880d25c940456c5997a2ac93fc073c78
>Fixes: c8726ede83ac117cb18c0b0a1fbfeeac8b80384b
>Signed-off-by: Andrea Bolognani <abologna at redhat.com>
>---
> src/util/virxml.c | 4 ----
> 1 file changed, 4 deletions(-)
>
>diff --git a/src/util/virxml.c b/src/util/virxml.c
>index 8ff59e7cda..db5212de20 100644
>--- a/src/util/virxml.c
>+++ b/src/util/virxml.c
>@@ -545,8 +545,6 @@ virXMLPropTristateBool(xmlNodePtr node,
>                        virXMLPropFlags flags,
>                        virTristateBool *result)
> {
>-    flags |= VIR_XML_PROP_NONZERO;
>-

I would rather change this flag to something like
VIR_XML_PROP_ALLOW_ZERO and only allow parsing default values with this
flag making the callers be able to opt in for this behaviour rather then
all the others having to opt out.

Honestly I do not understand the flag as it is currently because it does
completely nothing when these functions just add the flag in
unconditionally.

>     return virXMLPropEnumInternal(node, name, virTristateBoolTypeFromString,
>                                   flags, result, VIR_TRISTATE_BOOL_ABSENT);
> }
>@@ -573,8 +571,6 @@ virXMLPropTristateSwitch(xmlNodePtr node,
>                          virXMLPropFlags flags,
>                          virTristateSwitch *result)
> {
>-    flags |= VIR_XML_PROP_NONZERO;
>-
>     return virXMLPropEnumInternal(node, name, virTristateSwitchTypeFromString,
>                                   flags, result, VIR_TRISTATE_SWITCH_ABSENT);
> }
>-- 
>2.35.1
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220324/9443cdeb/attachment.sig>