On Tue, Nov 03, 2009 at 04:40:01PM +0100, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/virterror.h | 1 + src/Makefile.am | 9 +- src/conf/capabilities.c | 31 ++++- src/conf/capabilities.h | 6 + src/conf/cpu_conf.c | 345 +++++++++++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 110 ++++++++++++++ src/conf/domain_conf.c | 15 ++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 8 + src/util/virterror.c | 3 + 10 files changed, 527 insertions(+), 3 deletions(-) create mode 100644 src/conf/cpu_conf.c create mode 100644 src/conf/cpu_conf.h
[...]
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c [...] + /* + * sockets, cores, and threads must all be either zero or nonzero; + * if all of them are zero, we don't care about topology + */ + if ((!def->sockets || !def->cores || !def->threads) && !all_zero) { + virCPUReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Invalid CPU topology")); + goto error; + } + }
Hum, I would just ban all being 0 and drop the element in that case which would match my suggestion at the schemas level.
+ n = virXPathNodeSet(conn, "./feature", ctxt, &nodes); + if (n < 0) + goto error; + + if (n > 0) { + if (VIR_ALLOC_N(def->features, n) < 0) + goto no_memory; + def->nfeatures = n; + } + + for (i = 0 ; i < n ; i++) { + char *name; [...] + + if (!(name = (char *) xmlNodeGetContent(nodes[i])) + || *name == 0) { + VIR_FREE(name); + virCPUReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Invalid CPU feature name")); + goto error; + }
let's avoid xmlNodeGetContent() and just use the name in an attribute it's better and simpler at this point, plus better if we want this construct to evolve in the future.
+ len = strlen(name); + for (j = 0 ; j < len ; j++) + name[j] = c_tolower(name[j]);
hum ... are we really normalizing features names ? On one hand we don't define the list, but on the other hand we lower-case them. Sounds a bit weird, either we control them or not. Patch looks fine overall, just those could of suggestions especially about matching the proposed changes in the schemas and avoiding tolower() Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/