[libvirt] [PATCH 0/4] libxl: implement some chuncks of the NUMA interface

[Jim Fehlig]

On 06/28/2013 08:32 AM, Dario Faggioli wrote:
> Hi Jim, Everyone,
>
> First patch series, so maybe a small introduction is required: I'm Dario and I
> work for Citrix on improving the NUMA support of Xen.

Welcome Dario!

> This patch series implements some of the missing bits and pieces, in the libxl
> driver, regarding obtaining per-host and per-domain NUMA related information.
> It's not the full libvirt NUMA interface, since we don't have all we would need
> for that in Xen yet (we will in the next version, 4.4), but it's certainly
> better than having nothing! :-)

Yep, incremental improvements are fine.

> Basically, I'm enhancing capability reporting, to cover NUMA topology (patch
> 01), and I'm implementing  nodeGetCellsFreeMemory (patch 02) and
> virDomainGetNumaParameters (patch 04) for the libxl driver. This last one
> requires at least Xen 4.3, so I put the implementation within the proper
> #ifdef-ery.

Ok, I'll comment in the individual patches too.

> What I'm really not sure about is patch 03, which is something I need if I want
> patch 04 to function properly. Basically it is about advertising that the libxl
> driver supports VIR_TYPED_PARAM_STRING. I looked at how that is done in the
> qemu driver, but I'm not entirely sure I completely understood the logic behind
> it, so, please, tell me if I missed or misinterpreted anything!

I think we'll need another libvirt dev more familiar with this to verify, but 
afaict advertising that the driver supports VIR_TYPED_PARAM_STRING is required 
when returning a string in virTypedParam, for compatibility with older libvirt 
clients. Without it, strings wouldn't be returned to newer clients that support 
VIR_TYPED_PARAM_STRING.

>    In particular,
> should I have added more of those "flags &= ~VIR_TYPED_PARAM_STRING_OKAY;"
> statements, as it happens in the qemu driver? If yes, in which functions?

Once the driver advertises VIR_TYPED_PARAM_STRING, I think the functions taking 
virTypedParamPtr as a parameter must accommodate that, at least the getters.  
E.g. qemuDomainGetSchedulerParametersFlags() in the qemu driver has

     virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                   VIR_DOMAIN_AFFECT_CONFIG |
                   VIR_TYPED_PARAM_STRING_OKAY, -1);

     /* We don't return strings, and thus trivially support this flag.  */
     flags &= ~VIR_TYPED_PARAM_STRING_OKAY;

I think the same applies for libxlDomainGetSchedulerParametersFlags(), which 
looks to be the only getter function in the libxl driver taking virTypedParam.


> Finally, allow me to say that it was a while that I wanted start hacking a bit
> on libvirt.  I'm really glad I've eventually been able to do so, and I
> definitely plan to continue (with particular focus on NUMA related stuff).

Great to hear and looking forward to your work!

Regards,
Jim