[pve-devel] [PATCH qemu 2/4] Apply fixups for 4.1
Stefan Reiter
s.reiter at proxmox.com
Wed Nov 20 15:45:36 CET 2019
* Fix VMA tool build
* Change PVE code to new blockjob API
* Acquire missing lock for block_job_add_bdrv
Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---
First two patches by Thomas.
Last one added by me, otherwise Backup immediately crashes with 'Permission
denied' in 4.1. My patch makes it work, but I'm really not sure if acquiring a
lock for no reason other than to avoid a crash is the way to go. Couldn't figure
out an easy solution without changing a lot of unrelated QEMU code though.
.../patches/pve/0042-PVE-fixup-vma-tool.patch | 33 ++++++++
...ev-pvebackup-integration-fix-blockjo.patch | 81 +++++++++++++++++++
...ext-before-calling-block_job_add_bdr.patch | 38 +++++++++
debian/patches/series | 3 +
4 files changed, 155 insertions(+)
create mode 100644 debian/patches/pve/0042-PVE-fixup-vma-tool.patch
create mode 100644 debian/patches/pve/0043-PVE-fixup-blockdev-pvebackup-integration-fix-blockjo.patch
create mode 100644 debian/patches/pve/0044-Acquire-aio_context-before-calling-block_job_add_bdr.patch
diff --git a/debian/patches/pve/0042-PVE-fixup-vma-tool.patch b/debian/patches/pve/0042-PVE-fixup-vma-tool.patch
new file mode 100644
index 0000000..828a7fe
--- /dev/null
+++ b/debian/patches/pve/0042-PVE-fixup-vma-tool.patch
@@ -0,0 +1,33 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Thomas Lamprecht <t.lamprecht at proxmox.com>
+Date: Mon, 26 Aug 2019 18:40:10 +0200
+Subject: [PATCH] PVE: fixup: vma tool
+
+Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
+---
+ vma.c | 5 ++++-
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/vma.c b/vma.c
+index 3289fd722f..a82752448a 100644
+--- a/vma.c
++++ b/vma.c
+@@ -16,6 +16,7 @@
+
+ #include "vma.h"
+ #include "qemu-common.h"
++#include "qemu/module.h"
+ #include "qemu/error-report.h"
+ #include "qemu/main-loop.h"
+ #include "qemu/cutils.h"
+@@ -800,7 +801,9 @@ int main(int argc, char **argv)
+ const char *cmdname;
+ Error *main_loop_err = NULL;
+
+- error_set_progname(argv[0]);
++ error_init(argv[0]);
++ module_call_init(MODULE_INIT_TRACE);
++ qemu_init_exec_dir(argv[0]);
+
+ if (qemu_init_main_loop(&main_loop_err)) {
+ g_error("%s", error_get_pretty(main_loop_err));
diff --git a/debian/patches/pve/0043-PVE-fixup-blockdev-pvebackup-integration-fix-blockjo.patch b/debian/patches/pve/0043-PVE-fixup-blockdev-pvebackup-integration-fix-blockjo.patch
new file mode 100644
index 0000000..7f0b28a
--- /dev/null
+++ b/debian/patches/pve/0043-PVE-fixup-blockdev-pvebackup-integration-fix-blockjo.patch
@@ -0,0 +1,81 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Thomas Lamprecht <t.lamprecht at proxmox.com>
+Date: Mon, 26 Aug 2019 18:40:44 +0200
+Subject: [PATCH] PVE: fixup: blockdev pvebackup integration: fix blockjob
+
+Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
+Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
+---
+ blockdev.c | 49 +++++++++++++++++++++++++++----------------------
+ 1 file changed, 27 insertions(+), 22 deletions(-)
+
+diff --git a/blockdev.c b/blockdev.c
+index 083ada6c8e..46e8a2780a 100644
+--- a/blockdev.c
++++ b/blockdev.c
+@@ -3416,15 +3416,17 @@ static void coroutine_fn pvebackup_co_cancel(void *opaque)
+ PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
+ l = g_list_next(l);
+ if (!di->completed && di->bs) {
+- BlockJob *job = di->bs->job;
+- if (job) {
+- AioContext *aio_context = blk_get_aio_context(job->blk);
+- aio_context_acquire(aio_context);
+- if (!di->completed) {
+- running_jobs += 1;
+- job_cancel(&job->job, false);
++ for (BlockJob *job = block_job_next(NULL); job; job = block_job_next(job)) {
++ if (block_job_has_bdrv(job, di->bs)) {
++ AioContext *aio_context = job->job.aio_context;
++ aio_context_acquire(aio_context);
++
++ if (!di->completed) {
++ running_jobs += 1;
++ job_cancel(&job->job, false);
++ }
++ aio_context_release(aio_context);
+ }
+- aio_context_release(aio_context);
+ }
+ }
+ }
+@@ -3499,22 +3501,25 @@ static void coroutine_fn pvebackup_co_run_next_job(void)
+ while (l) {
+ PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
+ l = g_list_next(l);
+- if (!di->completed && di->bs && di->bs->job) {
+- BlockJob *job = di->bs->job;
+- AioContext *aio_context = blk_get_aio_context(job->blk);
+- bool cancel_job = backup_state.error || backup_state.cancel;
+- qemu_co_mutex_unlock(&backup_state.backup_mutex);
+-
+- aio_context_acquire(aio_context);
+- if (job_should_pause(&job->job)) {
+- if (cancel_job) {
+- job_cancel(&job->job, false);
+- } else {
+- job_resume(&job->job);
++ if (!di->completed && di->bs) {
++ for (BlockJob *job = block_job_next(NULL); job; job = block_job_next(job)) {
++ if (block_job_has_bdrv(job, di->bs)) {
++ AioContext *aio_context = job->job.aio_context;
++ qemu_co_mutex_unlock(&backup_state.backup_mutex);
++ aio_context_acquire(aio_context);
++
++
++ if (job_should_pause(&job->job)) {
++ if (backup_state.error || backup_state.cancel) {
++ job_cancel(&job->job, false);
++ } else {
++ job_resume(&job->job);
++ }
++ }
++ aio_context_release(aio_context);
++ return;
+ }
+ }
+- aio_context_release(aio_context);
+- return;
+ }
+ }
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
diff --git a/debian/patches/pve/0044-Acquire-aio_context-before-calling-block_job_add_bdr.patch b/debian/patches/pve/0044-Acquire-aio_context-before-calling-block_job_add_bdr.patch
new file mode 100644
index 0000000..01f7f4f
--- /dev/null
+++ b/debian/patches/pve/0044-Acquire-aio_context-before-calling-block_job_add_bdr.patch
@@ -0,0 +1,38 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Stefan Reiter <s.reiter at proxmox.com>
+Date: Mon, 21 Oct 2019 17:32:22 +0200
+Subject: [PATCH] Acquire aio_context before calling block_job_add_bdrv
+
+Otherwise backups immediately fail with 'permission denied' since
+_add_bdrv tries to release a lock we don't own.
+
+Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
+---
+ blockjob.c | 10 ++++++++++
+ 1 file changed, 10 insertions(+)
+
+diff --git a/blockjob.c b/blockjob.c
+index 20b7f557da..c6067474a2 100644
+--- a/blockjob.c
++++ b/blockjob.c
+@@ -439,10 +439,20 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+ notifier_list_add(&job->job.on_ready, &job->ready_notifier);
+ notifier_list_add(&job->job.on_idle, &job->idle_notifier);
+
++ /* block_job_add_bdrv expects us to hold the aio context lock, so acquire it
++ * before calling if we're not in the main context anyway. */
++ if (job->job.aio_context != qemu_get_aio_context()) {
++ aio_context_acquire(job->job.aio_context);
++ }
++
+ error_setg(&job->blocker, "block device is in use by block job: %s",
+ job_type_str(&job->job));
+ block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort);
+
++ if (job->job.aio_context != qemu_get_aio_context()) {
++ aio_context_release(job->job.aio_context);
++ }
++
+ bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
+
+ blk_set_allow_aio_context_change(blk, true);
diff --git a/debian/patches/series b/debian/patches/series
index 11f0507..af0f24a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -40,3 +40,6 @@ pve/0038-pvebackup_co_dump_cb-do-not-call-job-cancel.patch
pve/0039-fix-backup-job-completion.patch
pve/0040-pvebackup_complete_cb-avoid-poll-loop-if-already-ins.patch
pve/0041-PVE-backup-consider-source-cluster-size-as-well.patch
+pve/0042-PVE-fixup-vma-tool.patch
+pve/0043-PVE-fixup-blockdev-pvebackup-integration-fix-blockjo.patch
+pve/0044-Acquire-aio_context-before-calling-block_job_add_bdr.patch
--
2.20.1
More information about the pve-devel
mailing list