On Fri, Apr 18, 2014 at 11:51:33AM +0200, Yohan BELLEGUIC wrote:
The machine is unregistered and its vbox XML file is changed in order to add snapshot information. The machine is then registered with the snapshot to redefine. --- src/vbox/vbox_tmpl.c | 949 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 942 insertions(+), 7 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index ac000cf..f8667f6 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -38,12 +38,12 @@ #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> -#include <libxml/xmlwriter.h>
Ah, removing the bogus include I mentioned in patch 1. Can just squash this change into the first patch instead.
+static int +vboxSnapshotRedefine(virDomainPtr dom, + virDomainSnapshotDefPtr def, + bool isCurrent) +{
+ /* + * Now we have done this swap, we remove the snapshot xml file from the + * current machine location. + */ + if (remove(currentSnapshotXmlFilePath) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to delete file %s"), currentSnapshotXmlFilePath); + goto cleanup; + }
Small preference to use 'unlink' instead of remove, since we know this is a plain file. Also use virReportSystemError(errno, _("....")...) since we have an errno we can usefully provide here.
+ /* + * We save the snapshot xml file to retrieve the real read-write disk during the + * next define. This file is saved as "'machineLocation'/snapshot-'uuid'.xml" + */ + VIR_FREE(currentSnapshotXmlFilePath); + if (virAsprintf(¤tSnapshotXmlFilePath, "%s%s.xml", machineLocationPath, snapshotMachineDesc->currentSnapshot) < 0) + goto cleanup; + char *snapshotContent = virDomainSnapshotDefFormat(NULL, def, VIR_DOMAIN_XML_SECURE, 0);
Need to check for snapshotContent == NULL here.
+ xmlDocPtr newXml = virXMLParse(NULL, snapshotContent, NULL);
Or this will cause a crash on OOM
+ VIR_FREE(snapshotContent); + if (newXml && xmlSaveFile(currentSnapshotXmlFilePath, newXml) < 0) {
This whole sequence of functions is wierd. You have 'snapshotContent' which is a string containing an XML document. You then parse this to get an xmlDocPtr. You then call xmlSaveFile which turns xmlDocPtr back into a string and writes it to a file. IMHO you should just use virFileWriteStr(snapshotContent) and remove this parsing
+ virReportError(VIR_ERR_XML_ERROR, "%s", + _("Unable to save new snapshot xml file")); + goto cleanup; + } + }
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|