Re: [libvirt] [PATCH] libxl: implement NUMA capabilities reporting
Daniel P. Berrange wrote:
On Fri, Aug 16, 2013 at 03:46:29PM -0600, Jim Fehlig wrote:
> static int
> +libxlCapsInitNuma(libxl_ctx *ctx, virCapsPtr caps)
> +{
> + libxl_numainfo *numa_info = NULL;
> + libxl_cputopology *cpu_topo = NULL;
> + int nr_nodes = 0, nr_cpus = 0;
> + virCapsHostNUMACellCPUPtr *cpus = NULL;
> + int *nr_cpus_node = NULL;
> + size_t i;
> + int ret = -1;
> +
> + /* Let's try to fetch all the topology information */
> + numa_info = libxl_get_numainfo(ctx, &nr_nodes);
> + if (numa_info == NULL || nr_nodes == 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("libxl_get_numainfo failed"));
>
You're reporting a useful error here....
> + goto cleanup;
> + } else {
> + cpu_topo = libxl_get_cpu_topology(ctx, &nr_cpus);
> + if (cpu_topo == NULL || nr_cpus == 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("libxl_get_cpu_topology failed"));
>
And here, and so on....
> +
> + ret = 0;
> +
> + cleanup:
> + if (ret != 0) {
> + /* Something went wrong: deallocate everything and unref caps */
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("libxenlight failed to build the NUMA
topology"));
>
And overwriting the error with something useless. Just remove this
call to virReportError. Add a VIR_DEBUG log in its place if you
want something to highlight the situation in debugging modes.
Thanks for catching that. I've removed the unnecessary error reporting
and pushed the patch.
Regards,
Jim
> +
> + for (i = 0; i < nr_nodes; i++)
> + VIR_FREE(cpus[i]);
> + virCapabilitiesFreeNUMAInfo(caps);
> + }
> +
> + VIR_FREE(cpus);
> + VIR_FREE(nr_cpus_node);
> + libxl_cputopology_list_free(cpu_topo, nr_cpus);
> + libxl_numainfo_list_free(numa_info, nr_nodes);
> +
> + return ret;
> +}
> +
> +static int
> libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
> {
> const libxl_version_info *ver_info;
> @@ -880,6 +993,9 @@ libxlMakeCapabilities(libxl_ctx *ctx)
> if (libxlCapsInitHost(ctx, caps) < 0)
> goto error;
>
> + if (libxlCapsInitNuma(ctx, caps) < 0)
> + goto error;
> +
> if (libxlCapsInitGuests(ctx, caps) < 0)
> goto error;
>
ACK if that change above is made before pushing
Daniel