On Mon, Sep 24, 2007 at 11:30:53PM -0400, beth kon wrote:

[PATCH 2/2] - add capability to access topology information (cell to cpu mapping) for each numa cell.

Signed-off-by: Beth Kon (eak@us.ibm.com)

-- Elizabeth Kon (Beth) IBM Linux Technology Center Open Hypervisor Team email: eak@us.ibm.com

diff -urpN libvirt.cellsMemory/src/xend_internal.c libvirt.topology/src/xend_internal.c --- libvirt.cellsMemory/src/xend_internal.c 2007-09-24 21:51:30.000000000 -0400 +++ libvirt.topology/src/xend_internal.c 2007-09-24 23:09:20.000000000 -0400 @@ -30,6 +30,7 @@ #include <arpa/inet.h> #include <netdb.h> #include <libxml/uri.h> +#include <ctype.h> #include <errno.h>

#include "libvirt/libvirt.h" @@ -1873,6 +1874,181 @@ sexpr_to_xend_node_info(struct sexpr *ro return (0); }

+/** + * getNumber: + * @pointer: pointer to string beginning with numerical characters + * @result: pointer to integer for storing the numerical result + * + * Internal routine extracting a number from the beginning of a string + * + * Returns the number of characters that were extracted as digits + * or -1 if no digits were found. + */ +static int +getNumber (const char * pointer, int * result) { + int len = 0; + while (isdigit(*(pointer + len))) + len++; + if (len == 0) + return -1; + *(result) = atoi(pointer); + return (len); +}

I'm always a bit vary of libc isXXXX since they tend to fluctuate based on locale while you don't expect so when parsing some input. In that case it's safe though.

+/** + * sexpr_to_xend_topology_xml: + * @root: an S-Expression describing a node + * + * Internal routine creating an XML string with the values from + * the node root provided. + * + * Returns 0 in case of success, -1 in case of error + */ +static int +sexpr_to_xend_topology_xml(virConnectPtr conn, struct sexpr *root, virBufferPtr xml) +{ + const char *nodeToCpu, *offset; + int cellNum, numCells = 0, numCpus, cellCpuCount = 0, nodeCpuCount = 0, start, finish, r; + int i, len, cpuNum, *cpuIdsPtr = NULL, *iCpuIdsPtr = NULL; + char next; + + nodeToCpu = sexpr_node(root, "node/node_to_cpu"); + if (nodeToCpu == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to parse topology information")); + goto error; + }

So if the host is not NUMA or an older Xen version, will you get that error back to the user ?

+ numCells = sexpr_int(root, "node/nr_nodes"); + numCpus = sexpr_int(root, "node/nr_cpus"); + + /* array for holding all cpu numbers associated with a single cell. Should never need + more than numCpus (which is total number of cpus for the node) */ + cpuIdsPtr = iCpuIdsPtr = malloc(numCpus * sizeof(int)); + if (cpuIdsPtr == NULL) { + goto vir_buffer_failed; + } + + /* start filling in xml */ + r = virBufferVSprintf (xml, + "\ +<topology>\n\ + <cells num='%d'>\n", + numCells);

I know this indentation was borrowed from other code, but this hurts my love for proper indentation :-)

+ if (r == -1) goto vir_buffer_failed; + + offset = nodeToCpu; + /* now iterate through all cells and find associated cpu ids */ + /* example of string being parsed: "node0:0-3,7,9-10\n node1:11-14\n" */

For multi lines comments we usually use /* * ..... * ..... */

+ while ((offset = strstr(offset, "node")) != NULL) { should you just skip blanks there instead ?

+ cpuIdsPtr = iCpuIdsPtr; + cellCpuCount = 0; + offset +=4; + if ((len = getNumber(offset, &cellNum)) < 0) { + virXendError(conn, VIR_ERR_XEN_CALL, " topology string syntax error"); + goto error; + } + offset += len; + if (*(offset) != ':') { + virXendError(conn, VIR_ERR_XEN_CALL, " topology string syntax error"); + goto error; + } + offset++; + /* get list of cpus associated w/ single cell */ + while (1) { + if ((len = getNumber(offset, &cpuNum)) < 0) { + virXendError(conn, VIR_ERR_XEN_CALL, " topology string syntax error"); + goto error; + } + offset += len; + next = *(offset); + if (next == '-') { + offset++; + start = cpuNum; + if ((len = getNumber(offset, &finish)) < 0) { + virXendError(conn, VIR_ERR_XEN_CALL, " topology string syntax error"); + goto error; + } + if (start > finish) { + virXendError(conn, VIR_ERR_XEN_CALL, " topology string syntax error"); + goto error; + + } + for (i=start; i<=finish && nodeCpuCount<numCpus; i++) { + *(cpuIdsPtr++) = i; + cellCpuCount++; + nodeCpuCount++; + } + if (nodeCpuCount >= numCpus) { + virXendError(conn, VIR_ERR_XEN_CALL, "conflicting cpu counts"); + goto error; + } + offset += len; + next = *(offset); + offset++; + if (next == ',') { + continue; + } else if (next == '\n') { + break; + } else { + virXendError(conn, VIR_ERR_XEN_CALL, " topology string syntax error"); + goto error; + } + } else { + /* add the single number */ + if (nodeCpuCount >= numCpus) { + virXendError(conn, VIR_ERR_XEN_CALL, "conflicting cpu counts"); + goto error; + } + *(cpuIdsPtr) = cpuNum; + cpuIdsPtr++; + cellCpuCount++; + nodeCpuCount++; + if (next == ',') { + offset++; + continue; + } else if (next == '\n') { + break; + } else { + virXendError(conn, VIR_ERR_XEN_CALL, " topology string syntax error"); + goto error; + } + } + }

Loops look fine from visual inspection

+ /* add xml for all cpus associated with one cell */ + r = virBufferVSprintf (xml, "\ + <cell id='%d'>\n\ + <cpus num='%d'>\n", cellNum, cellCpuCount); + if (r == -1) goto vir_buffer_failed; + + for (i = 0; i < cellCpuCount; i++) { + r = virBufferVSprintf (xml, "\ + <cpu id='%d'/>\n", *(iCpuIdsPtr + i)); + if (r == -1) goto vir_buffer_failed; + } + r = virBufferAdd (xml, "\ + </cpus>\n\ + </cell>\n", -1); + if (r == -1) goto vir_buffer_failed; + } + r = virBufferAdd (xml, "\ + </cells>\n\ +</topology>\n", -1); + if (r == -1) goto vir_buffer_failed; + free(cpuIdsPtr); + return (0); + + +vir_buffer_failed: + virXendError(conn, VIR_ERR_NO_MEMORY, _("allocate new buffer")); + +error: + if (cpuIdsPtr) + free(cpuIdsPtr); + return (-1); +} + #ifndef PROXY /** * sexpr_to_domain: @@ -2601,6 +2777,39 @@ xenDaemonNodeGetInfo(virConnectPtr conn, return (ret); }

