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

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt;