On Thu, Sep 20, 2007 at 05:10:49PM -0400, beth kon wrote:
Here is my first pass at a patch for accessing the available memory
associated with each NUMA cell through libvirt.
Thanks !
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
That applied fine to my tree using CVS HEAD + the patch.
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.
I looked at it, it looks fine but need testing for the differentiation
between 3.1 which doesn't have the feature and current xen-unstable which does.
2) In Daniel's patch, libvirt.h and libvirt.h.in were identical.
I
nearly identical, yes
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's right, I just provided both as that's what CVS diff gave after
autogen, no problem here.
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.
It compiles only a subset of the xen_internal.c code. Don't worry too much
about it, the proxy code fate is to disapear at some point.
int virNodeGetCellsFreeMemory(virConnectPtr conn,
- unsigned long *freeMems,
- int nbCells);
+ long long *freeMems,
+ int startCell,
+ int maxCells);
okay, so switch to long long and add a start cell number, perfect.
/**
* virNodeGetCellFreeMemory:
+
* @conn: pointer to the hypervisor connection
- * @freeMems: pointer to the array of unsigned long
- * @nbCells: number of entries available in freeMems
+ * @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])
*
- * This call allows to ask the amount of free memory in each NUMA cell.
+ * This call is a request for the amount of free memory, either in the whole
+ * node, or in each NUMA cell.
* The @freeMems array must be allocated by the caller and will be filled
* with the amounts of free memory in kilobytes for each cell starting
- * from cell #0 and up to @nbCells -1 or the number of cell in the Node
+ * from cell #0 and up to @maxCells -1 or the number of cells in the Node
* (which can be found using virNodeGetInfo() see the nodes entry in the
* structure).
+ * If maxCells == -1, the free memory will be summed across all cells and
+ * returned in freeMems[0].
Hum, that -1 handling is a bit surprizing, but apparently that's what
the Xen hypercall does. It's a bit convoluted as an API to get the full
free memory on a Node, maybe this should be separated as a different API
as people looking for this information may not think about looking at
NUMA special entry points. We don't have currently a simple API to
get the free memory on a Node, maybe that should be added.
+struct xen_v2s4_availheap {
+ uint32_t min_bitwidth; /* Smallest address width (zero if don't care). */
+ uint32_t max_bitwidth; /* Largest address width (zero if don't care). */
I'm a bit puzzled by those 2 fields I still wonder what it is about :-)
+ int32_t node; /* NUMA node (-1 for sum across all
nodes). */
+ uint64_t avail_bytes; /* Bytes available in the specified region. */
+};
+
+typedef struct xen_v2s4_availheap xen_v2s4_availheap;
+
[...]
@@ -2012,7 +2028,7 @@ xenHypervisorInit(void)
#endif
return(-1);
}
- /* Currently consider RHEL5.0 Fedora7 and xen-unstable */
+ /* Currently consider RHEL5.0 Fedora7, xen-3.1, and xen-unstable */
sys_interface_version = 2; /* XEN_SYSCTL_INTERFACE_VERSION */
if (virXen_getdomaininfo(fd, 0, &info) == 1) {
/* RHEL 5.0 */
@@ -2035,7 +2051,7 @@ xenHypervisorInit(void)
sys_interface_version = 3; /* XEN_SYSCTL_INTERFACE_VERSION */
if (virXen_getdomaininfo(fd, 0, &info) == 1) {
- /* xen-unstable */
+ /* xen-3.1 */
dom_interface_version = 5; /* XEN_DOMCTL_INTERFACE_VERSION */
if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0){
#ifdef DEBUG
@@ -2045,6 +2061,18 @@ xenHypervisorInit(void)
}
}
+ sys_interface_version = 4; /* XEN_SYSCTL_INTERFACE_VERSION */
+ if (virXen_getdomaininfo(fd, 0, &info) == 1) {
+ /* xen-unstable */
+ dom_interface_version = 5; /* XEN_DOMCTL_INTERFACE_VERSION */
+ if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0){
+#ifdef DEBUG
+ fprintf(stderr, "Using hypervisor call v2, sys ver4 dom ver5\n");
+#endif
+ goto done;
+ }
+ }
+
hypervisor_version = 1;
sys_interface_version = -1;
if (virXen_getdomaininfo(fd, 0, &info) == 1) {
That looks right, I agree, but need testing for example on a normal F-7
which has 3.1 and one with updated xen.
+xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, long long
*freeMems,
+ int startCell, int maxCells)
{
- if ((conn == NULL) || (freeMems == NULL) || (nbCells < 0))
- return -1;
+ xen_op_v2_sys op_sys;
+ int i, ret, nbCells;
+ virNodeInfo nodeInfo;
+ virNodeInfoPtr nodeInfoPtr = &nodeInfo;
+ xenUnifiedPrivatePtr priv;
+
+ if ((conn == NULL) || (freeMems == NULL) || (maxCells < 0))
+ return -1;
Hum, actually, that would catch the (maxCells == -1) so that won't work
and won't catch maxCells == 0 which could lead to a segfault. Maybe
(maxCells == 0) || (maxCells < -1) should be used instead.
+ /* get actual number of cells */
+ if (xenDaemonNodeGetInfo(conn, nodeInfoPtr)) {
Hum, since the number of cells is static, maybe this should be stored
permanently in a variable on init. The xenDaemon RPC will be orders of magnitude
more expensive than the direct hypercall below.
+ virXenError(VIR_ERR_XEN_CALL, " cannot determine actual
number of cells", 0);
+ return -1;
+ }
+ nbCells = nodeInfoPtr->nodes;
+
+ if (maxCells > nbCells)
+ maxCells = nbCells;
Hum ... maxCells is the number of entries in the array, I'm afraid you will
need 2 counters or I misunderstood the way the API works (possible :-),
I would fill in freeMems[] starting at index 0 for startCell, and going up.
That feels more coherent than leaving uninitialized values at the start of
the array:
for (i = startCell, j = 0;(i < nbCells) && (j < maxCells);i++,j++) {
op_sys.u.availheap.node = i;
ret = xenHypervisorDoV2Sys(priv->handle, &op_sys);
if (ret < 0)
return(-1);
freeMems[j] = op_sys.u.availheap.avail_bytes;
}
return (j);
+
+ if (startCell > nbCells - 1)
+ return -1;
+
+ for (i = startCell; i < maxCells; i++) {
+ op_sys.u.availheap.node = i;
+ ret = xenHypervisorDoV2Sys(priv->handle, &op_sys);
+ if (ret < 0)
+ return(-1);
+ freeMems[i] = op_sys.u.availheap.avail_bytes;
+ }
+ return (maxCells - startCell);
}
A few things to check, but that's looks like a good start :-)
thanks a lot !
Daniel
--
Red Hat Virtualization group
http://redhat.com/virtualization/
Daniel Veillard | virtualization library
http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit
http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine
http://rpmfind.net/