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