On Tue, 2016-04-19 at 18:52 -0400, Cole Robinson wrote:
> On 04/18/2016 01:44 PM, Andrea Bolognani wrote:
> >
> > Implement support for saving GIC capabilities in the cache and
> > read them back.
> > ---
> > src/qemu/qemu_capabilities.c | 87 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 87 insertions(+)
>
> I was surprised by lack of test cases, but it seems a common problem in this
> area of the code, so nothing to handle for this patch series...
Yeah, I mentioned that in the cover letter...
There's some stuff in tests/domaincapstest.c but it's far from
having good coverage - AFAICT it only tests formatting the
capabilities, not parsing them.
> > + for (i = 0; i < qemuCaps->ngicCapabilities; i++) {
> > + virGICCapabilityPtr cap;
> > + bool kernel;
> > + bool emulated;
> > +
> > + cap = &qemuCaps->gicCapabilities[i];
> > + kernel = (cap->implementation & VIR_GIC_IMPLEMENTATION_KERNEL);
> > + emulated = (cap->implementation & VIR_GIC_IMPLEMENTATION_EMULATED);
> > +
> > + virBufferAsprintf(&buf,
> > + "<gic version='%d' kernel='%s' emulated='%s'/>\n",
> > + cap->version,
> > + kernel ? "true" : "false",
> > + emulated ? "true" : "false");
> > + }
> > +
> > virBufferAdjustIndent(&buf, -2);
> > virBufferAddLit(&buf, "</qemuCaps>\n");
>
> Use of literal 'true'/'false' isn't common in our XML formats. This isn't user
> facing, but still probably better to use 'yes'/'no', if only so that some
> future cleanup can streamline things here :)
I've changed it to yes/no and pushed the whole series.
Thanks for your review! :)
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list