c/pve-qemu-qoup
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

run backup related code inside co-routines and improve locking

Browse Source 
Patches-by: Dietmar Maurer <dietmar@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2019-10-23 09:14:42 +02:00
parent c306e84e86
commit cbb547903c
4 changed files with 823 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

511
debian/patches/pve/0032-qmp_backup-run-backup-related-code-inside-coroutines.patch vendored Normal file
View File

@ -0,0 +1,511 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Dietmar Maurer <dietmar@proxmox.com>
Date: Tue, 22 Oct 2019 12:48:17 +0200
Subject: [PATCH] qmp_backup: run backup related code inside coroutines
So that we can safely use coroutines/yield everywhere.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
blockdev.c | 250 ++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 180 insertions(+), 70 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 2466a02cbd..85031de942 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3153,6 +3153,34 @@ out:
/* PVE backup related function */
+typedef struct BlockOnCoroutineWrapper {
+ AioContext *ctx;
+ CoroutineEntry *entry;
+ void *entry_arg;
+ bool finished;
+} BlockOnCoroutineWrapper;
+
+static void coroutine_fn block_on_coroutine_wrapper(void *opaque)
+{
+ BlockOnCoroutineWrapper *wrapper = opaque;
+ wrapper->entry(wrapper->entry_arg);
+ wrapper->finished = true;
+}
+
+static void block_on_coroutine_fn(CoroutineEntry *entry, void *entry_arg)
+{
+ AioContext *ctx = qemu_get_current_aio_context();
+ BlockOnCoroutineWrapper wrapper = {
+ .finished = false,
+ .entry = entry,
+ .entry_arg = entry_arg,
+ .ctx = ctx,
+ };
+ Coroutine *wrapper_co = qemu_coroutine_create(block_on_coroutine_wrapper, &wrapper);
+ aio_co_enter(ctx, wrapper_co);
+ AIO_WAIT_WHILE(ctx, !wrapper.finished);
+}
+
static struct PVEBackupState {
Error *error;
bool cancel;
@@ -3180,12 +3208,14 @@ typedef struct PVEBackupDevInfo {
BlockDriverState *target;
} PVEBackupDevInfo;
-static void pvebackup_run_next_job(void);
+static void pvebackup_co_run_next_job(void);
-static int pvebackup_dump_cb(void *opaque, BlockBackend *target,
+static int coroutine_fn pvebackup_co_dump_cb(void *opaque, BlockBackend *target,
uint64_t start, uint64_t bytes,
const void *pbuf)
{
+ assert(qemu_in_coroutine());
+
const uint64_t size = bytes;
const unsigned char *buf = pbuf;
PVEBackupDevInfo *di = opaque;
@@ -3247,8 +3277,10 @@ static int pvebackup_dump_cb(void *opaque, BlockBackend *target,
return size;
}
-static void pvebackup_cleanup(void)
+static void coroutine_fn pvebackup_co_cleanup(void)
{
+ assert(qemu_in_coroutine());
+
qemu_mutex_lock(&backup_state.backup_mutex);
// Avoid race between block jobs and backup-cancel command:
if (!backup_state.vmaw) {
@@ -3270,18 +3302,19 @@ static void pvebackup_cleanup(void)
qemu_mutex_unlock(&backup_state.backup_mutex);
}
-static void coroutine_fn backup_close_vma_stream(void *opaque)
-{
- PVEBackupDevInfo *di = opaque;
+typedef struct PVEBackupCompeteCallbackData {
+ PVEBackupDevInfo *di;
+ int result;
+} PVEBackupCompeteCallbackData;
- vma_writer_close_stream(backup_state.vmaw, di->dev_id);
-}
-
-static void pvebackup_complete_cb(void *opaque, int ret)
+static void coroutine_fn pvebackup_co_complete_cb(void *opaque)
{
- // This always runs in the main loop
+ assert(qemu_in_coroutine());
- PVEBackupDevInfo *di = opaque;
+ PVEBackupCompeteCallbackData *cb_data = opaque;
+
+ PVEBackupDevInfo *di = cb_data->di;
+ int ret = cb_data->result;
di->completed = true;
@@ -3294,8 +3327,7 @@ static void pvebackup_complete_cb(void *opaque, int ret)
di->target = NULL;
if (backup_state.vmaw) {
- Coroutine *co = qemu_coroutine_create(backup_close_vma_stream, di);
- qemu_coroutine_enter(co);
+ vma_writer_close_stream(backup_state.vmaw, di->dev_id);
}
// remove self from job queue
@@ -3305,12 +3337,25 @@ static void pvebackup_complete_cb(void *opaque, int ret)
qemu_mutex_unlock(&backup_state.backup_mutex);
if (!backup_state.cancel) {
- pvebackup_run_next_job();
+ pvebackup_co_run_next_job();
}
}
-static void pvebackup_cancel(void *opaque)
+static void pvebackup_complete_cb(void *opaque, int ret)
{
+ // This always called from the main loop
+ PVEBackupCompeteCallbackData cb_data = {
+ .di = opaque,
+ .result = ret,
+ };
+
+ block_on_coroutine_fn(pvebackup_co_complete_cb, &cb_data);
+}
+
+static void coroutine_fn pvebackup_co_cancel(void *opaque)
+{
+ assert(qemu_in_coroutine());
+
backup_state.cancel = true;
qemu_mutex_lock(&backup_state.backup_mutex);
// Avoid race between block jobs and backup-cancel command:
@@ -3345,21 +3390,16 @@ static void pvebackup_cancel(void *opaque)
}
}
- qemu_mutex_unlock(&backup_state.backup_mutex);
- pvebackup_cleanup();
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
+ pvebackup_co_cleanup();
}
void qmp_backup_cancel(Error **errp)
{
if (!backup_state.backup_mutex_initialized)
return;
- Coroutine *co = qemu_coroutine_create(pvebackup_cancel, NULL);
- qemu_coroutine_enter(co);
- while (backup_state.vmaw) {
- /* vma writer use main aio context */
- aio_poll(qemu_get_aio_context(), true);
- }
+ block_on_coroutine_fn(pvebackup_co_cancel, NULL);
}
static int config_to_vma(const char *file, BackupFormat format,
@@ -3400,8 +3440,11 @@ static int config_to_vma(const char *file, BackupFormat format,
}
bool job_should_pause(Job *job);
-static void pvebackup_run_next_job(void)
+
+static void coroutine_fn pvebackup_co_run_next_job(void)
{
+ assert(qemu_in_coroutine());
+
qemu_mutex_lock(&backup_state.backup_mutex);
GList *l = backup_state.di_list;
@@ -3427,16 +3470,33 @@ static void pvebackup_run_next_job(void)
qemu_mutex_unlock(&backup_state.backup_mutex);
// no more jobs, run the cleanup
- pvebackup_cleanup();
+ pvebackup_co_cleanup();
}
-UuidInfo *qmp_backup(const char *backup_file, bool has_format,
- BackupFormat format,
- bool has_config_file, const char *config_file,
- bool has_firewall_file, const char *firewall_file,
- bool has_devlist, const char *devlist,
- bool has_speed, int64_t speed, Error **errp)
+typedef struct QmpBackupTask {
+ const char *backup_file;
+ bool has_format;
+ BackupFormat format;
+ bool has_config_file;
+ const char *config_file;
+ bool has_firewall_file;
+ const char *firewall_file;
+ bool has_devlist;
+ const char *devlist;
+ bool has_speed;
+ int64_t speed;
+ Error **errp;
+ UuidInfo *result;
+} QmpBackupTask;
+
+static void coroutine_fn pvebackup_co_start(void *opaque)
{
+ assert(qemu_in_coroutine());
+
+ QmpBackupTask *task = opaque;
+
+ task->result = NULL; // just to be sure
+
BlockBackend *blk;
BlockDriverState *bs = NULL;
const char *backup_dir = NULL;
@@ -3455,16 +3515,16 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
}
if (backup_state.di_list) {
- error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+ error_set(task->errp, ERROR_CLASS_GENERIC_ERROR,
"previous backup not finished");
- return NULL;
+ return;
}
/* Todo: try to auto-detect format based on file name */
- format = has_format ? format : BACKUP_FORMAT_VMA;
+ BackupFormat format = task->has_format ? task->format : BACKUP_FORMAT_VMA;
- if (has_devlist) {
- devs = g_strsplit_set(devlist, ",;:", -1);
+ if (task->has_devlist) {
+ devs = g_strsplit_set(task->devlist, ",;:", -1);
gchar **d = devs;
while (d && *d) {
@@ -3472,18 +3532,18 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
if (blk) {
bs = blk_bs(blk);
if (bdrv_is_read_only(bs)) {
- error_setg(errp, "Node '%s' is read only", *d);
+ error_setg(task->errp, "Node '%s' is read only", *d);
goto err;
}
if (!bdrv_is_inserted(bs)) {
- error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, *d);
+ error_setg(task->errp, QERR_DEVICE_HAS_NO_MEDIUM, *d);
goto err;
}
PVEBackupDevInfo *di = g_new0(PVEBackupDevInfo, 1);
di->bs = bs;
di_list = g_list_append(di_list, di);
} else {
- error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+ error_set(task->errp, ERROR_CLASS_DEVICE_NOT_FOUND,
"Device '%s' not found", *d);
goto err;
}
@@ -3506,7 +3566,7 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
}
if (!di_list) {
- error_set(errp, ERROR_CLASS_GENERIC_ERROR, "empty device list");
+ error_set(task->errp, ERROR_CLASS_GENERIC_ERROR, "empty device list");
goto err;
}
@@ -3516,13 +3576,13 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
while (l) {
PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
l = g_list_next(l);
- if (bdrv_op_is_blocked(di->bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+ if (bdrv_op_is_blocked(di->bs, BLOCK_OP_TYPE_BACKUP_SOURCE, task->errp)) {
goto err;
}
ssize_t size = bdrv_getlength(di->bs);
if (size < 0) {
- error_setg_errno(errp, -di->size, "bdrv_getlength failed");
+ error_setg_errno(task->errp, -di->size, "bdrv_getlength failed");
goto err;
}
di->size = size;
@@ -3532,10 +3592,10 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
uuid_generate(uuid);
if (format == BACKUP_FORMAT_VMA) {
- vmaw = vma_writer_create(backup_file, uuid, &local_err);
+ vmaw = vma_writer_create(task->backup_file, uuid, &local_err);
if (!vmaw) {
if (local_err) {
- error_propagate(errp, local_err);
+ error_propagate(task->errp, local_err);
}
goto err;
}
@@ -3549,18 +3609,18 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
const char *devname = bdrv_get_device_name(di->bs);
di->dev_id = vma_writer_register_stream(vmaw, devname, di->size);
if (di->dev_id <= 0) {
- error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+ error_set(task->errp, ERROR_CLASS_GENERIC_ERROR,
"register_stream failed");
goto err;
}
}
} else if (format == BACKUP_FORMAT_DIR) {
- if (mkdir(backup_file, 0640) != 0) {
- error_setg_errno(errp, errno, "can't create directory '%s'\n",
- backup_file);
+ if (mkdir(task->backup_file, 0640) != 0) {
+ error_setg_errno(task->errp, errno, "can't create directory '%s'\n",
+ task->backup_file);
goto err;
}
- backup_dir = backup_file;
+ backup_dir = task->backup_file;
l = di_list;
while (l) {
@@ -3574,31 +3634,31 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
bdrv_img_create(di->targetfile, "raw", NULL, NULL, NULL,
di->size, flags, false, &local_err);
if (local_err) {
- error_propagate(errp, local_err);
+ error_propagate(task->errp, local_err);
goto err;
}
di->target = bdrv_open(di->targetfile, NULL, NULL, flags, &local_err);
if (!di->target) {
- error_propagate(errp, local_err);
+ error_propagate(task->errp, local_err);
goto err;
}
}
} else {
- error_set(errp, ERROR_CLASS_GENERIC_ERROR, "unknown backup format");
+ error_set(task->errp, ERROR_CLASS_GENERIC_ERROR, "unknown backup format");
goto err;
}
/* add configuration file to archive */
- if (has_config_file) {
- if (config_to_vma(config_file, format, backup_dir, vmaw, errp) != 0) {
+ if (task->has_config_file) {
+ if (config_to_vma(task->config_file, format, backup_dir, vmaw, task->errp) != 0) {
goto err;
}
}
/* add firewall file to archive */
- if (has_firewall_file) {
- if (config_to_vma(firewall_file, format, backup_dir, vmaw, errp) != 0) {
+ if (task->has_firewall_file) {
+ if (config_to_vma(task->firewall_file, format, backup_dir, vmaw, task->errp) != 0) {
goto err;
}
}
@@ -3611,7 +3671,7 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
backup_state.error = NULL;
}
- backup_state.speed = (has_speed && speed > 0) ? speed : 0;
+ backup_state.speed = (task->has_speed && task->speed > 0) ? task->speed : 0;
backup_state.start_time = time(NULL);
backup_state.end_time = 0;
@@ -3619,7 +3679,7 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
if (backup_state.backup_file) {
g_free(backup_state.backup_file);
}
- backup_state.backup_file = g_strdup(backup_file);
+ backup_state.backup_file = g_strdup(task->backup_file);
backup_state.vmaw = vmaw;
@@ -3638,14 +3698,13 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
while (l) {
PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
l = g_list_next(l);
- job = backup_job_create(NULL, di->bs, di->target, speed, MIRROR_SYNC_MODE_FULL, NULL,
+ job = backup_job_create(NULL, di->bs, di->target, backup_state.speed, MIRROR_SYNC_MODE_FULL, NULL,
false, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
- JOB_DEFAULT,
- pvebackup_dump_cb, pvebackup_complete_cb, di,
+ JOB_DEFAULT, pvebackup_co_dump_cb, pvebackup_complete_cb, di,
1, NULL, &local_err);
if (!job || local_err != NULL) {
error_setg(&backup_state.error, "backup_job_create failed");
- pvebackup_cancel(NULL);
+ pvebackup_co_cancel(NULL);
} else {
job_start(&job->job);
}
@@ -3658,13 +3717,14 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
qemu_mutex_unlock(&backup_state.backup_mutex);
if (!backup_state.error) {
- pvebackup_run_next_job(); // run one job
+ pvebackup_co_run_next_job(); // run one job
}
uuid_info = g_malloc0(sizeof(*uuid_info));
uuid_info->UUID = g_strdup(backup_state.uuid_str);
- return uuid_info;
+ task->result = uuid_info;
+ return;
err:
@@ -3691,23 +3751,61 @@ err:
if (vmaw) {
Error *err = NULL;
vma_writer_close(vmaw, &err);
- unlink(backup_file);
+ unlink(task->backup_file);
}
if (backup_dir) {
rmdir(backup_dir);
}
- return NULL;
+ task->result = NULL;
+ return;
}
-BackupStatus *qmp_query_backup(Error **errp)
+UuidInfo *qmp_backup(const char *backup_file, bool has_format,
+ BackupFormat format,
+ bool has_config_file, const char *config_file,
+ bool has_firewall_file, const char *firewall_file,
+ bool has_devlist, const char *devlist,
+ bool has_speed, int64_t speed, Error **errp)
+{
+ QmpBackupTask task = {
+ .backup_file = backup_file,
+ .has_format = has_format,
+ .format = format,
+ .has_config_file = has_config_file,
+ .config_file = config_file,
+ .has_firewall_file = has_firewall_file,
+ .firewall_file = firewall_file,
+ .has_devlist = has_devlist,
+ .devlist = devlist,
+ .has_speed = has_speed,
+ .speed = speed,
+ .errp = errp,
+ };
+
+ block_on_coroutine_fn(pvebackup_co_start, &task);
+ return task.result;
+}
+
+
+typedef struct QmpQueryBackupTask {
+ Error **errp;
+ BackupStatus *result;
+} QmpQueryBackupTask;
+
+static void coroutine_fn pvebackup_co_query(void *opaque)
{
+ assert(qemu_in_coroutine());
+
+ QmpQueryBackupTask *task = opaque;
+
BackupStatus *info = g_malloc0(sizeof(*info));
if (!backup_state.start_time) {
/* not started, return {} */
- return info;
+ task->result = info;
+ return;
}
info->has_status = true;
@@ -3743,7 +3841,19 @@ BackupStatus *qmp_query_backup(Error **errp)
info->has_transferred = true;
info->transferred = backup_state.transferred;
- return info;
+ task->result = info;
+}
+
+BackupStatus *qmp_query_backup(Error **errp)
+{
+ QmpQueryBackupTask task = {
+ .errp = errp,
+ .result = NULL,
+ };
+
+ block_on_coroutine_fn(pvebackup_co_query, &task);
+
+ return task.result;
}
void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,

274
debian/patches/pve/0033-qmp_backup-use-a-CoMutex-to-protect-access-to-backup.patch vendored Normal file
View File

@ -0,0 +1,274 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Dietmar Maurer <dietmar@proxmox.com>
Date: Tue, 22 Oct 2019 12:48:18 +0200
Subject: [PATCH] qmp_backup: use a CoMutex to protect access to backup_state
And use job_cancel instead of job_cancel_sync (which hangs sometimes)
Also avoid calling pvebackup_co_cleanup if not necessary. If there are
running jobs, this is called from the block job completion callbacks
anyways.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
blockdev.c | 74 ++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 52 insertions(+), 22 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 85031de942..7e9241cf42 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3195,7 +3195,7 @@ static struct PVEBackupState {
size_t total;
size_t transferred;
size_t zero_bytes;
- QemuMutex backup_mutex;
+ CoMutex backup_mutex;
bool backup_mutex_initialized;
} backup_state;
@@ -3220,7 +3220,10 @@ static int coroutine_fn pvebackup_co_dump_cb(void *opaque, BlockBackend *target,
const unsigned char *buf = pbuf;
PVEBackupDevInfo *di = opaque;
+ qemu_co_mutex_lock(&backup_state.backup_mutex);
+
if (backup_state.cancel) {
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
return size; // return success
}
@@ -3231,6 +3234,7 @@ static int coroutine_fn pvebackup_co_dump_cb(void *opaque, BlockBackend *target,
"got unaligned write inside backup dump "
"callback (sector %ld)", start);
}
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
return -1; // not aligned to cluster size
}
@@ -3272,6 +3276,8 @@ static int coroutine_fn pvebackup_co_dump_cb(void *opaque, BlockBackend *target,
backup_state.transferred += size;
}
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
+
// Note: always return success, because we want that writes succeed anyways.
return size;
@@ -3281,10 +3287,9 @@ static void coroutine_fn pvebackup_co_cleanup(void)
{
assert(qemu_in_coroutine());
- qemu_mutex_lock(&backup_state.backup_mutex);
- // Avoid race between block jobs and backup-cancel command:
+ qemu_co_mutex_lock(&backup_state.backup_mutex);
if (!backup_state.vmaw) {
- qemu_mutex_unlock(&backup_state.backup_mutex);
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
return;
}
@@ -3299,7 +3304,7 @@ static void coroutine_fn pvebackup_co_cleanup(void)
g_list_free(backup_state.di_list);
backup_state.di_list = NULL;
- qemu_mutex_unlock(&backup_state.backup_mutex);
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
}
typedef struct PVEBackupCompeteCallbackData {
@@ -3313,6 +3318,8 @@ static void coroutine_fn pvebackup_co_complete_cb(void *opaque)
PVEBackupCompeteCallbackData *cb_data = opaque;
+ qemu_co_mutex_lock(&backup_state.backup_mutex);
+
PVEBackupDevInfo *di = cb_data->di;
int ret = cb_data->result;
@@ -3331,12 +3338,14 @@ static void coroutine_fn pvebackup_co_complete_cb(void *opaque)
}
// remove self from job queue
- qemu_mutex_lock(&backup_state.backup_mutex);
backup_state.di_list = g_list_remove(backup_state.di_list, di);
g_free(di);
- qemu_mutex_unlock(&backup_state.backup_mutex);
- if (!backup_state.cancel) {
+ bool cancel = backup_state.cancel;
+
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
+
+ if (!cancel) {
pvebackup_co_run_next_job();
}
}
@@ -3356,11 +3365,13 @@ static void coroutine_fn pvebackup_co_cancel(void *opaque)
{
assert(qemu_in_coroutine());
+ qemu_co_mutex_lock(&backup_state.backup_mutex);
+
backup_state.cancel = true;
- qemu_mutex_lock(&backup_state.backup_mutex);
+
// Avoid race between block jobs and backup-cancel command:
if (!backup_state.vmaw) {
- qemu_mutex_unlock(&backup_state.backup_mutex);
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
return;
}
@@ -3373,6 +3384,7 @@ static void coroutine_fn pvebackup_co_cancel(void *opaque)
vma_writer_set_error(backup_state.vmaw, "backup cancelled");
}
+ bool running_jobs = 0;
GList *l = backup_state.di_list;
while (l) {
PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
@@ -3383,6 +3395,7 @@ static void coroutine_fn pvebackup_co_cancel(void *opaque)
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);
}
aio_context_release(aio_context);
@@ -3391,7 +3404,8 @@ static void coroutine_fn pvebackup_co_cancel(void *opaque)
}
qemu_co_mutex_unlock(&backup_state.backup_mutex);
- pvebackup_co_cleanup();
+
+ if (running_jobs == 0) pvebackup_co_cleanup(); // else job will call completion handler
}
void qmp_backup_cancel(Error **errp)
@@ -3445,7 +3459,7 @@ static void coroutine_fn pvebackup_co_run_next_job(void)
{
assert(qemu_in_coroutine());
- qemu_mutex_lock(&backup_state.backup_mutex);
+ qemu_co_mutex_lock(&backup_state.backup_mutex);
GList *l = backup_state.di_list;
while (l) {
@@ -3454,11 +3468,13 @@ static void coroutine_fn pvebackup_co_run_next_job(void)
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);
- qemu_mutex_unlock(&backup_state.backup_mutex);
if (job_should_pause(&job->job)) {
- if (backup_state.error || backup_state.cancel) {
- job_cancel_sync(&job->job);
+ if (cancel_job) {
+ job_cancel(&job->job, false);
} else {
job_resume(&job->job);
}
@@ -3467,7 +3483,7 @@ static void coroutine_fn pvebackup_co_run_next_job(void)
return;
}
}
- qemu_mutex_unlock(&backup_state.backup_mutex);
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
// no more jobs, run the cleanup
pvebackup_co_cleanup();
@@ -3510,11 +3526,14 @@ static void coroutine_fn pvebackup_co_start(void *opaque)
BlockJob *job;
if (!backup_state.backup_mutex_initialized) {
- qemu_mutex_init(&backup_state.backup_mutex);
+ qemu_co_mutex_init(&backup_state.backup_mutex);
backup_state.backup_mutex_initialized = true;
}
+ qemu_co_mutex_lock(&backup_state.backup_mutex);
+
if (backup_state.di_list) {
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
error_set(task->errp, ERROR_CLASS_GENERIC_ERROR,
"previous backup not finished");
return;
@@ -3686,7 +3705,6 @@ static void coroutine_fn pvebackup_co_start(void *opaque)
uuid_copy(backup_state.uuid, uuid);
uuid_unparse_lower(uuid, backup_state.uuid_str);
- qemu_mutex_lock(&backup_state.backup_mutex);
backup_state.di_list = di_list;
backup_state.total = total;
@@ -3704,20 +3722,21 @@ static void coroutine_fn pvebackup_co_start(void *opaque)
1, NULL, &local_err);
if (!job || local_err != NULL) {
error_setg(&backup_state.error, "backup_job_create failed");
- pvebackup_co_cancel(NULL);
- } else {
- job_start(&job->job);
+ break;
}
+ job_start(&job->job);
if (di->target) {
bdrv_unref(di->target);
di->target = NULL;
}
}
- qemu_mutex_unlock(&backup_state.backup_mutex);
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
if (!backup_state.error) {
pvebackup_co_run_next_job(); // run one job
+ } else {
+ pvebackup_co_cancel(NULL);
}
uuid_info = g_malloc0(sizeof(*uuid_info));
@@ -3758,6 +3777,8 @@ err:
rmdir(backup_dir);
}
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
+
task->result = NULL;
return;
}
@@ -3785,6 +3806,7 @@ UuidInfo *qmp_backup(const char *backup_file, bool has_format,
};
block_on_coroutine_fn(pvebackup_co_start, &task);
+
return task.result;
}
@@ -3802,9 +3824,15 @@ static void coroutine_fn pvebackup_co_query(void *opaque)
BackupStatus *info = g_malloc0(sizeof(*info));
+ if (!backup_state.backup_mutex_initialized)
+ return;
+
+ qemu_co_mutex_lock(&backup_state.backup_mutex);
+
if (!backup_state.start_time) {
/* not started, return {} */
task->result = info;
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
return;
}
@@ -3842,6 +3870,8 @@ static void coroutine_fn pvebackup_co_query(void *opaque)
info->transferred = backup_state.transferred;
task->result = info;
+
+ qemu_co_mutex_unlock(&backup_state.backup_mutex);
}
BackupStatus *qmp_query_backup(Error **errp)

35
debian/patches/pve/0034-vma_writer_close-avoid-call-to-aio_poll-acquire-flus.patch vendored Normal file
View File

@ -0,0 +1,35 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Dietmar Maurer <dietmar@proxmox.com>
Date: Tue, 22 Oct 2019 12:48:19 +0200
Subject: [PATCH] vma_writer_close: avoid call to aio_poll (acquire flush_lock
instead)
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
vma-writer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/vma-writer.c b/vma-writer.c
index b163fa2d3a..fe86b18a60 100644
--- a/vma-writer.c
+++ b/vma-writer.c
@@ -705,9 +705,7 @@ int vma_writer_close(VmaWriter *vmaw, Error **errp)
int i;
- while (vmaw->co_writer) {
- aio_poll(qemu_get_aio_context(), true);
- }
+ qemu_co_mutex_lock(&vmaw->flush_lock); // wait for pending writes
assert(vmaw->co_writer == NULL);
@@ -748,6 +746,8 @@ int vma_writer_close(VmaWriter *vmaw, Error **errp)
error_setg(errp, "%s", vmaw->errmsg);
}
+ qemu_co_mutex_unlock(&vmaw->flush_lock);
+
return vmaw->status;
}

3
debian/patches/series vendored
View File

@ -30,3 +30,6 @@ pve/0028-PVE-savevm-async-kick-AIO-wait-on-block-state-write.patch
pve/0029-PVE-move-snapshot-cleanup-into-bottom-half.patch
pve/0030-PVE-monitor-disable-oob-capability.patch
pve/0031-PVE-bug-fix-1071-vma-writer.c-use-correct-AioContext.patch
pve/0032-qmp_backup-run-backup-related-code-inside-coroutines.patch
pve/0033-qmp_backup-use-a-CoMutex-to-protect-access-to-backup.patch
pve/0034-vma_writer_close-avoid-call-to-aio_poll-acquire-flus.patch