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 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list