+/** + * xenDaemonNodeGetTopology: + * @conn: pointer to the Xen Daemon block

* @xml: buffer where the resulting XML will be stored

+ * + * This method retrieves a node's topology information. + * + * Returns -1 in case of error, 0 otherwise. + */ +int +xenDaemonNodeGetTopology(virConnectPtr conn, virBufferPtr xml) { + int ret = -1; + struct sexpr *root; + + if (!VIR_IS_CONNECT(conn)) { + virXendError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__); + return (-1); + } + + if (xml == NULL) { + virXendError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + return (-1); + } + + root = sexpr_get(conn, "/xend/node/"); + if (root == NULL) { + return (-1); + } + + ret = sexpr_to_xend_topology_xml(conn, root, xml); + sexpr_free(root); + return (ret); +} + #ifndef PROXY /** * xenDaemonGetType: diff -urpN libvirt.cellsMemory/src/xend_internal.h libvirt.topology/src/xend_internal.h --- libvirt.cellsMemory/src/xend_internal.h 2007-09-10 17:35:39.000000000 -0400 +++ libvirt.topology/src/xend_internal.h 2007-09-24 22:32:53.000000000 -0400 @@ -19,6 +19,7 @@ #include <stdbool.h>

#include "libvirt/libvirt.h" +#include "buf.h"

#ifdef __cplusplus extern "C" { @@ -182,6 +183,7 @@ int xenDaemonOpen(virConnectPtr conn, co int xenDaemonClose(virConnectPtr conn); int xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer); int xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info); +int xenDaemonNodeGetTopology(virConnectPtr conn, virBufferPtr xml); int xenDaemonDomainSuspend(virDomainPtr domain); int xenDaemonDomainResume(virDomainPtr domain); int xenDaemonDomainShutdown(virDomainPtr domain); diff -urpN libvirt.cellsMemory/src/xen_internal.c libvirt.topology/src/xen_internal.c --- libvirt.cellsMemory/src/xen_internal.c 2007-09-24 22:04:05.000000000 -0400 +++ libvirt.topology/src/xen_internal.c 2007-09-24 22:32:53.000000000 -0400 @@ -2228,7 +2228,7 @@ xenHypervisorMakeCapabilitiesXML(virConn char line[1024], *str, *token; regmatch_t subs[4]; char *saveptr = NULL; - int i, r; + int i, r, topology;

char hvm_type[4] = ""; /* "vmx" or "svm" (or "" if not in CPU). */ int host_pae = 0; @@ -2466,6 +2466,10 @@ xenHypervisorMakeCapabilitiesXML(virConn </guest>\n", -1); if (r == -1) goto vir_buffer_failed; } + topology = xenDaemonNodeGetTopology(conn, xml); + if (topology != 0) + goto topology_failed; + r = virBufferAdd (xml, "\ </capabilities>\n", -1); @@ -2478,6 +2482,7 @@ xenHypervisorMakeCapabilitiesXML(virConn

vir_buffer_failed: virXenError(VIR_ERR_NO_MEMORY, __FUNCTION__, 0); + topology_failed: virBufferFree (xml); return NULL; }

Okay, end looks fine to me. Now we just need to check if this works :-) thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/