From nobody Sun Nov 2 11:43:25 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1527626515756301.5284936600058; Tue, 29 May 2018 13:41:55 -0700 (PDT) Received: from localhost ([::1]:34935 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNlRK-0004vz-RJ for importer@patchew.org; Tue, 29 May 2018 16:41:54 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51738) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fNlOx-0003HQ-AX for qemu-devel@nongnu.org; Tue, 29 May 2018 16:39:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fNlOv-0007tX-TA for qemu-devel@nongnu.org; Tue, 29 May 2018 16:39:27 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:39378 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fNlOs-0007pW-Jy; Tue, 29 May 2018 16:39:22 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2DFC27D83F; Tue, 29 May 2018 20:39:22 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-193.ams2.redhat.com [10.36.116.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id D3CBC63F21; Tue, 29 May 2018 20:39:20 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 29 May 2018 22:38:57 +0200 Message-Id: <20180529203910.7615-4-kwolf@redhat.com> In-Reply-To: <20180529203910.7615-1-kwolf@redhat.com> References: <20180529203910.7615-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 29 May 2018 20:39:22 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Tue, 29 May 2018 20:39:22 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v2 03/16] job: Add error message for failing jobs X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, pkrempa@redhat.com, jsnow@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" So far we relied on job->ret and strerror() to produce an error message for failed jobs. Not surprisingly, this tends to result in completely useless messages. This adds a Job.error field that can contain an error string for a failing job, and a parameter to job_completed() that sets the field. As a default, if NULL is passed, we continue to use strerror(job->ret). All existing callers are changed to pass NULL. They can be improved in separate patches. Signed-off-by: Kevin Wolf Reviewed-by: Jeff Cody Reviewed-by: Max Reitz --- include/qemu/job.h | 7 ++++++- block/backup.c | 2 +- block/commit.c | 2 +- block/mirror.c | 2 +- block/stream.c | 2 +- job-qmp.c | 9 ++------- job.c | 16 ++++++++++++++-- tests/test-bdrv-drain.c | 2 +- tests/test-blockjob-txn.c | 2 +- tests/test-blockjob.c | 2 +- 10 files changed, 29 insertions(+), 17 deletions(-) diff --git a/include/qemu/job.h b/include/qemu/job.h index 8c8badf75e..1d820530fa 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -124,6 +124,9 @@ typedef struct Job { /** Estimated progress_current value at the completion of the job */ int64_t progress_total; =20 + /** Error string for a failed job (NULL if, and only if, job->ret =3D= =3D 0) */ + char *error; + /** ret code passed to job_completed. */ int ret; =20 @@ -466,13 +469,15 @@ void job_transition_to_ready(Job *job); /** * @job: The job being completed. * @ret: The status code. + * @error: The error message for a failing job (only with @ret < 0). If @r= et is + * negative, but NULL is given for @error, strerror() is used. * * Marks @job as completed. If @ret is non-zero, the job transaction it is= part * of is aborted. If @ret is zero, the job moves into the WAITING state. I= f it * is the last job to complete in its transaction, all jobs in the transac= tion * move from WAITING to PENDING. */ -void job_completed(Job *job, int ret); +void job_completed(Job *job, int ret, Error *error); =20 /** Asynchronously complete the specified @job. */ void job_complete(Job *job, Error **errp); diff --git a/block/backup.c b/block/backup.c index 4e228e959b..5661435675 100644 --- a/block/backup.c +++ b/block/backup.c @@ -321,7 +321,7 @@ static void backup_complete(Job *job, void *opaque) { BackupCompleteData *data =3D opaque; =20 - job_completed(job, data->ret); + job_completed(job, data->ret, NULL); g_free(data); } =20 diff --git a/block/commit.c b/block/commit.c index 620666161b..e1814d9693 100644 --- a/block/commit.c +++ b/block/commit.c @@ -117,7 +117,7 @@ static void commit_complete(Job *job, void *opaque) * bdrv_set_backing_hd() to fail. */ block_job_remove_all_bdrv(bjob); =20 - job_completed(job, ret); + job_completed(job, ret, NULL); g_free(data); =20 /* If bdrv_drop_intermediate() didn't already do that, remove the comm= it diff --git a/block/mirror.c b/block/mirror.c index dcb66ec3be..435268bbbf 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -581,7 +581,7 @@ static void mirror_exit(Job *job, void *opaque) blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort); blk_insert_bs(bjob->blk, mirror_top_bs, &error_abort); =20 - job_completed(job, data->ret); + job_completed(job, data->ret, NULL); =20 g_free(data); bdrv_drained_end(src); diff --git a/block/stream.c b/block/stream.c index a5d6e0cf8a..9264b68a1e 100644 --- a/block/stream.c +++ b/block/stream.c @@ -93,7 +93,7 @@ out: } =20 g_free(s->backing_file_str); - job_completed(job, data->ret); + job_completed(job, data->ret, NULL); g_free(data); } =20 diff --git a/job-qmp.c b/job-qmp.c index 7f38f63336..410775df61 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -136,14 +136,9 @@ void qmp_job_dismiss(const char *id, Error **errp) static JobInfo *job_query_single(Job *job, Error **errp) { JobInfo *info; - const char *errmsg =3D NULL; =20 assert(!job_is_internal(job)); =20 - if (job->ret < 0) { - errmsg =3D strerror(-job->ret); - } - info =3D g_new(JobInfo, 1); *info =3D (JobInfo) { .id =3D g_strdup(job->id), @@ -151,8 +146,8 @@ static JobInfo *job_query_single(Job *job, Error **errp) .status =3D job->status, .current_progress =3D job->progress_current, .total_progress =3D job->progress_total, - .has_error =3D !!errmsg, - .error =3D g_strdup(errmsg), + .has_error =3D !!job->error, + .error =3D g_strdup(job->error), }; =20 return info; diff --git a/job.c b/job.c index f026661b0f..84e140238b 100644 --- a/job.c +++ b/job.c @@ -369,6 +369,7 @@ void job_unref(Job *job) =20 QLIST_REMOVE(job, job_list); =20 + g_free(job->error); g_free(job->id); g_free(job); } @@ -660,6 +661,9 @@ static void job_update_rc(Job *job) job->ret =3D -ECANCELED; } if (job->ret) { + if (!job->error) { + job->error =3D g_strdup(strerror(-job->ret)); + } job_state_transition(job, JOB_STATUS_ABORTING); } } @@ -782,6 +786,7 @@ static int job_prepare(Job *job) { if (job->ret =3D=3D 0 && job->driver->prepare) { job->ret =3D job->driver->prepare(job); + job_update_rc(job); } return job->ret; } @@ -855,10 +860,17 @@ static void job_completed_txn_success(Job *job) } } =20 -void job_completed(Job *job, int ret) +void job_completed(Job *job, int ret, Error *error) { assert(job && job->txn && !job_is_completed(job)); + job->ret =3D ret; + if (error) { + assert(job->ret < 0); + job->error =3D g_strdup(error_get_pretty(error)); + error_free(error); + } + job_update_rc(job); trace_job_completed(job, ret, job->ret); if (job->ret) { @@ -876,7 +888,7 @@ void job_cancel(Job *job, bool force) } job_cancel_async(job, force); if (!job_started(job)) { - job_completed(job, -ECANCELED); + job_completed(job, -ECANCELED, NULL); } else if (job->deferred_to_main_loop) { job_completed_txn_abort(job); } else { diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 2cba63b881..a11c4cfbf2 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -498,7 +498,7 @@ typedef struct TestBlockJob { =20 static void test_job_completed(Job *job, void *opaque) { - job_completed(job, 0); + job_completed(job, 0, NULL); } =20 static void coroutine_fn test_job_start(void *opaque) diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index fce836639a..58d9b87fb2 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -34,7 +34,7 @@ static void test_block_job_complete(Job *job, void *opaqu= e) rc =3D -ECANCELED; } =20 - job_completed(job, rc); + job_completed(job, rc, NULL); bdrv_unref(bs); } =20 diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index e408d52351..cb42f06e61 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -167,7 +167,7 @@ static void cancel_job_completed(Job *job, void *opaque) { CancelJob *s =3D opaque; s->completed =3D true; - job_completed(job, 0); + job_completed(job, 0, NULL); } =20 static void cancel_job_complete(Job *job, Error **errp) --=20 2.13.6