[libvirt] [PATCH 04/11] qemu: backup: Move deletion of backup images to job termination

Peter Krempa posted 11 patches 6 years, 1 month ago
[libvirt] [PATCH 04/11] qemu: backup: Move deletion of backup images to job termination
Posted by Peter Krempa 6 years, 1 month ago
While qemu is running both locations are identical in semantics, but the
move will allow us to fix the scenario when the VM is destroyed or
crashes where we'd leak the images.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_backup.c   | 24 ++++++++++++++++++++++++
 src/qemu/qemu_blockjob.c | 15 +--------------
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index de4730441b..4ab9a2b17e 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -614,6 +614,7 @@ qemuBackupJobTerminate(virDomainObjPtr vm,

 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    size_t i;

     qemuDomainJobInfoUpdateTime(priv->job.current);

@@ -630,6 +631,29 @@ qemuBackupJobTerminate(virDomainObjPtr vm,

     qemuDomainEventEmitJobCompleted(priv->driver, vm);

+    if (!(priv->job.apiFlags & VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL) &&
+        (priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PULL ||
+         (priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PUSH &&
+          jobstatus != QEMU_DOMAIN_JOB_STATUS_COMPLETED))) {
+
+        g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
+
+        for (i = 0; i < priv->backup->ndisks; i++) {
+            virDomainBackupDiskDefPtr backupdisk = priv->backup->disks + i;
+            uid_t uid;
+            gid_t gid;
+
+            if (!backupdisk->store ||
+                backupdisk->store->type != VIR_STORAGE_TYPE_FILE)
+                continue;
+
+            qemuDomainGetImageIds(cfg, vm, backupdisk->store, NULL, &uid, &gid);
+            if (virFileRemove(backupdisk->store->path, uid, gid) < 0)
+                VIR_WARN("failed to remove scratch file '%s'",
+                         backupdisk->store->path);
+        }
+    }
+
     virDomainBackupDefFree(priv->backup);
     priv->backup = NULL;
     qemuDomainObjEndAsyncJob(priv->driver, vm);
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 498e2a716f..131b53d88d 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1335,9 +1335,6 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver,
                                         unsigned long long progressCurrent,
                                         unsigned long long progressTotal)
 {
-    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-    uid_t uid;
-    gid_t gid;
     g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL;
     g_autoptr(virJSONValue) actions = NULL;

@@ -1369,18 +1366,8 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         return;

-    if (job->data.backup.store) {
+    if (job->data.backup.store)
         qemuDomainStorageSourceAccessRevoke(driver, vm, job->data.backup.store);
-
-        if (job->data.backup.deleteStore &&
-            job->data.backup.store->type == VIR_STORAGE_TYPE_FILE) {
-            qemuDomainGetImageIds(cfg, vm, job->data.backup.store, NULL, &uid, &gid);
-
-            if (virFileRemove(job->data.backup.store->path, uid, gid) < 0)
-                VIR_WARN("failed to remove scratch file '%s'",
-                         job->data.backup.store->path);
-        }
-    }
 }


-- 
2.23.0

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

Re: [libvirt] [PATCH 04/11] qemu: backup: Move deletion of backup images to job termination
Posted by Daniel P. Berrangé 6 years, 1 month ago
On Fri, Dec 20, 2019 at 02:25:22PM +0100, Peter Krempa wrote:
> While qemu is running both locations are identical in semantics, but the
> move will allow us to fix the scenario when the VM is destroyed or
> crashes where we'd leak the images.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_backup.c   | 24 ++++++++++++++++++++++++
>  src/qemu/qemu_blockjob.c | 15 +--------------
>  2 files changed, 25 insertions(+), 14 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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