[pve-devel] [PATCH] PVE: fix and clean up error handling for create_backup_jobs
Stefan Reiter
s.reiter at proxmox.com
Thu Oct 22 17:13:07 CEST 2020
No more weird bool returns, just the standard "errp" format used
everywhere else too. With this, if backup_job_create fails, the error
message is actually returned over QMP and can be shown to the user.
Also add a job_cancel_sync before job_unref, since a job must be in
STATUS_NULL to be deleted by unref, which could trigger an assert
before.
Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---
Another small fix, which went under the radar since testing error handling is
obviously unnecessary considering our code is perfect and never fails ;)
pve-backup.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/pve-backup.c b/pve-backup.c
index af2db0d4b9..1a013afd89 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -450,7 +450,7 @@ static int coroutine_fn pvebackup_co_add_config(
* struct and starting the jobs, and from within a QMP call. This means that no
* other QMP call can interrupt, and no background job is running yet.
*/
-static bool create_backup_jobs(void) {
+static void create_backup_jobs(Error **errp) {
assert(!qemu_in_coroutine());
@@ -487,12 +487,9 @@ static bool create_backup_jobs(void) {
aio_context_release(aio_context);
- if (!job || local_err != NULL) {
- Error *create_job_err = NULL;
- error_setg(&create_job_err, "backup_job_create failed: %s",
+ if (!job || *errp != NULL) {
+ error_setg(errp, "backup_job_create failed: %s",
local_err ? error_get_pretty(local_err) : "null");
-
- pvebackup_propagate_error(create_job_err);
break;
}
@@ -502,9 +499,7 @@ static bool create_backup_jobs(void) {
di->target = NULL;
}
- bool errors = pvebackup_error_or_canceled();
-
- if (errors) {
+ if (*errp) {
l = backup_state.di_list;
while (l) {
PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
@@ -516,12 +511,14 @@ static bool create_backup_jobs(void) {
}
if (di->job) {
+ AioContext *ctx = di->job->job.aio_context;
+ aio_context_acquire(ctx);
+ job_cancel_sync(&di->job->job);
job_unref(&di->job->job);
+ aio_context_release(ctx);
}
}
}
-
- return errors;
}
typedef struct QmpBackupTask {
@@ -1002,9 +999,9 @@ UuidInfo *qmp_backup(
block_on_coroutine_fn(pvebackup_co_prepare, &task);
if (*errp == NULL) {
- bool errors = create_backup_jobs();
+ create_backup_jobs(errp);
- if (!errors) {
+ if (*errp == NULL) {
// start the first job in the transaction
job_txn_start_seq(backup_state.txn);
}
--
2.20.1
More information about the pve-devel
mailing list