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

Andrea Bolognani abologna at redhat.com
Thu Mar 24 10:32:51 UTC 2022


On Thu, Mar 24, 2022 at 09:58:28AM +0100, Martin Kletzander wrote:
> What I assume is that allowReboot is one of the few, if not the only
> exception where we format the default zero value.

My guess is that you're right, but I haven't actually verified this
yet :)

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

Yeah, this sounds better from the caller's point of view, but it
would require adding a check to make sure that only one of
VIR_XML_PROP_NONZERO and VIR_XML_PROP_ALLOW_ZERO has been passed.
I'll see how clunky that looks.

> 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.

There are other helpers, such as virXMLPropUInt(), that don't
unconditionally set the flag. For all those, the behavior is up to
the caller, and having the flag makes sense.

-- 
Andrea Bolognani / Red Hat / Virtualization



More information about the libvir-list mailing list