From 22ff24871daee6d7c09fa639cdb4838a47a219c7 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Wed, 30 Oct 2019 16:03:56 +0100 Subject: [PATCH] various small pve backup co-routine related fixes Signed-off-by: Thomas Lamprecht --- ...up_co_dump_cb-do-not-call-job-cancel.patch | 27 ++++++++++++ .../pve/0039-fix-backup-job-completion.patch | 44 +++++++++++++++++++ ...te_cb-avoid-poll-loop-if-already-ins.patch | 43 ++++++++++++++++++ debian/patches/series | 3 ++ 4 files changed, 117 insertions(+) create mode 100644 debian/patches/pve/0038-pvebackup_co_dump_cb-do-not-call-job-cancel.patch create mode 100644 debian/patches/pve/0039-fix-backup-job-completion.patch create mode 100644 debian/patches/pve/0040-pvebackup_complete_cb-avoid-poll-loop-if-already-ins.patch diff --git a/debian/patches/pve/0038-pvebackup_co_dump_cb-do-not-call-job-cancel.patch b/debian/patches/pve/0038-pvebackup_co_dump_cb-do-not-call-job-cancel.patch new file mode 100644 index 0000000..1ff243f --- /dev/null +++ b/debian/patches/pve/0038-pvebackup_co_dump_cb-do-not-call-job-cancel.patch @@ -0,0 +1,27 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Dietmar Maurer +Date: Wed, 30 Oct 2019 12:15:44 +0100 +Subject: [PATCH] pvebackup_co_dump_cb: do not call job->cancel() + +The backup loop will automatically abort if we return an error. +--- + blockdev.c | 6 ++---- + 1 file changed, 2 insertions(+), 4 deletions(-) + +diff --git a/blockdev.c b/blockdev.c +index 786921da0a..a36c4ce23d 100644 +--- a/blockdev.c ++++ b/blockdev.c +@@ -3254,10 +3254,8 @@ static int coroutine_fn pvebackup_co_dump_cb(void *opaque, BlockBackend *target, + if (!backup_state.error) { + vma_writer_error_propagate(backup_state.vmaw, &backup_state.error); + } +- if (di->bs && di->bs->job) { +- job_cancel(&di->bs->job->job, true); +- } +- break; ++ qemu_co_mutex_unlock(&backup_state.backup_mutex); ++ return ret; + } else { + backup_state.zero_bytes += zero_bytes; + if (remaining >= VMA_CLUSTER_SIZE) { diff --git a/debian/patches/pve/0039-fix-backup-job-completion.patch b/debian/patches/pve/0039-fix-backup-job-completion.patch new file mode 100644 index 0000000..f99201a --- /dev/null +++ b/debian/patches/pve/0039-fix-backup-job-completion.patch @@ -0,0 +1,44 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Dietmar Maurer +Date: Wed, 30 Oct 2019 12:15:45 +0100 +Subject: [PATCH] fix backup job completion + +With recent changes, pvebackup_co_run_next_job cancels the job async, +so we need to run pvebackup_co_cleanup in the completion handler +instead. We call pvebackup_co_run_next as long as there are +jobs in the list. +--- + blockdev.c | 9 ++++----- + 1 file changed, 4 insertions(+), 5 deletions(-) + +diff --git a/blockdev.c b/blockdev.c +index a36c4ce23d..421240fbb8 100644 +--- a/blockdev.c ++++ b/blockdev.c +@@ -3339,12 +3339,14 @@ static void coroutine_fn pvebackup_co_complete_cb(void *opaque) + backup_state.di_list = g_list_remove(backup_state.di_list, di); + g_free(di); + +- bool cancel = backup_state.cancel; ++ int pending_jobs = g_list_length(backup_state.di_list); + + qemu_co_mutex_unlock(&backup_state.backup_mutex); + +- if (!cancel) { ++ if (pending_jobs > 0) { + pvebackup_co_run_next_job(); ++ } else { ++ pvebackup_co_cleanup(); + } + } + +@@ -3490,9 +3492,6 @@ static void coroutine_fn pvebackup_co_run_next_job(void) + } + } + qemu_co_mutex_unlock(&backup_state.backup_mutex); +- +- // no more jobs, run the cleanup +- pvebackup_co_cleanup(); + } + + typedef struct QmpBackupTask { diff --git a/debian/patches/pve/0040-pvebackup_complete_cb-avoid-poll-loop-if-already-ins.patch b/debian/patches/pve/0040-pvebackup_complete_cb-avoid-poll-loop-if-already-ins.patch new file mode 100644 index 0000000..7bd11a5 --- /dev/null +++ b/debian/patches/pve/0040-pvebackup_complete_cb-avoid-poll-loop-if-already-ins.patch @@ -0,0 +1,43 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Dietmar Maurer +Date: Wed, 30 Oct 2019 12:15:46 +0100 +Subject: [PATCH] pvebackup_complete_cb: avoid poll loop if already inside + coroutine + +--- + blockdev.c | 10 ++++++++-- + 1 file changed, 8 insertions(+), 2 deletions(-) + +diff --git a/blockdev.c b/blockdev.c +index 421240fbb8..e889bd13d5 100644 +--- a/blockdev.c ++++ b/blockdev.c +@@ -3169,6 +3169,8 @@ static void coroutine_fn block_on_coroutine_wrapper(void *opaque) + + static void block_on_coroutine_fn(CoroutineEntry *entry, void *entry_arg) + { ++ assert(!qemu_in_coroutine()); ++ + AioContext *ctx = qemu_get_current_aio_context(); + BlockOnCoroutineWrapper wrapper = { + .finished = false, +@@ -3352,13 +3354,17 @@ static void coroutine_fn pvebackup_co_complete_cb(void *opaque) + + static void pvebackup_complete_cb(void *opaque, int ret) + { +- // This always called from the main loop ++ // This can be called from the main loop, or from a coroutine + PVEBackupCompeteCallbackData cb_data = { + .di = opaque, + .result = ret, + }; + +- block_on_coroutine_fn(pvebackup_co_complete_cb, &cb_data); ++ if (qemu_in_coroutine()) { ++ pvebackup_co_complete_cb(&cb_data); ++ } else { ++ block_on_coroutine_fn(pvebackup_co_complete_cb, &cb_data); ++ } + } + + static void coroutine_fn pvebackup_co_cancel(void *opaque) diff --git a/debian/patches/series b/debian/patches/series index 029f261..d56cc1b 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -36,3 +36,6 @@ pve/0034-vma_writer_close-avoid-call-to-aio_poll-acquire-flus.patch pve/0035-backup_job_create-pass-cluster-size-for-dump.patch pve/0036-avoid-calling-dump_cb-with-NULL-data-pointer-for-sma.patch pve/0037-rename-config_to_vma-into-pvebackup_co_add_config.patch +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