[libvirt] [PATCH] OpenVZ xml refactoring

[Evgeniy Sokolov]

> 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 ?
good idea! done
> 
>>      /* 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...
Yes, you found bug. Fixed now. Thanks!
> 
>> +    } 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,

fixed patch is attached.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: libvirt-openvz-xml-refactoring.diff
Type: text/x-patch
Size: 21296 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20080728/3ecb62c5/attachment-0001.bin>