On Fri, Jul 25, 2008 at 04:44:09PM +0400, Evgeniy Sokolov wrote:
Patch switch OpenVZ XML format to generic. main changes: - I used generic virDomainNetDef to define network in container. And wrote function to apply virDomainNetDef for container. Method virDomainNetDefParseXML is public now. - tag "container" is changed to "devices" - changed format of tag "filesystem" - added processing "vcpu" tag.
Generally looks fine to me, just a few remarks
+/* Parse filesystem section +Sample: +<filesystem type="template"> + <source name="fedora-core-5-i386"/> + <quota type="size" max="10000"/> + <quota type="inodes" max="100"/> +</filesystem> +*/ +static int openvzParseDomainFS(virConnectPtr conn, + struct openvz_fs_def *fs, + xmlXPathContextPtr ctxt) +{ + xmlNodePtr cur, obj; + char *type; + + obj = virXPathNode("/domain/devices/filesystem[1]", ctxt); + if (obj == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("missing filesystem tag")); + return -1; + }
hum, maybe use virXPathNodeSet and checking you won't get more than one ?
/* Extract domain uuid */ - obj = xmlXPathEval(BAD_CAST "string(/domain/uuid[1])", ctxt); - if ((obj == NULL) || (obj->type != XPATH_STRING) || - (obj->stringval == NULL) || (obj->stringval[0] == 0)) { + prop = virXPathString("string(./uuid[1])", ctxt); + if (!prop) { int err;
Hum, if you start using relative XPath queries like that it's a good idea to make sure ctxt->node is set to the node you're starting the lookup from I don't see that in the patch, maybe I missed it...
+ } else { + if (virUUIDParse(prop, def->uuid) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("malformed uuid element")); + goto bail_out; + } + VIR_FREE(prop);
[...]
+ //TODO: processing NAT and phisical device
typo physical :-) In general that looks way cleaner to me, I will give it a few nmore days and apply, unless you suggest another version, 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/