[Libvir] [RFC][PATCH 2/2] NUMA memory and topology patches
Daniel Veillard
veillard at redhat.com
Tue Sep 25 15:33:26 UTC 2007
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 at us.ibm.com)
>
> --
> Elizabeth Kon (Beth)
> IBM Linux Technology Center
> Open Hypervisor Team
> email: eak at 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 at redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
More information about the libvir-list
mailing list