When blockcommit finishes successfully, one of the
qemuBlockJobProcessEventCompletedCommit() and
qemuBlockJobProcessEventCompletedActiveCommit() event handlers is called.
This is where the delete flag (stored in qemuBlockJobCommitData since the
previous commit) can actually be used to delete the committed snapshot
images if requested.

Signed-off-by: Pavel Mores <pmores@redhat.com>
---
 src/qemu/qemu_blockjob.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 7d94a6ce38..dae1fa44c8 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -965,6 +965,31 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
 }
 
 
+/**
+ * Helper for qemuBlockJobProcessEventCompletedCommit() and
+ * qemuBlockJobProcessEventCompletedActiveCommit().  Relies on adjustments
+ * these functions perform on the 'backingStore' chain to function correctly.
+ *
+ * TODO look into removing backing store for non-local snapshots too
+ */
+static void
+qemuBlockJobUnlinkCommittedStorage(virStorageSourcePtr top)
+{
+    virStorageSourcePtr p = top;
+    const size_t errmsg_len = 128;
+    char errmsg[errmsg_len];
+
+    for (; p != NULL; p = p->backingStore) {
+        if (virStorageSourceIsLocalStorage(p))
+            if (unlink(p->path) < 0) {
+                char *dummy = strerror_r(errno, errmsg, errmsg_len);
+                (void)dummy;
+                VIR_WARN("Unable to remove snapshot image file %s (%s)",
+                         p->path, errmsg);
+            }
+    }
+}
+
 /**
  * qemuBlockJobProcessEventCompletedCommit:
  * @driver: qemu driver object
@@ -1031,6 +1056,9 @@ qemuBlockJobProcessEventCompletedCommit(virQEMUDriverPtr driver,
     job->data.commit.topparent->backingStore = job->data.commit.base;
 
     qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top);
+
+    if (job->data.commit.deleteCommittedImages)
+        qemuBlockJobUnlinkCommittedStorage(job->data.commit.top);
     virObjectUnref(job->data.commit.top);
     job->data.commit.top = NULL;
 
@@ -1121,6 +1149,9 @@ qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver,
     job->disk->src->readonly = job->data.commit.top->readonly;
 
     qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top);
+
+    if (job->data.commit.deleteCommittedImages)
+        qemuBlockJobUnlinkCommittedStorage(job->data.commit.top);
     virObjectUnref(job->data.commit.top);
     job->data.commit.top = NULL;
     /* the mirror element does not serve functional purpose for the commit job */
-- 
2.21.0

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

On Wed, Nov 20, 2019 at 10:55:25AM +0100, Pavel Mores wrote:
> When blockcommit finishes successfully, one of the
> qemuBlockJobProcessEventCompletedCommit() and
> qemuBlockJobProcessEventCompletedActiveCommit() event handlers is called.
> This is where the delete flag (stored in qemuBlockJobCommitData since the
> previous commit) can actually be used to delete the committed snapshot
> images if requested.
> 
> Signed-off-by: Pavel Mores <pmores@redhat.com>
> ---
>  src/qemu/qemu_blockjob.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index 7d94a6ce38..dae1fa44c8 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -965,6 +965,31 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr driver,
>  }
>  
>  
> +/**
> + * Helper for qemuBlockJobProcessEventCompletedCommit() and
> + * qemuBlockJobProcessEventCompletedActiveCommit().  Relies on adjustments
> + * these functions perform on the 'backingStore' chain to function correctly.
> + *
> + * TODO look into removing backing store for non-local snapshots too
> + */
> +static void
> +qemuBlockJobUnlinkCommittedStorage(virStorageSourcePtr top)
> +{
> +    virStorageSourcePtr p = top;
> +    const size_t errmsg_len = 128;
> +    char errmsg[errmsg_len];
> +
> +    for (; p != NULL; p = p->backingStore) {
> +        if (virStorageSourceIsLocalStorage(p))
> +            if (unlink(p->path) < 0) {
> +                char *dummy = strerror_r(errno, errmsg, errmsg_len);
> +                (void)dummy;

Sorry, just realised that a non-standard version of strerror_r() is used here
where the return value is actually useful.  I'll fix this is in v2.

> +                VIR_WARN("Unable to remove snapshot image file %s (%s)",
> +                         p->path, errmsg);
> +            }
> +    }
> +}
> +
>  /**
>   * qemuBlockJobProcessEventCompletedCommit:
>   * @driver: qemu driver object
> @@ -1031,6 +1056,9 @@ qemuBlockJobProcessEventCompletedCommit(virQEMUDriverPtr driver,
>      job->data.commit.topparent->backingStore = job->data.commit.base;
>  
>      qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top);
> +
> +    if (job->data.commit.deleteCommittedImages)
> +        qemuBlockJobUnlinkCommittedStorage(job->data.commit.top);
>      virObjectUnref(job->data.commit.top);
>      job->data.commit.top = NULL;
>  
> @@ -1121,6 +1149,9 @@ qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver,
>      job->disk->src->readonly = job->data.commit.top->readonly;
>  
>      qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top);
> +
> +    if (job->data.commit.deleteCommittedImages)
> +        qemuBlockJobUnlinkCommittedStorage(job->data.commit.top);
>      virObjectUnref(job->data.commit.top);
>      job->data.commit.top = NULL;
>      /* the mirror element does not serve functional purpose for the commit job */
> -- 
> 2.21.0
> 

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