From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A170461D28 for ; Thu, 22 Oct 2020 17:13:16 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 982F121268 for ; Thu, 22 Oct 2020 17:13:16 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 12D762125F for ; Thu, 22 Oct 2020 17:13:16 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 84FFF45ED4 for ; Thu, 22 Oct 2020 17:13:15 +0200 (CEST) From: Stefan Reiter To: pve-devel@lists.proxmox.com Date: Thu, 22 Oct 2020 17:13:07 +0200 Message-Id: <20201022151307.19447-1-s.reiter@proxmox.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.031 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH] PVE: fix and clean up error handling for create_backup_jobs X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Oct 2020 15:13:16 -0000 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 --- 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