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

[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(snaps

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

[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

[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 nul