]> git.proxmox.com Git - pve-qemu.git/commitdiff
avoid using QemuRecMutex inside coroutines, QemuMutex to lock outside
authorThomas Lamprecht <t.lamprecht@proxmox.com>
Fri, 17 Apr 2020 06:17:59 +0000 (08:17 +0200)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Fri, 17 Apr 2020 07:09:12 +0000 (09:09 +0200)
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
debian/patches/pve/0041-PVE-Backup-avoid-use-QemuRecMutex-inside-coroutines.patch [new file with mode: 0644]
debian/patches/pve/0041-PVE-Use-non-recursive-locks.patch [deleted file]
debian/patches/pve/0042-PVE-Backup-use-QemuMutex-instead-of-QemuRecMutex.patch [new file with mode: 0644]
debian/patches/series

diff --git a/debian/patches/pve/0041-PVE-Backup-avoid-use-QemuRecMutex-inside-coroutines.patch b/debian/patches/pve/0041-PVE-Backup-avoid-use-QemuRecMutex-inside-coroutines.patch
new file mode 100644 (file)
index 0000000..00439d9
--- /dev/null
@@ -0,0 +1,211 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Dietmar Maurer <dietmar@proxmox.com>
+Date: Fri, 17 Apr 2020 08:57:47 +0200
+Subject: [PATCH] PVE Backup: avoid use QemuRecMutex inside coroutines
+
+---
+ pve-backup.c | 59 +++++++++++++++++++++++++++++++++-------------------
+ 1 file changed, 38 insertions(+), 21 deletions(-)
+
+diff --git a/pve-backup.c b/pve-backup.c
+index 169f0c68d0..dddf430399 100644
+--- a/pve-backup.c
++++ b/pve-backup.c
+@@ -11,6 +11,23 @@
+ /* PVE backup state and related function */
++/*
++ * Note: A resume from a qemu_coroutine_yield can happen in a different thread,
++ * so you may not use normal mutexes within coroutines:
++ *
++ * ---bad-example---
++ * qemu_rec_mutex_lock(lock)
++ * ...
++ * qemu_coroutine_yield() // wait for something
++ * // we are now inside a different thread
++ * qemu_rec_mutex_unlock(lock) // Crash - wrong thread!!
++ * ---end-bad-example--
++ *
++ * ==> Always use CoMutext inside coroutines.
++ * ==> Never acquire/release AioContext withing coroutines (because that use QemuRecMutex)
++ *
++ */
++
+ static struct PVEBackupState {
+     struct {
+         // Everithing accessed from qmp_backup_query command is protected using lock
+@@ -30,12 +47,14 @@ static struct PVEBackupState {
+     ProxmoxBackupHandle *pbs;
+     GList *di_list;
+     QemuRecMutex backup_mutex;
++    CoMutex dump_callback_mutex;
+ } backup_state;
+ static void pvebackup_init(void)
+ {
+     qemu_rec_mutex_init(&backup_state.stat.lock);
+     qemu_rec_mutex_init(&backup_state.backup_mutex);
++    qemu_co_mutex_init(&backup_state.dump_callback_mutex);
+ }
+ // initialize PVEBackupState at startup
+@@ -114,16 +133,16 @@ pvebackup_co_dump_pbs_cb(
+     Error *local_err = NULL;
+     int pbs_res = -1;
+-    qemu_rec_mutex_lock(&backup_state.backup_mutex);
++    qemu_co_mutex_lock(&backup_state.dump_callback_mutex);
+     // avoid deadlock if job is cancelled
+     if (pvebackup_error_or_canceled()) {
+-        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++        qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
+         return -1;
+     }
+     pbs_res = proxmox_backup_co_write_data(backup_state.pbs, di->dev_id, buf, start, size, &local_err);
+-    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++    qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
+     if (pbs_res < 0) {
+         pvebackup_propagate_error(local_err);
+@@ -149,7 +168,6 @@ pvebackup_co_dump_vma_cb(
+     const unsigned char *buf = pbuf;
+     PVEBackupDevInfo *di = opaque;
+-
+     int ret = -1;
+     assert(backup_state.vmaw);
+@@ -167,16 +185,16 @@ pvebackup_co_dump_vma_cb(
+     }
+     while (remaining > 0) {
+-        qemu_rec_mutex_lock(&backup_state.backup_mutex);
++        qemu_co_mutex_lock(&backup_state.dump_callback_mutex);
+         // avoid deadlock if job is cancelled
+         if (pvebackup_error_or_canceled()) {
+-            qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++            qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
+             return -1;
+         }
+         size_t zero_bytes = 0;
+         ret = vma_writer_write(backup_state.vmaw, di->dev_id, cluster_num, buf, &zero_bytes);
+-        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++        qemu_co_mutex_unlock(&backup_state.dump_callback_mutex);
+         ++cluster_num;
+         if (buf) {
+@@ -203,12 +221,11 @@ pvebackup_co_dump_vma_cb(
+     return size;
+ }
++// assumes the caller holds backup_mutex
+ static void coroutine_fn pvebackup_co_cleanup(void *unused)
+ {
+     assert(qemu_in_coroutine());
+-    qemu_rec_mutex_lock(&backup_state.backup_mutex);
+-
+     qemu_rec_mutex_lock(&backup_state.stat.lock);
+     backup_state.stat.end_time = time(NULL);
+     qemu_rec_mutex_unlock(&backup_state.stat.lock);
+@@ -239,9 +256,9 @@ static void coroutine_fn pvebackup_co_cleanup(void *unused)
+     g_list_free(backup_state.di_list);
+     backup_state.di_list = NULL;
+-    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
+ }
++// assumes the caller holds backup_mutex
+ static void coroutine_fn pvebackup_complete_stream(void *opaque)
+ {
+     PVEBackupDevInfo *di = opaque;
+@@ -295,6 +312,8 @@ static void pvebackup_complete_cb(void *opaque, int ret)
+ static void pvebackup_cancel(void)
+ {
++    assert(!qemu_in_coroutine());
++
+     Error *cancel_err = NULL;
+     error_setg(&cancel_err, "backup canceled");
+     pvebackup_propagate_error(cancel_err);
+@@ -348,6 +367,7 @@ void qmp_backup_cancel(Error **errp)
+     pvebackup_cancel();
+ }
++// assumes the caller holds backup_mutex
+ static int coroutine_fn pvebackup_co_add_config(
+     const char *file,
+     const char *name,
+@@ -431,9 +451,9 @@ static void pvebackup_run_next_job(void)
+         }
+     }
+-    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
+-
+     block_on_coroutine_fn(pvebackup_co_cleanup, NULL); // no more jobs, run cleanup
++
++    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
+ }
+ static bool create_backup_jobs(void) {
+@@ -520,6 +540,7 @@ typedef struct QmpBackupTask {
+     UuidInfo *result;
+ } QmpBackupTask;
++// assumes the caller holds backup_mutex
+ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+ {
+     assert(qemu_in_coroutine());
+@@ -543,11 +564,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+     const char *config_name = "qemu-server.conf";
+     const char *firewall_name = "qemu-server.fw";
+-    qemu_rec_mutex_lock(&backup_state.backup_mutex);
+-
+     if (backup_state.di_list) {
+-        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
+-        error_set(task->errp, ERROR_CLASS_GENERIC_ERROR,
++         error_set(task->errp, ERROR_CLASS_GENERIC_ERROR,
+                   "previous backup not finished");
+         return;
+     }
+@@ -792,8 +810,6 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+     backup_state.di_list = di_list;
+-    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
+-
+     uuid_info = g_malloc0(sizeof(*uuid_info));
+     uuid_info->UUID = uuid_str;
+@@ -836,8 +852,6 @@ err:
+         rmdir(backup_dir);
+     }
+-    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
+-
+     task->result = NULL;
+     return;
+ }
+@@ -881,13 +895,16 @@ UuidInfo *qmp_backup(
+         .errp = errp,
+     };
++    qemu_rec_mutex_lock(&backup_state.backup_mutex);
++
+     block_on_coroutine_fn(pvebackup_co_prepare, &task);
+     if (*errp == NULL) {
+-        qemu_rec_mutex_lock(&backup_state.backup_mutex);
+         create_backup_jobs();
+         qemu_rec_mutex_unlock(&backup_state.backup_mutex);
+         pvebackup_run_next_job();
++    } else {
++        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
+     }
+     return task.result;
diff --git a/debian/patches/pve/0041-PVE-Use-non-recursive-locks.patch b/debian/patches/pve/0041-PVE-Use-non-recursive-locks.patch
deleted file mode 100644 (file)
index 774f4d4..0000000
+++ /dev/null
@@ -1,403 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Stefan Reiter <s.reiter@proxmox.com>
-Date: Thu, 16 Apr 2020 11:40:16 +0200
-Subject: [PATCH] PVE: Use non-recursive locks
-
-Release the lock on qemu_coroutine_yield, so coroutines don't deadlock.
-
-Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
----
- pve-backup.c | 82 +++++++++++++++++++++++++++-------------------------
- vma-writer.c | 28 ++++++++++++++----
- vma.h        |  1 +
- 3 files changed, 66 insertions(+), 45 deletions(-)
-
-diff --git a/pve-backup.c b/pve-backup.c
-index 169f0c68d0..46a3d6f995 100644
---- a/pve-backup.c
-+++ b/pve-backup.c
-@@ -14,7 +14,7 @@
- static struct PVEBackupState {
-     struct {
-         // Everithing accessed from qmp_backup_query command is protected using lock
--        QemuRecMutex lock;
-+        QemuMutex lock;
-         Error *error;
-         time_t start_time;
-         time_t end_time;
-@@ -29,13 +29,13 @@ static struct PVEBackupState {
-     VmaWriter *vmaw;
-     ProxmoxBackupHandle *pbs;
-     GList *di_list;
--    QemuRecMutex backup_mutex;
-+    QemuMutex backup_mutex;
- } backup_state;
- static void pvebackup_init(void)
- {
--    qemu_rec_mutex_init(&backup_state.stat.lock);
--    qemu_rec_mutex_init(&backup_state.backup_mutex);
-+    qemu_mutex_init(&backup_state.stat.lock);
-+    qemu_mutex_init(&backup_state.backup_mutex);
- }
- // initialize PVEBackupState at startup
-@@ -72,26 +72,26 @@ lookup_active_block_job(PVEBackupDevInfo *di)
- static void pvebackup_propagate_error(Error *err)
- {
--    qemu_rec_mutex_lock(&backup_state.stat.lock);
-+    qemu_mutex_lock(&backup_state.stat.lock);
-     error_propagate(&backup_state.stat.error, err);
--    qemu_rec_mutex_unlock(&backup_state.stat.lock);
-+    qemu_mutex_unlock(&backup_state.stat.lock);
- }
- static bool pvebackup_error_or_canceled(void)
- {
--    qemu_rec_mutex_lock(&backup_state.stat.lock);
-+    qemu_mutex_lock(&backup_state.stat.lock);
-     bool error_or_canceled = !!backup_state.stat.error;
--    qemu_rec_mutex_unlock(&backup_state.stat.lock);
-+    qemu_mutex_unlock(&backup_state.stat.lock);
-     return error_or_canceled;
- }
- static void pvebackup_add_transfered_bytes(size_t transferred, size_t zero_bytes)
- {
--    qemu_rec_mutex_lock(&backup_state.stat.lock);
-+    qemu_mutex_lock(&backup_state.stat.lock);
-     backup_state.stat.zero_bytes += zero_bytes;
-     backup_state.stat.transferred += transferred;
--    qemu_rec_mutex_unlock(&backup_state.stat.lock);
-+    qemu_mutex_unlock(&backup_state.stat.lock);
- }
- // This may get called from multiple coroutines in multiple io-threads
-@@ -114,16 +114,16 @@ pvebackup_co_dump_pbs_cb(
-     Error *local_err = NULL;
-     int pbs_res = -1;
--    qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+    qemu_mutex_lock(&backup_state.backup_mutex);
-     // avoid deadlock if job is cancelled
-     if (pvebackup_error_or_canceled()) {
--        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+        qemu_mutex_unlock(&backup_state.backup_mutex);
-         return -1;
-     }
-     pbs_res = proxmox_backup_co_write_data(backup_state.pbs, di->dev_id, buf, start, size, &local_err);
--    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+    qemu_mutex_unlock(&backup_state.backup_mutex);
-     if (pbs_res < 0) {
-         pvebackup_propagate_error(local_err);
-@@ -167,16 +167,16 @@ pvebackup_co_dump_vma_cb(
-     }
-     while (remaining > 0) {
--        qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+        qemu_mutex_lock(&backup_state.backup_mutex);
-         // avoid deadlock if job is cancelled
-         if (pvebackup_error_or_canceled()) {
--            qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+            qemu_mutex_unlock(&backup_state.backup_mutex);
-             return -1;
-         }
-         size_t zero_bytes = 0;
-         ret = vma_writer_write(backup_state.vmaw, di->dev_id, cluster_num, buf, &zero_bytes);
--        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+        qemu_mutex_unlock(&backup_state.backup_mutex);
-         ++cluster_num;
-         if (buf) {
-@@ -207,11 +207,11 @@ static void coroutine_fn pvebackup_co_cleanup(void *unused)
- {
-     assert(qemu_in_coroutine());
--    qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+    qemu_mutex_lock(&backup_state.backup_mutex);
--    qemu_rec_mutex_lock(&backup_state.stat.lock);
-+    qemu_mutex_lock(&backup_state.stat.lock);
-     backup_state.stat.end_time = time(NULL);
--    qemu_rec_mutex_unlock(&backup_state.stat.lock);
-+    qemu_mutex_unlock(&backup_state.stat.lock);
-     if (backup_state.vmaw) {
-         Error *local_err = NULL;
-@@ -239,7 +239,7 @@ static void coroutine_fn pvebackup_co_cleanup(void *unused)
-     g_list_free(backup_state.di_list);
-     backup_state.di_list = NULL;
--    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+    qemu_mutex_unlock(&backup_state.backup_mutex);
- }
- static void coroutine_fn pvebackup_complete_stream(void *opaque)
-@@ -267,7 +267,7 @@ static void pvebackup_complete_cb(void *opaque, int ret)
-     PVEBackupDevInfo *di = opaque;
--    qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+    qemu_mutex_lock(&backup_state.backup_mutex);
-     di->completed = true;
-@@ -288,7 +288,7 @@ static void pvebackup_complete_cb(void *opaque, int ret)
-     g_free(di);
--    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+    qemu_mutex_unlock(&backup_state.backup_mutex);
-     pvebackup_run_next_job();
- }
-@@ -299,7 +299,7 @@ static void pvebackup_cancel(void)
-     error_setg(&cancel_err, "backup canceled");
-     pvebackup_propagate_error(cancel_err);
--    qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+    qemu_mutex_lock(&backup_state.backup_mutex);
-     if (backup_state.vmaw) {
-         /* make sure vma writer does not block anymore */
-@@ -310,13 +310,13 @@ static void pvebackup_cancel(void)
-         proxmox_backup_abort(backup_state.pbs, "backup canceled");
-     }
--    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+    qemu_mutex_unlock(&backup_state.backup_mutex);
-     for(;;) {
-         BlockJob *next_job = NULL;
--        qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+        qemu_mutex_lock(&backup_state.backup_mutex);
-         GList *l = backup_state.di_list;
-         while (l) {
-@@ -330,7 +330,7 @@ static void pvebackup_cancel(void)
-             }
-         }
--        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+        qemu_mutex_unlock(&backup_state.backup_mutex);
-         if (next_job) {
-             AioContext *aio_context = next_job->job.aio_context;
-@@ -403,7 +403,7 @@ static void pvebackup_run_next_job(void)
- {
-     assert(!qemu_in_coroutine());
--    qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+    qemu_mutex_lock(&backup_state.backup_mutex);
-     GList *l = backup_state.di_list;
-     while (l) {
-@@ -413,7 +413,7 @@ static void pvebackup_run_next_job(void)
-         BlockJob *job = lookup_active_block_job(di);
-         if (job) {
--            qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+            qemu_mutex_unlock(&backup_state.backup_mutex);
-             AioContext *aio_context = job->job.aio_context;
-             aio_context_acquire(aio_context);
-@@ -431,7 +431,7 @@ static void pvebackup_run_next_job(void)
-         }
-     }
--    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+    qemu_mutex_unlock(&backup_state.backup_mutex);
-     block_on_coroutine_fn(pvebackup_co_cleanup, NULL); // no more jobs, run cleanup
- }
-@@ -543,10 +543,10 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
-     const char *config_name = "qemu-server.conf";
-     const char *firewall_name = "qemu-server.fw";
--    qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+    qemu_mutex_lock(&backup_state.backup_mutex);
-     if (backup_state.di_list) {
--        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+        qemu_mutex_unlock(&backup_state.backup_mutex);
-         error_set(task->errp, ERROR_CLASS_GENERIC_ERROR,
-                   "previous backup not finished");
-         return;
-@@ -689,6 +689,8 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
-             goto err;
-         }
-+      vma_writer_set_external_lock(vmaw, &backup_state.backup_mutex);
-+
-         /* register all devices for vma writer */
-         l = di_list;
-         while (l) {
-@@ -760,7 +762,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
-     }
-     /* initialize global backup_state now */
--    qemu_rec_mutex_lock(&backup_state.stat.lock);
-+    qemu_mutex_lock(&backup_state.stat.lock);
-     if (backup_state.stat.error) {
-         error_free(backup_state.stat.error);
-@@ -783,7 +785,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
-     backup_state.stat.transferred = 0;
-     backup_state.stat.zero_bytes = 0;
--    qemu_rec_mutex_unlock(&backup_state.stat.lock);
-+    qemu_mutex_unlock(&backup_state.stat.lock);
-     backup_state.speed = (task->has_speed && task->speed > 0) ? task->speed : 0;
-@@ -792,7 +794,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
-     backup_state.di_list = di_list;
--    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+    qemu_mutex_unlock(&backup_state.backup_mutex);
-     uuid_info = g_malloc0(sizeof(*uuid_info));
-     uuid_info->UUID = uuid_str;
-@@ -836,7 +838,7 @@ err:
-         rmdir(backup_dir);
-     }
--    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+    qemu_mutex_unlock(&backup_state.backup_mutex);
-     task->result = NULL;
-     return;
-@@ -884,9 +886,9 @@ UuidInfo *qmp_backup(
-     block_on_coroutine_fn(pvebackup_co_prepare, &task);
-     if (*errp == NULL) {
--        qemu_rec_mutex_lock(&backup_state.backup_mutex);
-+        qemu_mutex_lock(&backup_state.backup_mutex);
-         create_backup_jobs();
--        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
-+        qemu_mutex_unlock(&backup_state.backup_mutex);
-         pvebackup_run_next_job();
-     }
-@@ -897,11 +899,11 @@ BackupStatus *qmp_query_backup(Error **errp)
- {
-     BackupStatus *info = g_malloc0(sizeof(*info));
--    qemu_rec_mutex_lock(&backup_state.stat.lock);
-+    qemu_mutex_lock(&backup_state.stat.lock);
-     if (!backup_state.stat.start_time) {
-         /* not started, return {} */
--        qemu_rec_mutex_unlock(&backup_state.stat.lock);
-+        qemu_mutex_unlock(&backup_state.stat.lock);
-         return info;
-     }
-@@ -938,7 +940,7 @@ BackupStatus *qmp_query_backup(Error **errp)
-     info->has_transferred = true;
-     info->transferred = backup_state.stat.transferred;
--    qemu_rec_mutex_unlock(&backup_state.stat.lock);
-+    qemu_mutex_unlock(&backup_state.stat.lock);
-     return info;
- }
-diff --git a/vma-writer.c b/vma-writer.c
-index fe86b18a60..f3fa8e3b4c 100644
---- a/vma-writer.c
-+++ b/vma-writer.c
-@@ -68,8 +68,14 @@ struct VmaWriter {
-     uint32_t config_names[VMA_MAX_CONFIGS]; /* offset into blob_buffer table */
-     uint32_t config_data[VMA_MAX_CONFIGS];  /* offset into blob_buffer table */
-     uint32_t config_count;
-+
-+    QemuMutex *external_lock;
- };
-+void vma_writer_set_external_lock(VmaWriter *vmaw, QemuMutex *lock) {
-+    vmaw->external_lock = lock;
-+}
-+
- void vma_writer_set_error(VmaWriter *vmaw, const char *fmt, ...)
- {
-     va_list ap;
-@@ -199,13 +205,20 @@ int vma_writer_register_stream(VmaWriter *vmaw, const char *devname,
-     return n;
- }
--static void coroutine_fn yield_until_fd_writable(int fd)
-+static void coroutine_fn yield_until_fd_writable(int fd, QemuMutex *external_lock)
- {
-     assert(qemu_in_coroutine());
-     AioContext *ctx = qemu_get_current_aio_context();
-     aio_set_fd_handler(ctx, fd, false, NULL, (IOHandler *)qemu_coroutine_enter,
-                        NULL, qemu_coroutine_self());
-+    if (external_lock) {
-+      /* still protected from re-entering via flush_lock */
-+      qemu_mutex_unlock(external_lock);
-+    }
-     qemu_coroutine_yield();
-+    if (!external_lock) {
-+      qemu_mutex_lock(external_lock);
-+    }
-     aio_set_fd_handler(ctx, fd, false, NULL, NULL, NULL, NULL);
- }
-@@ -227,11 +240,16 @@ vma_queue_write(VmaWriter *vmaw, const void *buf, size_t bytes)
-     while (done < bytes) {
-         if (vmaw->status < 0) {
--            DPRINTF("vma_queue_write detected canceled backup\n");
-+            DPRINTF("vma_queue_write detected cancelled backup\n");
-+            done = -1;
-+            break;
-+        }
-+        yield_until_fd_writable(vmaw->fd, vmaw->external_lock);
-+        if (vmaw->closed || vmaw->status < 0) {
-+            DPRINTF("vma_queue_write backup cancelled after waiting for fd\n");
-             done = -1;
-             break;
-         }
--        yield_until_fd_writable(vmaw->fd);
-         ret = write(vmaw->fd, buf + done, bytes - done);
-         if (ret > 0) {
-             done += ret;
-@@ -501,7 +519,7 @@ vma_writer_flush_output(VmaWriter *vmaw)
-     int ret = vma_writer_flush(vmaw);
-     qemu_co_mutex_unlock(&vmaw->flush_lock);
-     if (ret < 0) {
--        vma_writer_set_error(vmaw, "vma_writer_flush_header failed");
-+        vma_writer_set_error(vmaw, "vma_writer_flush_output failed");
-     }
-     return ret;
- }
-@@ -570,7 +588,7 @@ static int vma_writer_get_buffer(VmaWriter *vmaw)
-     while (vmaw->outbuf_count >= (VMA_BLOCKS_PER_EXTENT - 1)) {
-         ret = vma_writer_flush(vmaw);
-         if (ret < 0) {
--            vma_writer_set_error(vmaw, "vma_writer_get_buffer: flush failed");
-+            vma_writer_set_error(vmaw, "vma_writer_get_header: flush failed");
-             break;
-         }
-     }
-diff --git a/vma.h b/vma.h
-index c895c97f6d..ec6f09e83e 100644
---- a/vma.h
-+++ b/vma.h
-@@ -115,6 +115,7 @@ typedef struct VmaDeviceInfo {
- } VmaDeviceInfo;
- VmaWriter *vma_writer_create(const char *filename, uuid_t uuid, Error **errp);
-+void vma_writer_set_external_lock(VmaWriter *vmaw, QemuMutex *lock);
- int vma_writer_close(VmaWriter *vmaw, Error **errp);
- void vma_writer_error_propagate(VmaWriter *vmaw, Error **errp);
- void vma_writer_destroy(VmaWriter *vmaw);
diff --git a/debian/patches/pve/0042-PVE-Backup-use-QemuMutex-instead-of-QemuRecMutex.patch b/debian/patches/pve/0042-PVE-Backup-use-QemuMutex-instead-of-QemuRecMutex.patch
new file mode 100644 (file)
index 0000000..b18762d
--- /dev/null
@@ -0,0 +1,227 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Dietmar Maurer <dietmar@proxmox.com>
+Date: Fri, 17 Apr 2020 08:57:48 +0200
+Subject: [PATCH] PVE Backup: use QemuMutex instead of QemuRecMutex
+
+We acquire/release all mutexes outside coroutines now, so we can now
+correctly use a normal mutex.
+---
+ pve-backup.c | 58 ++++++++++++++++++++++++++--------------------------
+ 1 file changed, 29 insertions(+), 29 deletions(-)
+
+diff --git a/pve-backup.c b/pve-backup.c
+index dddf430399..bb917ee972 100644
+--- a/pve-backup.c
++++ b/pve-backup.c
+@@ -31,7 +31,7 @@
+ static struct PVEBackupState {
+     struct {
+         // Everithing accessed from qmp_backup_query command is protected using lock
+-        QemuRecMutex lock;
++        QemuMutex lock;
+         Error *error;
+         time_t start_time;
+         time_t end_time;
+@@ -46,14 +46,14 @@ static struct PVEBackupState {
+     VmaWriter *vmaw;
+     ProxmoxBackupHandle *pbs;
+     GList *di_list;
+-    QemuRecMutex backup_mutex;
++    QemuMutex backup_mutex;
+     CoMutex dump_callback_mutex;
+ } backup_state;
+ static void pvebackup_init(void)
+ {
+-    qemu_rec_mutex_init(&backup_state.stat.lock);
+-    qemu_rec_mutex_init(&backup_state.backup_mutex);
++    qemu_mutex_init(&backup_state.stat.lock);
++    qemu_mutex_init(&backup_state.backup_mutex);
+     qemu_co_mutex_init(&backup_state.dump_callback_mutex);
+ }
+@@ -91,26 +91,26 @@ lookup_active_block_job(PVEBackupDevInfo *di)
+ static void pvebackup_propagate_error(Error *err)
+ {
+-    qemu_rec_mutex_lock(&backup_state.stat.lock);
++    qemu_mutex_lock(&backup_state.stat.lock);
+     error_propagate(&backup_state.stat.error, err);
+-    qemu_rec_mutex_unlock(&backup_state.stat.lock);
++    qemu_mutex_unlock(&backup_state.stat.lock);
+ }
+ static bool pvebackup_error_or_canceled(void)
+ {
+-    qemu_rec_mutex_lock(&backup_state.stat.lock);
++    qemu_mutex_lock(&backup_state.stat.lock);
+     bool error_or_canceled = !!backup_state.stat.error;
+-    qemu_rec_mutex_unlock(&backup_state.stat.lock);
++    qemu_mutex_unlock(&backup_state.stat.lock);
+     return error_or_canceled;
+ }
+ static void pvebackup_add_transfered_bytes(size_t transferred, size_t zero_bytes)
+ {
+-    qemu_rec_mutex_lock(&backup_state.stat.lock);
++    qemu_mutex_lock(&backup_state.stat.lock);
+     backup_state.stat.zero_bytes += zero_bytes;
+     backup_state.stat.transferred += transferred;
+-    qemu_rec_mutex_unlock(&backup_state.stat.lock);
++    qemu_mutex_unlock(&backup_state.stat.lock);
+ }
+ // This may get called from multiple coroutines in multiple io-threads
+@@ -226,9 +226,9 @@ static void coroutine_fn pvebackup_co_cleanup(void *unused)
+ {
+     assert(qemu_in_coroutine());
+-    qemu_rec_mutex_lock(&backup_state.stat.lock);
++    qemu_mutex_lock(&backup_state.stat.lock);
+     backup_state.stat.end_time = time(NULL);
+-    qemu_rec_mutex_unlock(&backup_state.stat.lock);
++    qemu_mutex_unlock(&backup_state.stat.lock);
+     if (backup_state.vmaw) {
+         Error *local_err = NULL;
+@@ -284,7 +284,7 @@ static void pvebackup_complete_cb(void *opaque, int ret)
+     PVEBackupDevInfo *di = opaque;
+-    qemu_rec_mutex_lock(&backup_state.backup_mutex);
++    qemu_mutex_lock(&backup_state.backup_mutex);
+     di->completed = true;
+@@ -305,7 +305,7 @@ static void pvebackup_complete_cb(void *opaque, int ret)
+     g_free(di);
+-    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++    qemu_mutex_unlock(&backup_state.backup_mutex);
+     pvebackup_run_next_job();
+ }
+@@ -318,7 +318,7 @@ static void pvebackup_cancel(void)
+     error_setg(&cancel_err, "backup canceled");
+     pvebackup_propagate_error(cancel_err);
+-    qemu_rec_mutex_lock(&backup_state.backup_mutex);
++    qemu_mutex_lock(&backup_state.backup_mutex);
+     if (backup_state.vmaw) {
+         /* make sure vma writer does not block anymore */
+@@ -329,13 +329,13 @@ static void pvebackup_cancel(void)
+         proxmox_backup_abort(backup_state.pbs, "backup canceled");
+     }
+-    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++    qemu_mutex_unlock(&backup_state.backup_mutex);
+     for(;;) {
+         BlockJob *next_job = NULL;
+-        qemu_rec_mutex_lock(&backup_state.backup_mutex);
++        qemu_mutex_lock(&backup_state.backup_mutex);
+         GList *l = backup_state.di_list;
+         while (l) {
+@@ -349,7 +349,7 @@ static void pvebackup_cancel(void)
+             }
+         }
+-        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++        qemu_mutex_unlock(&backup_state.backup_mutex);
+         if (next_job) {
+             AioContext *aio_context = next_job->job.aio_context;
+@@ -423,7 +423,7 @@ static void pvebackup_run_next_job(void)
+ {
+     assert(!qemu_in_coroutine());
+-    qemu_rec_mutex_lock(&backup_state.backup_mutex);
++    qemu_mutex_lock(&backup_state.backup_mutex);
+     GList *l = backup_state.di_list;
+     while (l) {
+@@ -433,7 +433,7 @@ static void pvebackup_run_next_job(void)
+         BlockJob *job = lookup_active_block_job(di);
+         if (job) {
+-            qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++            qemu_mutex_unlock(&backup_state.backup_mutex);
+             AioContext *aio_context = job->job.aio_context;
+             aio_context_acquire(aio_context);
+@@ -453,7 +453,7 @@ static void pvebackup_run_next_job(void)
+     block_on_coroutine_fn(pvebackup_co_cleanup, NULL); // no more jobs, run cleanup
+-    qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++    qemu_mutex_unlock(&backup_state.backup_mutex);
+ }
+ static bool create_backup_jobs(void) {
+@@ -778,7 +778,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+     }
+     /* initialize global backup_state now */
+-    qemu_rec_mutex_lock(&backup_state.stat.lock);
++    qemu_mutex_lock(&backup_state.stat.lock);
+     if (backup_state.stat.error) {
+         error_free(backup_state.stat.error);
+@@ -801,7 +801,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+     backup_state.stat.transferred = 0;
+     backup_state.stat.zero_bytes = 0;
+-    qemu_rec_mutex_unlock(&backup_state.stat.lock);
++    qemu_mutex_unlock(&backup_state.stat.lock);
+     backup_state.speed = (task->has_speed && task->speed > 0) ? task->speed : 0;
+@@ -895,16 +895,16 @@ UuidInfo *qmp_backup(
+         .errp = errp,
+     };
+-    qemu_rec_mutex_lock(&backup_state.backup_mutex);
++    qemu_mutex_lock(&backup_state.backup_mutex);
+     block_on_coroutine_fn(pvebackup_co_prepare, &task);
+     if (*errp == NULL) {
+         create_backup_jobs();
+-        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++        qemu_mutex_unlock(&backup_state.backup_mutex);
+         pvebackup_run_next_job();
+     } else {
+-        qemu_rec_mutex_unlock(&backup_state.backup_mutex);
++        qemu_mutex_unlock(&backup_state.backup_mutex);
+     }
+     return task.result;
+@@ -914,11 +914,11 @@ BackupStatus *qmp_query_backup(Error **errp)
+ {
+     BackupStatus *info = g_malloc0(sizeof(*info));
+-    qemu_rec_mutex_lock(&backup_state.stat.lock);
++    qemu_mutex_lock(&backup_state.stat.lock);
+     if (!backup_state.stat.start_time) {
+         /* not started, return {} */
+-        qemu_rec_mutex_unlock(&backup_state.stat.lock);
++        qemu_mutex_unlock(&backup_state.stat.lock);
+         return info;
+     }
+@@ -955,7 +955,7 @@ BackupStatus *qmp_query_backup(Error **errp)
+     info->has_transferred = true;
+     info->transferred = backup_state.stat.transferred;
+-    qemu_rec_mutex_unlock(&backup_state.stat.lock);
++    qemu_mutex_unlock(&backup_state.stat.lock);
+     return info;
+ }
index 054ea0bb78a00becc9bb2499629e4e44a3b06f42..130ba532934eb0b0ef947bbc874f8090227e1819 100644 (file)
@@ -38,4 +38,5 @@ pve/0037-mirror-switch-to-bdrv_dirty_bitmap_merge_internal.patch
 pve/0038-iotests-add-test-for-bitmap-mirror.patch
 pve/0039-mirror-move-some-checks-to-qmp.patch
 pve/0040-PVE-savevm-async-set-up-migration-state.patch
-pve/0041-PVE-Use-non-recursive-locks.patch
+pve/0041-PVE-Backup-avoid-use-QemuRecMutex-inside-coroutines.patch
+pve/0042-PVE-Backup-use-QemuMutex-instead-of-QemuRecMutex.patch