Re: [libvirt] [PATCHv6 4/5] vbox_tmpl.c: Patch for redefining snapshots

2014-02-19 Thread Daniel P. Berrange
On Thu, Jan 23, 2014 at 10:28:32AM +0100, Manuel VIVES wrote:
 The snapshots are saved in xml files, and then can be redefined
 ---
  src/vbox/vbox_tmpl.c | 1083 
 +-
  1 file changed, 1075 insertions(+), 8 deletions(-)

My comment here is that you ought to separate the XML handling
out from the driver. Create a new file vbox_snapshot_conf.{c,h}
In the header define a struct containing the data to be saved
in the XML. Then provide an API for writing the struct to an
XML doc, and another API for reading the XML to create a struct.
Then the vbox_tmpl.c would call those methods.

Also, a bit more info about the XML format in the commit message
would be useful. In particular i'm unclear if this XML document
is something you're inventing specifically for use by libvirt
here, or is a format the VirtualBox itself has defined.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv6 4/5] vbox_tmpl.c: Patch for redefining snapshots

2014-01-23 Thread Manuel VIVES
The snapshots are saved in xml files, and then can be redefined
---
 src/vbox/vbox_tmpl.c | 1083 +-
 1 file changed, 1075 insertions(+), 8 deletions(-)

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 0d7809a..50b541d 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -61,6 +61,7 @@
 #include virstring.h
 #include virtime.h
 #include virutil.h
+#include dirname.h
 
 /* This one changes from version to version. */
 #if VBOX_API_VERSION == 2002000
@@ -280,9 +281,19 @@ static vboxGlobalData *g_pVBoxGlobalData = NULL;
 
 #endif /* VBOX_API_VERSION = 400 */
 
+/*This error is a bit specific
+ *In the VBOX API it is named E_ACCESSDENIED
+ *It is returned when the called object is not ready. In
+ *particular when we do any call on a disk which has been closed
+*/
+#define VBOX_E_ACCESSDENIED 0x80070005
 static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml);
 static int vboxDomainCreate(virDomainPtr dom);
 static int vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags);
+static virStorageVolPtr vboxStorageVolLookupByPath(virConnectPtr conn, const 
char *path);
+static int vboxStorageDeleteOrClose(virStorageVolPtr vol,
+unsigned int flags,
+unsigned int flagDeleteOrClose);
 
 static void vboxDriverLock(vboxGlobalData *data) {
 virMutexLock(data-lock);
@@ -5956,6 +5967,1055 @@ cleanup:
 return snapshot;
 }
 
+#if VBOX_API_VERSION =4002000
+static void
+vboxSnapshotXmlRetrieveSnapshotNodeByName(xmlNodePtr a_node,
+   const char *name,
+   xmlNodePtr *snap_node)
+{
+xmlNodePtr cur_node = NULL;
+char *xmlName;
+for (cur_node = a_node; cur_node; cur_node = cur_node-next) {
+if (cur_node-type == XML_ELEMENT_NODE) {
+if (!xmlStrcmp(cur_node-name, (const xmlChar *) Snapshot)) {
+if ((xmlName = virXMLPropString(cur_node, name))) {
+if (STREQ(xmlName, name)) {
+VIR_FREE(xmlName);
+*snap_node = cur_node;
+return;
+}
+VIR_FREE(xmlName);
+}
+}
+}
+if (cur_node-children)
+vboxSnapshotXmlRetrieveSnapshotNodeByName(cur_node-children, name,
+  snap_node);
+}
+}
+
+static int
+vboxDetachAndCloseDisks(virDomainPtr dom,
+IMedium *disk)
+{
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+nsresult rc;
+PRUnichar *location = NULL;
+vboxArray childrenDiskArray = VBOX_ARRAY_INITIALIZER;
+virStorageVolPtr volPtr = NULL;
+char *location_utf8 = NULL;
+PRUint32 dummyState = 0;
+size_t i = 0;
+if (disk == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(null pointer to disk));
+goto cleanup;
+}
+rc = disk-vtbl-GetLocation(disk, location);
+if (rc == VBOX_E_ACCESSDENIED) {
+ret = 0;
+VIR_DEBUG(Disk already closed);
+goto cleanup;
+}
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(cannot get disk location));
+goto cleanup;
+}
+rc = vboxArrayGet(childrenDiskArray, disk, disk-vtbl-GetChildren);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(cannot get children disks));
+goto cleanup;
+}
+for (i = 0; i  childrenDiskArray.count; ++i) {
+IMedium *childDisk = childrenDiskArray.items[i];
+if (childDisk) {
+vboxDetachAndCloseDisks(dom, childDisk);
+}
+}
+rc = disk-vtbl-RefreshState(disk, dummyState);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(cannot refresh state));
+goto cleanup;
+}
+VBOX_UTF16_TO_UTF8(location, location_utf8);
+volPtr = vboxStorageVolLookupByPath(dom-conn, location_utf8);
+
+if (volPtr) {
+VIR_DEBUG(Closing %s, location_utf8);
+if (vboxStorageDeleteOrClose(volPtr, 0, VBOX_STORAGE_CLOSE_FLAG)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(error while closing disk));
+goto cleanup;
+}
+ret = 0;
+}
+cleanup:
+VBOX_UTF8_FREE(location_utf8);
+VBOX_UTF16_FREE(location);
+vboxArrayRelease(childrenDiskArray);
+return ret;
+}
+
+static void
+vboxSnapshotXmlAddChild(xmlNodePtr parent,
+xmlNodePtr child)
+{
+/*Used in order to add child without writing the stuff concerning xml 
namespaces*/
+xmlBufferPtr tmpBuf = xmlBufferCreate();
+char *tmpString = NULL;
+xmlNodePtr tmpNode = NULL;
+xmlNodeDump(tmpBuf, parent-doc, child, 0, 0);
+if (VIR_STRDUP(tmpString, (char