[libvirt] [PATCH] libxl: implement NUMA capabilities reporting

[Jim Fehlig]

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
>