[libvirt] [PATCH 2/2] conf: Fix formatting of <sev> element in domain capabilities XML

[Erik Skultety]

On Thu, Jun 14, 2018 at 05:59:53PM +0200, Ján Tomko wrote:
> On Thu, Jun 14, 2018 at 02:28:03PM +0200, Erik Skultety wrote:
> > We only formatted the <sev> element when QEMU supported the feature when
> > in fact we should always format the element to make clear that libvirt
> > knows about the feature and the fact whether it is or isn't supported
> > depends on QEMU version, in other words if QEMU doesn't support the
> > feature we're going to format the following into the domain capabilities
> > XML:
> >
> > <sev supported='no'/>
> >
> > Signed-off-by: Erik Skultety <eskultet at redhat.com>
> > ---
> > src/conf/domain_capabilities.c                       | 20 ++++++++++++--------
> > tests/domaincapsschemadata/basic.xml                 |  1 +
> > tests/domaincapsschemadata/bhyve_basic.x86_64.xml    |  1 +
> > tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml     |  1 +
> > tests/domaincapsschemadata/bhyve_uefi.x86_64.xml     |  1 +
> > tests/domaincapsschemadata/full.xml                  |  1 +
> > tests/domaincapsschemadata/libxl-xenfv-usb.xml       |  1 +
> > tests/domaincapsschemadata/libxl-xenfv.xml           |  1 +
> > tests/domaincapsschemadata/libxl-xenpv-usb.xml       |  1 +
> > tests/domaincapsschemadata/libxl-xenpv.xml           |  1 +
> > tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml     |  1 +
> > .../qemu_2.12.0-virt.aarch64.xml                     |  1 +
> > tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml     |  1 +
> > tests/domaincapsschemadata/qemu_2.12.0.s390x.xml     |  1 +
> > tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml    |  1 +
> > .../domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml    |  1 +
> > tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml      |  1 +
> > tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml     |  1 +
> > tests/domaincapsschemadata/qemu_2.7.0.s390x.xml      |  1 +
> > tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.8.0.s390x.xml      |  1 +
> > tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml     |  1 +
> > tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml     |  1 +
> > 26 files changed, 37 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> > index e5d943af50..9041e46622 100644
> > --- a/src/conf/domain_capabilities.c
> > +++ b/src/conf/domain_capabilities.c
> > @@ -559,16 +559,20 @@ static void
> > virDomainCapsFeatureSEVFormat(virBufferPtr buf,
> >                               virSEVCapabilityPtr const sev)
> > {
> > -    if (!sev)
> > -        return;
> >
> > -    virBufferAddLit(buf, "<sev supported='yes'>\n");
> > -    virBufferAdjustIndent(buf, 2);
> > -    virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
> > -    virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n",
> > +    if (!sev) {
> > +        virBufferAddLit(buf, "<sev supported='no'/>\n");
> > +    } else {
> > +        virBufferAddLit(buf, "<sev supported='yes'>\n");
> > +        virBufferAdjustIndent(buf, 2);
> > +        virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
> > +        virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n",
> >                           sev->reduced_phys_bits);
>
> Same issue as in previous patch.
>
> > -    virBufferAdjustIndent(buf, -2);
> > -    virBufferAddLit(buf, "</sev>\n");
> > +        virBufferAdjustIndent(buf, -2);
> > +        virBufferAddLit(buf, "</sev>\n");
> > +    }
>
> This may or may not be nicer with virXMLFormatElement. Thankfully we
> don't format anything if supported=no.
>
> With the reducedPhysBits change incorporated:
>
> Reviewed-by: Ján Tomko <jtomko at redhat.com>

Thanks, I'll actually have to squash this patch into the previous one, since
either domaincapstest or virschematest complained (depending on the order of the
changes).
I'll make the adjustment before merging.

Erik