On 3/27/19 9:01 AM, Ján Tomko wrote:
> On Wed, Mar 27, 2019 at 05:10:40AM -0500, Eric Blake wrote:
>> Add a new file checkpoint_conf.c that performs the translation to and
>> from new XML describing a checkpoint. The code shares a common base
>> class with snapshots, since a checkpoint similarly represents the
>> domain state at a moment in time. Add some basic testing of round trip
>> XML handling through the new code.
>>
>> +static virDomainCheckpointDefPtr
>> +virDomainCheckpointDefParse(xmlXPathContextPtr ctxt,
>> + virCapsPtr caps,
>> + virDomainXMLOptionPtr xmlopt,
>> + bool *current,
>> + unsigned int flags)
>> +{
>> + virDomainCheckpointDefPtr def = NULL;
>> + virDomainCheckpointDefPtr ret = NULL;
>> + xmlNodePtr *nodes = NULL;
>> + size_t i;
>> + int n;
>> + char *creation = NULL;
>> + struct timeval tv;
>> + int active;
>> + char *tmp;
>> +
>> + if (VIR_ALLOC(def) < 0)
>> + goto cleanup;
>> +
>> + gettimeofday(&tv, NULL);
>> +
>
> Eww. I'd expect an XML parsing function to behave the same regardless of
> the time of day.
>
> For domains, we'd put this kind of stuff into a post-parse function.
Ah, life has improved since I wrote snapshots. This is verbatim
copy-and-paste from what snapshots currently do, but I agree that it
would be nicer to first fix snapshots to switch to post-parse functions
and then copy that fixed approach here (it would also make writing tests
for this easier - I was struggling for how to write a meaningful test
that does not just filter out all lines containing a dynamic timestamp -
but if the test can supply its own post-parse hooks, we can provide a
reproducible result).
>> +
>> + if (flags & VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE) {
>> + if (virXPathLongLong("string(./creationTime)", ctxt,
>> + &def->common.creationTime) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("missing creationTime from existing
>> checkpoint"));
>> + goto cleanup;
>> + }
>> +
>> + def->common.parent = virXPathString("string(./parent/name)",
>> ctxt);
>> +
>
>> + if ((tmp = virXPathString("string(./domain/@type)", ctxt))) {
>
> tmp is freed right away and the error reported on else is duplicate with
> the
> one a few lines below.
>
>> + int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
>> + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
>> + xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
>> +
>> + VIR_FREE(tmp);
>> + if (!domainNode) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("missing domain in checkpoint"));
>> + goto cleanup;
>> + }
>> + def->common.dom = virDomainDefParseNode(ctxt->node->doc,
>> domainNode,
>> + caps, xmlopt, NULL,
>> + domainflags);
>> + if (!def->common.dom)
>> + goto cleanup;
>> + } else {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("missing domain in checkpoint redefine"));
>> + goto cleanup;
>> + }
>
Yeah, there's probably a way to write that more succinctly. The snapshot
code is hairier because it DOES permit an omitted <domain> subelement
for back-compat reasons, but I didn't want to copy that here.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
