[libvirt] [PATCH 2/3] snapshot: save domain description with snapshot

[Eric Blake]

On 04/12/2011 12:16 AM, Philipp Hahn wrote:
> Save the domain description with the XML snapshot data.
> TODOs:
> - XML file is no longer nicely indented

Cosmetic, and can be fixed later.

> - Fix esx driver
> - Fix vbox driver

Do these need to save domain xml state in the first place?  They aren't 
using libvirt to track domain state in the first time, but call out to 
the hypervisor for everything.  And if the hypervisor is already doing a 
good job of reverting across configuration changes, then it doesn't hurt 
if they continue to use just <domain>/<uuid> instead of full <domain> in 
the snapshot output that libvirt generates on virDomainSnapshotGetXMLDesc.

> @@ -8661,6 +8663,14 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
>       else
>           def->creationTime = tv.tv_sec;
>
> +    xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
> +    if (domainNode == NULL) {
> +        virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                _("missing domain from existing snapshot"));
> +        def->dom = NULL;
> +    } else
> +        def->dom = virDomainDefParseNode(caps, xml, domainNode, VIR_DOMAIN_XML_INACTIVE);

For virDomainSnapshotCreateXML, <domain> should be ignored (unless I get 
around to implementing my RFC of VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE); 
it is normally an output-only field, so I'm not sure this hunk is right.
> @@ -8694,9 +8705,17 @@ char *virDomainSnapshotDefFormat(char *domain_uuid,
>       }
>       virBufferVSprintf(&buf, "<creationTime>%ld</creationTime>\n",
>                         def->creationTime);
> -    virBufferAddLit(&buf, "<domain>\n");
> -    virBufferVSprintf(&buf, "<uuid>%s</uuid>\n", domain_uuid);
> -    virBufferAddLit(&buf, "</domain>\n");
> +    if (def->dom != NULL) {
> +        xml = virDomainDefFormat(def->dom, VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE);

Security hole.  You cannot blindly add VIR_DOMAIN_XML_SECURE if this is 
destined to external output, rather, it has to be passed in from the 
user's flags, and libvirt.c has to validate that 
virDomainSnapshotGetXMLDesc rejects the flag on read-only connections.

> +    }
> +    if (xml != NULL) {
> +        virBufferVSprintf(&buf, "  %s", xml);

Rather than printing the xml in place, we have to touch up the domain 
formatter to take a variable indentation; it's a lot of refactoring just 
for a pretty result (worth it in the long run, but not a show-stopper to 
getting this feature in first).

I'm seeing how easy this is to rebase, but I'll certainly be using 
something similar to this.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org