From dfed24e446a733eeadf0ee85f3ce8aec63b2b3bf Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 25 Sep 2018 22:34:08 +0100 Subject: [PATCH 05/28] jobs: canonize Error object RH-Author: John Snow Message-id: <20180925223431.24791-3-jsnow@redhat.com> Patchwork-id: 82262 O-Subject: [RHEL8/rhel qemu-kvm PATCH 02/25] jobs: canonize Error object Bugzilla: 1632939 RH-Acked-by: Jeffrey Cody RH-Acked-by: Max Reitz RH-Acked-by: Kevin Wolf Jobs presently use both an Error object in the case of the create job, and char strings in the case of generic errors elsewhere. Unify the two paths as just j->err, and remove the extra argument from job_completed. The integer error code for job_completed is kept for now, to be removed shortly in a separate patch. Signed-off-by: John Snow Message-id: 20180830015734.19765-3-jsnow@redhat.com [mreitz: Dropped a superfluous g_strdup()] Reviewed-by: Eric Blake Signed-off-by: Max Reitz (cherry picked from commit 3d1f8b07a4c241f81949eff507d9f3a8fd73b87b) Signed-off-by: John Snow Signed-off-by: Danilo C. L. de Paula --- block/backup.c | 2 +- block/commit.c | 2 +- block/create.c | 5 ++--- block/mirror.c | 2 +- block/stream.c | 2 +- include/qemu/job.h | 14 ++++++++------ job-qmp.c | 5 +++-- job.c | 18 ++++++------------ tests/test-bdrv-drain.c | 2 +- tests/test-blockjob-txn.c | 2 +- tests/test-blockjob.c | 2 +- 11 files changed, 26 insertions(+), 30 deletions(-) diff --git a/block/backup.c b/block/backup.c index a142518..a5bf699 100644 --- a/block/backup.c +++ b/block/backup.c @@ -388,7 +388,7 @@ static void backup_complete(Job *job, void *opaque) { BackupCompleteData *data = opaque; - job_completed(job, data->ret, NULL); + job_completed(job, data->ret); g_free(data); } diff --git a/block/commit.c b/block/commit.c index 905a1c5..af7579d 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, NULL); + job_completed(job, ret); g_free(data); /* If bdrv_drop_intermediate() didn't already do that, remove the commit diff --git a/block/create.c b/block/create.c index 04733c3..26a385c 100644 --- a/block/create.c +++ b/block/create.c @@ -35,14 +35,13 @@ typedef struct BlockdevCreateJob { BlockDriver *drv; BlockdevCreateOptions *opts; int ret; - Error *err; } BlockdevCreateJob; static void blockdev_create_complete(Job *job, void *opaque) { BlockdevCreateJob *s = container_of(job, BlockdevCreateJob, common); - job_completed(job, s->ret, s->err); + job_completed(job, s->ret); } static int coroutine_fn blockdev_create_run(Job *job, Error **errp) @@ -50,7 +49,7 @@ static int coroutine_fn blockdev_create_run(Job *job, Error **errp) BlockdevCreateJob *s = container_of(job, BlockdevCreateJob, common); job_progress_set_remaining(&s->common, 1); - s->ret = s->drv->bdrv_co_create(s->opts, &s->err); + s->ret = s->drv->bdrv_co_create(s->opts, errp); job_progress_update(&s->common, 1); qapi_free_BlockdevCreateOptions(s->opts); diff --git a/block/mirror.c b/block/mirror.c index 89a92c2..3c330ee 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, NULL); + job_completed(job, data->ret); g_free(data); bdrv_drained_end(src); diff --git a/block/stream.c b/block/stream.c index b4b987d..26a7753 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, NULL); + job_completed(job, data->ret); g_free(data); } diff --git a/include/qemu/job.h b/include/qemu/job.h index e81cc34..905dfdd 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -124,12 +124,16 @@ 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, and only if, job->ret == 0) */ - char *error; - /** ret code passed to job_completed. */ int ret; + /** + * Error object for a failed job. + * If job->ret is nonzero and an error object was not set, it will be set + * to strerror(-job->ret) during job_completed. + */ + Error *err; + /** The completion function that will be called when the job completes. */ BlockCompletionFunc *cb; @@ -469,15 +473,13 @@ 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, Error *error); +void job_completed(Job *job, int ret); /** Asynchronously complete the specified @job. */ void job_complete(Job *job, Error **errp); diff --git a/job-qmp.c b/job-qmp.c index 410775d..a969b2b 100644 --- a/job-qmp.c +++ b/job-qmp.c @@ -146,8 +146,9 @@ static JobInfo *job_query_single(Job *job, Error **errp) .status = job->status, .current_progress = job->progress_current, .total_progress = job->progress_total, - .has_error = !!job->error, - .error = g_strdup(job->error), + .has_error = !!job->err, + .error = job->err ? \ + g_strdup(error_get_pretty(job->err)) : NULL, }; return info; diff --git a/job.c b/job.c index d4e3041..17b4fad 100644 --- a/job.c +++ b/job.c @@ -369,7 +369,7 @@ void job_unref(Job *job) QLIST_REMOVE(job, job_list); - g_free(job->error); + error_free(job->err); g_free(job->id); g_free(job); } @@ -541,7 +541,7 @@ static void coroutine_fn job_co_entry(void *opaque) assert(job && job->driver && job->driver->run); job_pause_point(job); - job->ret = job->driver->run(job, NULL); + job->ret = job->driver->run(job, &job->err); } @@ -661,8 +661,8 @@ static void job_update_rc(Job *job) job->ret = -ECANCELED; } if (job->ret) { - if (!job->error) { - job->error = g_strdup(strerror(-job->ret)); + if (!job->err) { + error_setg(&job->err, "%s", strerror(-job->ret)); } job_state_transition(job, JOB_STATUS_ABORTING); } @@ -860,17 +860,11 @@ static void job_completed_txn_success(Job *job) } } -void job_completed(Job *job, int ret, Error *error) +void job_completed(Job *job, int ret) { 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) { @@ -888,7 +882,7 @@ void job_cancel(Job *job, bool force) } job_cancel_async(job, force); if (!job_started(job)) { - job_completed(job, -ECANCELED, NULL); + job_completed(job, -ECANCELED); } 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 798445a..8937894 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -498,7 +498,7 @@ typedef struct TestBlockJob { static void test_job_completed(Job *job, void *opaque) { - job_completed(job, 0, NULL); + job_completed(job, 0); } static int coroutine_fn test_job_run(Job *job, Error **errp) diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index 3194924..82cedee 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 *opaque) rc = -ECANCELED; } - job_completed(job, rc, NULL); + job_completed(job, rc); bdrv_unref(bs); } diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index b0462bf..408a226 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 = opaque; s->completed = true; - job_completed(job, 0, NULL); + job_completed(job, 0); } static void cancel_job_complete(Job *job, Error **errp) -- 1.8.3.1