Re: [libvirt] [PATCH v8 4/4] vbox_tmpl.c: Add function for undefining snapshot

2014-06-11 Thread John Ferlan 

Another module where Coverity was less than enthusiastic about the
changes - although less issues than the snapshot code...

This one's a bit easier - add a VIR_FREE(disk); to the two places shown
below and you're good.


John

On 05/19/2014 08:47 AM, Yohan BELLEGUIC wrote:
 All snapshots information will be deleted from the vbox XML, but
 differencing disks will be kept so the user will be able to redefine the
 snapshot.
 ---
  src/vbox/vbox_tmpl.c |  464 
 +-
  1 file changed, 463 insertions(+), 1 deletion(-)
 
 diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
 index a7f15d4..eb577f4 100644
 --- a/src/vbox/vbox_tmpl.c
 +++ b/src/vbox/vbox_tmpl.c
 @@ -8364,7 +8364,454 @@ vboxDomainSnapshotDeleteTree(vboxGlobalData *data,
  return ret;
  }
  
 +#if VBOX_API_VERSION = 4002000
 +static int
 +vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
 +{
 +/*
 + * This function will remove the node in the vbox xml corresponding to 
 the snapshot.
 + * It is usually called by vboxDomainSnapshotDelete() with the flag
 + * VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY.
 + * If you want to use it anywhere else, be careful, if the snapshot you 
 want to delete
 + * has children, the result is not granted, they will probably will be 
 deleted in the
 + * xml, but you may have a problem with hard drives.
 + *
 + * If the snapshot which is being deleted is the current one, we will 
 set the current
 + * snapshot of the machine to the parent of this snapshot. Before 
 writing the modified
 + * xml file, we undefine the machine from vbox. After writing the file, 
 we redefine
 + * the machine with the new file.
 + */
 +
 +virDomainPtr dom = snapshot-domain;
 +VBOX_OBJECT_CHECK(dom-conn, int, -1);
 +virDomainSnapshotDefPtr def= NULL;
 +char *defXml = NULL;
 +vboxIID domiid = VBOX_IID_INITIALIZER;
 +nsresult rc;
 +IMachine *machine = NULL;
 +PRUnichar *settingsFilePathUtf16 = NULL;
 +char *settingsFilepath = NULL;
 +virVBoxSnapshotConfMachinePtr snapshotMachineDesc = NULL;
 +int isCurrent = -1;
 +int it = 0;
 +PRUnichar *machineNameUtf16 = NULL;
 +char *machineName = NULL;
 +char *nameTmpUse = NULL;
 +char *machineLocationPath = NULL;
 +PRUint32 aMediaSize = 0;
 +IMedium **aMedia = NULL;
  
 +defXml = vboxDomainSnapshotGetXMLDesc(snapshot, 0);
 +if (!defXml) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Unable to get XML Desc of snapshot));
 +goto cleanup;
 +}
 +def = virDomainSnapshotDefParseString(defXml,
 +  data-caps,
 +  data-xmlopt,
 +  -1,
 +  VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
 +  
 VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
 +if (!def) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Unable to get a virDomainSnapshotDefPtr));
 +goto cleanup;
 +}
 +
 +vboxIIDFromUUID(domiid, dom-uuid);
 +rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine);
 +if (NS_FAILED(rc)) {
 +virReportError(VIR_ERR_NO_DOMAIN, %s,
 +   _(no domain with matching UUID));
 +goto cleanup;
 +}
 +rc = machine-vtbl-GetSettingsFilePath(machine, settingsFilePathUtf16);
 +if (NS_FAILED(rc)) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(cannot get settings file path));
 +goto cleanup;
 +}
 +VBOX_UTF16_TO_UTF8(settingsFilePathUtf16, settingsFilepath);
 +
 +/*Getting the machine name to retrieve the machine location path.*/
 +rc = machine-vtbl-GetName(machine, machineNameUtf16);
 +if (NS_FAILED(rc)) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(cannot get machine name));
 +goto cleanup;
 +}
 +VBOX_UTF16_TO_UTF8(machineNameUtf16, machineName);
 +if (virAsprintf(nameTmpUse, %s.vbox, machineName)  0)
 +goto cleanup;
 +machineLocationPath = virStringReplace(settingsFilepath, nameTmpUse, );
 +if (machineLocationPath == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Unable to get the machine location path));
 +goto cleanup;
 +}
 +snapshotMachineDesc = virVBoxSnapshotConfLoadVboxFile(settingsFilepath, 
 machineLocationPath);
 +if (!snapshotMachineDesc) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(cannot create a vboxSnapshotXmlPtr));
 +goto cleanup;
 +}
 +
 +isCurrent = virVBoxSnapshotConfIsCurrentSnapshot(snapshotMachineDesc, 
 def-name);
 +if (isCurrent  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Unable to know if the snapshot

Re: [libvirt] [PATCH v8 4/4] vbox_tmpl.c: Add function for undefining snapshot

2014-06-10 Thread Daniel P. Berrange 
On Mon, May 19, 2014 at 02:47:33PM +0200, Yohan BELLEGUIC wrote:
 All snapshots information will be deleted from the vbox XML, but
 differencing disks will be kept so the user will be able to redefine the
 snapshot.
 ---
  src/vbox/vbox_tmpl.c |  464 
 +-
  1 file changed, 463 insertions(+), 1 deletion(-)

ACK


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

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

[libvirt] [PATCH v8 4/4] vbox_tmpl.c: Add function for undefining snapshot

2014-05-19 Thread Yohan BELLEGUIC 
All snapshots information will be deleted from the vbox XML, but
differencing disks will be kept so the user will be able to redefine the
snapshot.
---
 src/vbox/vbox_tmpl.c |  464 +-
 1 file changed, 463 insertions(+), 1 deletion(-)

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a7f15d4..eb577f4 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -8364,7 +8364,454 @@ vboxDomainSnapshotDeleteTree(vboxGlobalData *data,
 return ret;
 }
 
+#if VBOX_API_VERSION = 4002000
+static int
+vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
+{
+/*
+ * This function will remove the node in the vbox xml corresponding to the 
snapshot.
+ * It is usually called by vboxDomainSnapshotDelete() with the flag
+ * VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY.
+ * If you want to use it anywhere else, be careful, if the snapshot you 
want to delete
+ * has children, the result is not granted, they will probably will be 
deleted in the
+ * xml, but you may have a problem with hard drives.
+ *
+ * If the snapshot which is being deleted is the current one, we will set 
the current
+ * snapshot of the machine to the parent of this snapshot. Before writing 
the modified
+ * xml file, we undefine the machine from vbox. After writing the file, we 
redefine
+ * the machine with the new file.
+ */
+
+virDomainPtr dom = snapshot-domain;
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+virDomainSnapshotDefPtr def= NULL;
+char *defXml = NULL;
+vboxIID domiid = VBOX_IID_INITIALIZER;
+nsresult rc;
+IMachine *machine = NULL;
+PRUnichar *settingsFilePathUtf16 = NULL;
+char *settingsFilepath = NULL;
+virVBoxSnapshotConfMachinePtr snapshotMachineDesc = NULL;
+int isCurrent = -1;
+int it = 0;
+PRUnichar *machineNameUtf16 = NULL;
+char *machineName = NULL;
+char *nameTmpUse = NULL;
+char *machineLocationPath = NULL;
+PRUint32 aMediaSize = 0;
+IMedium **aMedia = NULL;
 
+defXml = vboxDomainSnapshotGetXMLDesc(snapshot, 0);
+if (!defXml) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Unable to get XML Desc of snapshot));
+goto cleanup;
+}
+def = virDomainSnapshotDefParseString(defXml,
+  data-caps,
+  data-xmlopt,
+  -1,
+  VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
+  VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
+if (!def) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Unable to get a virDomainSnapshotDefPtr));
+goto cleanup;
+}
+
+vboxIIDFromUUID(domiid, dom-uuid);
+rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_NO_DOMAIN, %s,
+   _(no domain with matching UUID));
+goto cleanup;
+}
+rc = machine-vtbl-GetSettingsFilePath(machine, settingsFilePathUtf16);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(cannot get settings file path));
+goto cleanup;
+}
+VBOX_UTF16_TO_UTF8(settingsFilePathUtf16, settingsFilepath);
+
+/*Getting the machine name to retrieve the machine location path.*/
+rc = machine-vtbl-GetName(machine, machineNameUtf16);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(cannot get machine name));
+goto cleanup;
+}
+VBOX_UTF16_TO_UTF8(machineNameUtf16, machineName);
+if (virAsprintf(nameTmpUse, %s.vbox, machineName)  0)
+goto cleanup;
+machineLocationPath = virStringReplace(settingsFilepath, nameTmpUse, );
+if (machineLocationPath == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Unable to get the machine location path));
+goto cleanup;
+}
+snapshotMachineDesc = virVBoxSnapshotConfLoadVboxFile(settingsFilepath, 
machineLocationPath);
+if (!snapshotMachineDesc) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(cannot create a vboxSnapshotXmlPtr));
+goto cleanup;
+}
+
+isCurrent = virVBoxSnapshotConfIsCurrentSnapshot(snapshotMachineDesc, 
def-name);
+if (isCurrent  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Unable to know if the snapshot is the current 
snapshot));
+goto cleanup;
+}
+if (isCurrent) {
+/*
+ * If the snapshot is the current snapshot, it means that the machine 
has read-write
+ * disks. The first thing to do is to manipulate VirtualBox API to 
create
+ * differential read-write disks if the parent snapshot is not null.
+ */
+if (def-parent != NULL) {
+