[libvirt] [PATCH v3 8/8] test: Allow specifying domainsnapshot XML

[Cole Robinson]

On 09/27/2013 02:13 AM, Michal Privoznik wrote:
> On 25.09.2013 21:15, Cole Robinson wrote:
>> The user can pass it as a <test:domainsnapshot> subelement of a <domain>.
>> ---
>>  src/test/test_driver.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 98 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>> index 8a690fd..6b11e9c 100644
>> --- a/src/test/test_driver.c
>> +++ b/src/test/test_driver.c
>> @@ -162,12 +162,24 @@ struct _testDomainNamespaceDef {
>>      int runstate;
>>      bool transient;
>>      bool hasManagedSave;
>> +
>> +    unsigned int num_snap_nodes;
>> +    xmlNodePtr *snap_nodes;
>>  };
>>  
>>  static void
>>  testDomainDefNamespaceFree(void *data)
>>  {
>>      testDomainNamespaceDefPtr nsdata = data;
>> +    size_t i;
>> +
>> +    if (!nsdata)
>> +        return;
>> +
>> +    for (i = 0; i < nsdata->num_snap_nodes; i++)
>> +        xmlFreeNode(nsdata->snap_nodes[i]);
>> +
>> +    VIR_FREE(nsdata->snap_nodes);
>>      VIR_FREE(nsdata);
>>  }
>>  
>> @@ -178,7 +190,9 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED,
>>                              void **data)
>>  {
>>      testDomainNamespaceDefPtr nsdata = NULL;
>> -    int tmp;
>> +    xmlNodePtr *nodes = NULL;
>> +    int tmp, n;
>> +    size_t i;
>>      unsigned int tmpuint;
>>  
>>      if (xmlXPathRegisterNs(ctxt, BAD_CAST "test",
>> @@ -192,6 +206,26 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED,
>>      if (VIR_ALLOC(nsdata) < 0)
>>          return -1;
>>  
>> +    n = virXPathNodeSet("./test:domainsnapshot", ctxt, &nodes);
>> +    if (n < 0)
>> +        goto error;
>> +
>> +    if (n && VIR_ALLOC_N(nsdata->snap_nodes, n) < 0)
>> +        goto error;
>> +
>> +    nsdata->num_snap_nodes = 0;
> 
> This line ^^^ is not needed. VIR_ALLOC (not the one in this context, but
> a few lines above it) fills allocated memory with zeros. So
> num_snap_nodes is zero at this point. For sure.
> 
>> +    for (i = 0; i < n; i++) {
>> +        xmlNodePtr newnode = xmlCopyNode(nodes[i], 1);
>> +        if (!newnode) {
>> +            virReportOOMError();
>> +            goto error;
>> +        }
>> +
>> +        nsdata->snap_nodes[nsdata->num_snap_nodes] = newnode;
>> +        nsdata->num_snap_nodes++;
>> +    }
>> +    VIR_FREE(nodes);
>> +
>>      tmp = virXPathBoolean("boolean(./test:transient)", ctxt);
>>      if (tmp == -1) {
>>          virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid transient"));
>> @@ -237,6 +271,7 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED,
>>      return 0;
>>  
>>  error:
>> +    VIR_FREE(nodes);
>>      testDomainDefNamespaceFree(nsdata);
>>      return -1;
>>  }
>> @@ -934,6 +969,63 @@ error:
>>  }
>>  
>>  static int
>> +testParseDomainSnapshots(testConnPtr privconn,
>> +                         virDomainObjPtr domobj,
>> +                         const char *file,
>> +                         xmlXPathContextPtr ctxt)
>> +{
>> +    size_t i;
>> +    int ret = -1;
>> +    testDomainNamespaceDefPtr nsdata = domobj->def->namespaceData;
>> +    xmlNodePtr *nodes = nsdata->snap_nodes;
>> +
>> +    for (i = 0; i < nsdata->num_snap_nodes; i++) {
>> +        virDomainSnapshotObjPtr snap;
>> +        virDomainSnapshotDefPtr def;
>> +        xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file,
>> +                                                  "domainsnapshot");
>> +        if (!node)
>> +            goto error;
>> +
>> +        def = virDomainSnapshotDefParseNode(ctxt->doc, node,
>> +                                            privconn->caps,
>> +                                            privconn->xmlopt,
>> +                                            1 << VIR_DOMAIN_VIRT_TEST,
>> +                                            VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
>> +                                            VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL |
>> +                                            VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
>> +        if (!def)
>> +            goto error;
>> +
>> +        if (!(snap = virDomainSnapshotAssignDef(domobj->snapshots, def))) {
>> +            virDomainSnapshotDefFree(def);
>> +            goto error;
>> +        }
>> +
>> +        if (def->current) {
>> +            if (domobj->current_snapshot) {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                               _("more than one snapshot claims to be active"));
>> +                goto error;
>> +            }
>> +
>> +            domobj->current_snapshot = snap;
>> +        }
>> +    }
>> +
>> +    if (virDomainSnapshotUpdateRelations(domobj->snapshots) < 0) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Snapshots have inconsistent relations for "
>> +                         "domain %s"), domobj->def->name);
>> +        goto error;
>> +    }
>> +
>> +    ret = 0;
>> +error:
>> +    return ret;
>> +}
>> +
>> +static int
>>  testParseDomains(testConnPtr privconn,
>>                   const char *file,
>>                   xmlXPathContextPtr ctxt)
>> @@ -971,6 +1063,11 @@ testParseDomains(testConnPtr privconn,
>>              goto error;
>>          }
>>  
>> +        if (testParseDomainSnapshots(privconn, obj, file, ctxt) < 0) {
>> +            virObjectUnlock(obj);
>> +            goto error;
>> +        }
>> +
>>          nsdata = def->namespaceData;
>>          obj->persistent = !nsdata->transient;
>>          obj->hasManagedSave = nsdata->hasManagedSave;
>>
> 
> ACK
> 
> Michal
> 

Pushed with the suggested change.

Thanks,
Cole