beth kon wrote:
Here is my first pass at a patch for accessing the available memory associated with each NUMA cell through libvirt.The initial framework patch provided by Daniel Veillard is a prereq of this patch:https://www.redhat.com/archives/libvir-list/2007-September/msg00069.html I have not yet tested this. I'm just distributing for comments. A few comments/questions: 1) I think I got the versioning stuff right but that deserves a close look.2) In Daniel's patch, libvirt.h and libvirt.h.in were identical. I assumed the patch would be before running autogen.sh, and only contain changes in libvirt.h.in, so that's how I did mine. Let me know if I misunderstand something here.
That fine, although be aware that libvirt.h only gets recreated when you run ./configure explicitly. I think there should be a dependency in the Makefile so that it can be recreated from config.status, but that's another bug so don't worry about it for this.
3) I had to put #ifdef PROXY around calls to virXenErrorFunc in xenHypervisorNodeGetCellsFreeMemory to get this to build. I haven't figured out how the proxy code works so am not sure if this is the right approach.
As a general background, the Xen proxy was an earlier attempt to allow query Xen without having to be root. The Xen proxy was a setuid binary which only contains "safe" read-only code. It was built from the same sources as libvirt but with any code thought to be unsafe disabled (#ifndef PROXY ... #endif).
In any case the proxy ability has been completely superseded by a combination of remote access and PolicyKit. In fact I think Dan was planning to remove the Xen proxy entirely.
Comments on the code itself: int virNodeGetCellsFreeMemory(virConnectPtr conn, - unsigned long *freeMems, - int nbCells); + long long *freeMems, + int startCell, + int maxCells);I guess Daniel Veillard can comment on this change. Did we decide that this wouldn't be useful on realistic topologies because interesting cell ranges would never be contiguous?
+ * @freeMems: pointer to the array of long long + * @startCell: index of first cell to return freeMems info on (0 to + * maxCells - 1). + * @maxCells: number of entries available in freeMems (-1 for sum of + * free memory across all cells, returned in freeMems[0])I'm slightly confused by this documentation. If maxCells > 0, does this only fill a subset of the array freeMems?
- if ((freeMems == NULL) || (nbCells <= 0)) {
+ if ((freeMems == NULL) || (maxCells <= 0) || (startCell > maxCells
- 1)) {
virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return (-1);
}
But we allow maxCells == -1 though, so this test is wrong.
+ if ((conn == NULL) || (freeMems == NULL) || (maxCells < 0))
+ return -1;
Should always call the error function before returning an error.
Unfortunately the error handling in libvirt is over-engineered in the
extreme and, what is more, in order to get around this over-engineering
we have defined "simpler" wrapper error functions in different /
incompatible ways in each file. In this particular file
(xen_internal.c) the "simpler" error function is called virXenErrorFunc.
+ /* get actual number of cells */
+ if (xenDaemonNodeGetInfo(conn, nodeInfoPtr)) {
+ virXenError(VIR_ERR_XEN_CALL, " cannot determine actual number
of cells", 0);
+ return -1; + } + nbCells = nodeInfoPtr->nodes; Is there a way for me to find out the total number of cells (nbCells)? + if (maxCells > nbCells) + maxCells = nbCells;I would prefer to return an error here, but it's not too bad because the return value tells us how many cells were filled.
+ if (startCell > nbCells - 1) + return -1; Surely, the condition should be `startCell + maxCells > nbCells'? Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
smime.p7s
Description: S/MIME Cryptographic Signature
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
