Re: [libvirt] [RFC PATCH 2/6] conf: Tweak HPT parsing and formatting
On Thu, 2018-05-24 at 07:18 +0200, Peter Krempa wrote:
On Wed, May 23, 2018 at 18:50:04 +0200, Andrea Bolognani wrote:
> On Wed, 2018-05-23 at 18:42 +0200, Peter Krempa wrote:
> > On Wed, May 23, 2018 at 18:17:58 +0200, Andrea Bolognani wrote:
> > > + if (hasResizing) {
> > > + if (virAsprintf(&resizing, "
resizing='%s'",
> > > + virDomainHPTResizingTypeToStri
> > > ng(def->hpt_resizing)) < 0) {
> > > + goto error;
> > > + }
> > > + } else {
> > > + if (VIR_STRDUP(resizing, "") < 0)
> > > + goto error;
> > > + }
> > >
> > > - virBufferAsprintf(buf, "<hpt
resizing='%s'/>\n",
> > > - virDomainHPTResizingTypeToString
> > > (def->hpt_resizing));
> > > + virBufferAsprintf(buf, "<hpt%s/>\n",
> >
> > This formulation looks fishy.
Okay, I think I see now why you called it fishy :)
Basically there are three possibilities for how the output will
look like:
<hpt resizing='x'/>
<hpt>
<maxpagesize>x</maxpagesize>
</hpt>
<hpt resizing='x'>
<maxpagesize>x</maxpagesize>
</hpt>
Checks against hasResizing and hasMaxPageSize ensure only the
combinations that make sense result in output being produced, so
<hpt/>
will never happen, but that's not immediately apparent by looking
at the snippet above.
> I don't love it either, but I've tried a bunch of
alternative
> approaches and this seemed like the most sane to me.
>
> If you have suggestions on how to improve it, considering that the
> end result is what you see after patch 5/6, please do share! :)
virXMLFormatElement automatically closes the tag if the provided
'attrBuf' is empty. Currently it will not work for this particular case
but I think it is worth to add a version which will format the element
even if both buffers are empty.
I was not aware of that function, thanks for mentioning it!
As explained above, the case where both buffers are empty is not
supposed to produce any output, so the existing function already
does exactly what I need :)
--
Andrea Bolognani / Red Hat / Virtualization