Re: [libvirt] [PATCH v7 3/4] vbox_tmpl.c: Patch for redefining snapshots
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 :|