oops... meant to post to list as well
beth kon wrote:
Daniel Veillard wrote:
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 !
Thanks for the quick comments!
+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 :-)
I was puzzled too! These fields are related to the case when 32 bit
guests need to reserve certain ranges of memory for DMA, for example.
I checked with Ryan and he assured me we don't need to worry about
those fields for these purposes.
+ 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;
+
[...]
What does [...] mean? :-)
+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.
Yes, you're right. I added maxcells = -1 and startCell as an
afterthought, and it shows!
+ /* 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;
I wondered about that. Would you like me to make that change as part
of this patch?
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);
Yes, right again!
Thanks so much for the great feedback!
--
Elizabeth Kon (Beth)
IBM Linux Technology Center
Open Hypervisor Team
email: eak@xxxxxxxxxx
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list