From patchwork Fri May 25 16:33:16 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 10427959 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B2F20602D8 for ; Fri, 25 May 2018 16:37:16 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AD7B22974F for ; Fri, 25 May 2018 16:37:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A109329777; Fri, 25 May 2018 16:37:16 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 111172974F for ; Fri, 25 May 2018 16:37:16 +0000 (UTC) Received: from localhost ([::1]:45735 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fMFiN-0002kH-64 for patchwork-qemu-devel@patchwork.kernel.org; Fri, 25 May 2018 12:37:15 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fMFfG-0000pt-7V for qemu-devel@nongnu.org; Fri, 25 May 2018 12:34:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fMFfE-0002mq-UG for qemu-devel@nongnu.org; Fri, 25 May 2018 12:34:02 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:45220 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 1fMFfB-0002jL-TK; Fri, 25 May 2018 12:33:58 -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 7E63E805A52F; Fri, 25 May 2018 16:33:57 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-39.ams2.redhat.com [10.36.117.39]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4FA286353C; Fri, 25 May 2018 16:33:56 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Fri, 25 May 2018 18:33:16 +0200 Message-Id: <20180525163327.23097-4-kwolf@redhat.com> In-Reply-To: <20180525163327.23097-1-kwolf@redhat.com> References: <20180525163327.23097-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.8]); Fri, 25 May 2018 16:33:57 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 25 May 2018 16:33:57 +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 03/14] 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, qemu-devel@nongnu.org, mreitz@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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 | 15 +++++++++++++-- 7 files changed, 25 insertions(+), 14 deletions(-) diff --git a/include/qemu/job.h b/include/qemu/job.h index 8c8badf75e..b2e1dd00b9 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; + /** Error string for a failed job (NULL if job->ret == 0) */ + char *error; + /** ret code passed to job_completed. */ int ret; @@ -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 @ret 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. If it * is the last job to complete in its transaction, all jobs in the transaction * move from WAITING to PENDING. */ -void job_completed(Job *job, int ret); +void job_completed(Job *job, int ret, Error *error); /** 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 = opaque; - job_completed(job, data->ret); + job_completed(job, data->ret, NULL); g_free(data); } 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); - job_completed(job, ret); + job_completed(job, ret, NULL); g_free(data); /* If bdrv_drop_intermediate() didn't already do that, remove the commit 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); - job_completed(job, data->ret); + job_completed(job, data->ret, NULL); 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: } g_free(s->backing_file_str); - job_completed(job, data->ret); + job_completed(job, data->ret, NULL); g_free(data); } 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 = NULL; assert(!job_is_internal(job)); - if (job->ret < 0) { - errmsg = strerror(-job->ret); - } - info = g_new(JobInfo, 1); *info = (JobInfo) { .id = g_strdup(job->id), @@ -151,8 +146,8 @@ static JobInfo *job_query_single(Job *job, Error **errp) .status = job->status, .current_progress = job->progress_current, .total_progress = job->progress_total, - .has_error = !!errmsg, - .error = g_strdup(errmsg), + .has_error = !!job->error, + .error = g_strdup(job->error), }; return info; diff --git a/job.c b/job.c index f026661b0f..fc39eaaa5e 100644 --- a/job.c +++ b/job.c @@ -369,6 +369,7 @@ void job_unref(Job *job) QLIST_REMOVE(job, job_list); + g_free(job->error); g_free(job->id); g_free(job); } @@ -661,6 +662,9 @@ static void job_update_rc(Job *job) } if (job->ret) { job_state_transition(job, JOB_STATUS_ABORTING); + if (!job->error) { + job->error = g_strdup(strerror(-job->ret)); + } } } @@ -855,10 +859,17 @@ static void job_completed_txn_success(Job *job) } } -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 = ret; + if (error) { + assert(job->ret < 0); + job->error = 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 +887,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 {