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...
One comment below
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index ee80be2..be3e420 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2926,6 +2926,77 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename,
> }
> VIR_FREE(nodes);
>
> + if ((n = virXPathNodeSet("./gic", ctxt, &nodes)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to parse qemu capabilities gic"));
> + goto cleanup;
> + }
> + if (n > 0) {
> + unsigned int uintValue;
> + bool boolValue;
> +
> + qemuCaps->ngicCapabilities = n;
> + if (VIR_ALLOC_N(qemuCaps->gicCapabilities, n) < 0)
> + goto cleanup;
> +
> + for (i = 0; i < n; i++) {
> + virGICCapabilityPtr cap = &qemuCaps->gicCapabilities[i];
> +
> + if (!(str = virXMLPropString(nodes[i], "version"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing GIC version "
> + "in QEMU capabilities cache"));
> + goto cleanup;
> + }
> + if (str &&
> + virStrToLong_ui(str, NULL, 10, &uintValue) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("malformed GIC version "
> + "in QEMU capabilities cache"));
> + goto cleanup;
> + }
> + cap->version = uintValue;
> + VIR_FREE(str);
> +
> + if (!(str = virXMLPropString(nodes[i], "kernel"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing in-kernel GIC information "
> + "in QEMU capabilities cache"));
> + goto cleanup;
> + }
> + if (str &&
> + !(boolValue = STREQ(str, "true")) &&
> + STRNEQ(str, "false")) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("malformed in-kernel GIC information "
> + "in QEMU capabilities cache"));
> + goto cleanup;
> + }
> + if (boolValue)
> + cap->implementation |= VIR_GIC_IMPLEMENTATION_KERNEL;
> + VIR_FREE(str);
> +
> + if (!(str = virXMLPropString(nodes[i], "emulated"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing emulated GIC information "
> + "in QEMU capabilities cache"));
> + goto cleanup;
> + }
> + if (str &&
> + !(boolValue = STREQ(str, "true")) &&
> + STRNEQ(str, "false")) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("malformed emulated GIC information "
> + "in QEMU capabilities cache"));
> + goto cleanup;
> + }
> + if (boolValue)
> + cap->implementation |= VIR_GIC_IMPLEMENTATION_EMULATED;
> + VIR_FREE(str);
> + }
> + }
> + VIR_FREE(nodes);
> +
> ret = 0;
> cleanup:
> VIR_FREE(str);
> @@ -2992,6 +3063,22 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
> qemuCaps->machineMaxCpus[i]);
> }
>
> + 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 :)
ACK otherwise
- Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list