|
|
1bdc94 |
From 864c5b5a3146aac264065f9ef5cf397451747440 Mon Sep 17 00:00:00 2001
|
|
|
1bdc94 |
From: Kevin Wolf <kwolf@redhat.com>
|
|
|
1bdc94 |
Date: Tue, 26 Jun 2018 09:48:39 +0200
|
|
|
1bdc94 |
Subject: [PATCH 70/89] job: Add error message for failing jobs
|
|
|
1bdc94 |
|
|
|
1bdc94 |
RH-Author: Kevin Wolf <kwolf@redhat.com>
|
|
|
1bdc94 |
Message-id: <20180626094856.6924-57-kwolf@redhat.com>
|
|
|
1bdc94 |
Patchwork-id: 81110
|
|
|
1bdc94 |
O-Subject: [RHV-7.6 qemu-kvm-rhev PATCH v2 56/73] job: Add error message for failing jobs
|
|
|
1bdc94 |
Bugzilla: 1513543
|
|
|
1bdc94 |
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
|
|
|
1bdc94 |
RH-Acked-by: Max Reitz <mreitz@redhat.com>
|
|
|
1bdc94 |
RH-Acked-by: Fam Zheng <famz@redhat.com>
|
|
|
1bdc94 |
|
|
|
1bdc94 |
So far we relied on job->ret and strerror() to produce an error message
|
|
|
1bdc94 |
for failed jobs. Not surprisingly, this tends to result in completely
|
|
|
1bdc94 |
useless messages.
|
|
|
1bdc94 |
|
|
|
1bdc94 |
This adds a Job.error field that can contain an error string for a
|
|
|
1bdc94 |
failing job, and a parameter to job_completed() that sets the field. As
|
|
|
1bdc94 |
a default, if NULL is passed, we continue to use strerror(job->ret).
|
|
|
1bdc94 |
|
|
|
1bdc94 |
All existing callers are changed to pass NULL. They can be improved in
|
|
|
1bdc94 |
separate patches.
|
|
|
1bdc94 |
|
|
|
1bdc94 |
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
|
1bdc94 |
Reviewed-by: Max Reitz <mreitz@redhat.com>
|
|
|
1bdc94 |
Reviewed-by: Jeff Cody <jcody@redhat.com>
|
|
|
1bdc94 |
(cherry picked from commit 1266c9b9f5fa05877b979eece5963a2bd99c3bfd)
|
|
|
1bdc94 |
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
|
1bdc94 |
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
1bdc94 |
---
|
|
|
1bdc94 |
block/backup.c | 2 +-
|
|
|
1bdc94 |
block/commit.c | 2 +-
|
|
|
1bdc94 |
block/mirror.c | 2 +-
|
|
|
1bdc94 |
block/stream.c | 2 +-
|
|
|
1bdc94 |
include/qemu/job.h | 7 ++++++-
|
|
|
1bdc94 |
job-qmp.c | 9 ++-------
|
|
|
1bdc94 |
job.c | 16 ++++++++++++++--
|
|
|
1bdc94 |
tests/test-bdrv-drain.c | 2 +-
|
|
|
1bdc94 |
tests/test-blockjob-txn.c | 2 +-
|
|
|
1bdc94 |
tests/test-blockjob.c | 2 +-
|
|
|
1bdc94 |
10 files changed, 29 insertions(+), 17 deletions(-)
|
|
|
1bdc94 |
|
|
|
1bdc94 |
diff --git a/block/backup.c b/block/backup.c
|
|
|
1bdc94 |
index 4e228e9..5661435 100644
|
|
|
1bdc94 |
--- a/block/backup.c
|
|
|
1bdc94 |
+++ b/block/backup.c
|
|
|
1bdc94 |
@@ -321,7 +321,7 @@ static void backup_complete(Job *job, void *opaque)
|
|
|
1bdc94 |
{
|
|
|
1bdc94 |
BackupCompleteData *data = opaque;
|
|
|
1bdc94 |
|
|
|
1bdc94 |
- job_completed(job, data->ret);
|
|
|
1bdc94 |
+ job_completed(job, data->ret, NULL);
|
|
|
1bdc94 |
g_free(data);
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
|
|
|
1bdc94 |
diff --git a/block/commit.c b/block/commit.c
|
|
|
1bdc94 |
index 6206661..e1814d9 100644
|
|
|
1bdc94 |
--- a/block/commit.c
|
|
|
1bdc94 |
+++ b/block/commit.c
|
|
|
1bdc94 |
@@ -117,7 +117,7 @@ static void commit_complete(Job *job, void *opaque)
|
|
|
1bdc94 |
* bdrv_set_backing_hd() to fail. */
|
|
|
1bdc94 |
block_job_remove_all_bdrv(bjob);
|
|
|
1bdc94 |
|
|
|
1bdc94 |
- job_completed(job, ret);
|
|
|
1bdc94 |
+ job_completed(job, ret, NULL);
|
|
|
1bdc94 |
g_free(data);
|
|
|
1bdc94 |
|
|
|
1bdc94 |
/* If bdrv_drop_intermediate() didn't already do that, remove the commit
|
|
|
1bdc94 |
diff --git a/block/mirror.c b/block/mirror.c
|
|
|
1bdc94 |
index dcb66ec..435268b 100644
|
|
|
1bdc94 |
--- a/block/mirror.c
|
|
|
1bdc94 |
+++ b/block/mirror.c
|
|
|
1bdc94 |
@@ -581,7 +581,7 @@ static void mirror_exit(Job *job, void *opaque)
|
|
|
1bdc94 |
blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort);
|
|
|
1bdc94 |
blk_insert_bs(bjob->blk, mirror_top_bs, &error_abort);
|
|
|
1bdc94 |
|
|
|
1bdc94 |
- job_completed(job, data->ret);
|
|
|
1bdc94 |
+ job_completed(job, data->ret, NULL);
|
|
|
1bdc94 |
|
|
|
1bdc94 |
g_free(data);
|
|
|
1bdc94 |
bdrv_drained_end(src);
|
|
|
1bdc94 |
diff --git a/block/stream.c b/block/stream.c
|
|
|
1bdc94 |
index a5d6e0c..9264b68 100644
|
|
|
1bdc94 |
--- a/block/stream.c
|
|
|
1bdc94 |
+++ b/block/stream.c
|
|
|
1bdc94 |
@@ -93,7 +93,7 @@ out:
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
|
|
|
1bdc94 |
g_free(s->backing_file_str);
|
|
|
1bdc94 |
- job_completed(job, data->ret);
|
|
|
1bdc94 |
+ job_completed(job, data->ret, NULL);
|
|
|
1bdc94 |
g_free(data);
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
|
|
|
1bdc94 |
diff --git a/include/qemu/job.h b/include/qemu/job.h
|
|
|
1bdc94 |
index 8c8badf..1d82053 100644
|
|
|
1bdc94 |
--- a/include/qemu/job.h
|
|
|
1bdc94 |
+++ b/include/qemu/job.h
|
|
|
1bdc94 |
@@ -124,6 +124,9 @@ typedef struct Job {
|
|
|
1bdc94 |
/** Estimated progress_current value at the completion of the job */
|
|
|
1bdc94 |
int64_t progress_total;
|
|
|
1bdc94 |
|
|
|
1bdc94 |
+ /** Error string for a failed job (NULL if, and only if, job->ret == 0) */
|
|
|
1bdc94 |
+ char *error;
|
|
|
1bdc94 |
+
|
|
|
1bdc94 |
/** ret code passed to job_completed. */
|
|
|
1bdc94 |
int ret;
|
|
|
1bdc94 |
|
|
|
1bdc94 |
@@ -466,13 +469,15 @@ void job_transition_to_ready(Job *job);
|
|
|
1bdc94 |
/**
|
|
|
1bdc94 |
* @job: The job being completed.
|
|
|
1bdc94 |
* @ret: The status code.
|
|
|
1bdc94 |
+ * @error: The error message for a failing job (only with @ret < 0). If @ret is
|
|
|
1bdc94 |
+ * negative, but NULL is given for @error, strerror() is used.
|
|
|
1bdc94 |
*
|
|
|
1bdc94 |
* Marks @job as completed. If @ret is non-zero, the job transaction it is part
|
|
|
1bdc94 |
* of is aborted. If @ret is zero, the job moves into the WAITING state. If it
|
|
|
1bdc94 |
* is the last job to complete in its transaction, all jobs in the transaction
|
|
|
1bdc94 |
* move from WAITING to PENDING.
|
|
|
1bdc94 |
*/
|
|
|
1bdc94 |
-void job_completed(Job *job, int ret);
|
|
|
1bdc94 |
+void job_completed(Job *job, int ret, Error *error);
|
|
|
1bdc94 |
|
|
|
1bdc94 |
/** Asynchronously complete the specified @job. */
|
|
|
1bdc94 |
void job_complete(Job *job, Error **errp);
|
|
|
1bdc94 |
diff --git a/job-qmp.c b/job-qmp.c
|
|
|
1bdc94 |
index 7f38f63..410775d 100644
|
|
|
1bdc94 |
--- a/job-qmp.c
|
|
|
1bdc94 |
+++ b/job-qmp.c
|
|
|
1bdc94 |
@@ -136,14 +136,9 @@ void qmp_job_dismiss(const char *id, Error **errp)
|
|
|
1bdc94 |
static JobInfo *job_query_single(Job *job, Error **errp)
|
|
|
1bdc94 |
{
|
|
|
1bdc94 |
JobInfo *info;
|
|
|
1bdc94 |
- const char *errmsg = NULL;
|
|
|
1bdc94 |
|
|
|
1bdc94 |
assert(!job_is_internal(job));
|
|
|
1bdc94 |
|
|
|
1bdc94 |
- if (job->ret < 0) {
|
|
|
1bdc94 |
- errmsg = strerror(-job->ret);
|
|
|
1bdc94 |
- }
|
|
|
1bdc94 |
-
|
|
|
1bdc94 |
info = g_new(JobInfo, 1);
|
|
|
1bdc94 |
*info = (JobInfo) {
|
|
|
1bdc94 |
.id = g_strdup(job->id),
|
|
|
1bdc94 |
@@ -151,8 +146,8 @@ static JobInfo *job_query_single(Job *job, Error **errp)
|
|
|
1bdc94 |
.status = job->status,
|
|
|
1bdc94 |
.current_progress = job->progress_current,
|
|
|
1bdc94 |
.total_progress = job->progress_total,
|
|
|
1bdc94 |
- .has_error = !!errmsg,
|
|
|
1bdc94 |
- .error = g_strdup(errmsg),
|
|
|
1bdc94 |
+ .has_error = !!job->error,
|
|
|
1bdc94 |
+ .error = g_strdup(job->error),
|
|
|
1bdc94 |
};
|
|
|
1bdc94 |
|
|
|
1bdc94 |
return info;
|
|
|
1bdc94 |
diff --git a/job.c b/job.c
|
|
|
1bdc94 |
index f026661..84e1402 100644
|
|
|
1bdc94 |
--- a/job.c
|
|
|
1bdc94 |
+++ b/job.c
|
|
|
1bdc94 |
@@ -369,6 +369,7 @@ void job_unref(Job *job)
|
|
|
1bdc94 |
|
|
|
1bdc94 |
QLIST_REMOVE(job, job_list);
|
|
|
1bdc94 |
|
|
|
1bdc94 |
+ g_free(job->error);
|
|
|
1bdc94 |
g_free(job->id);
|
|
|
1bdc94 |
g_free(job);
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
@@ -660,6 +661,9 @@ static void job_update_rc(Job *job)
|
|
|
1bdc94 |
job->ret = -ECANCELED;
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
if (job->ret) {
|
|
|
1bdc94 |
+ if (!job->error) {
|
|
|
1bdc94 |
+ job->error = g_strdup(strerror(-job->ret));
|
|
|
1bdc94 |
+ }
|
|
|
1bdc94 |
job_state_transition(job, JOB_STATUS_ABORTING);
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
@@ -782,6 +786,7 @@ static int job_prepare(Job *job)
|
|
|
1bdc94 |
{
|
|
|
1bdc94 |
if (job->ret == 0 && job->driver->prepare) {
|
|
|
1bdc94 |
job->ret = job->driver->prepare(job);
|
|
|
1bdc94 |
+ job_update_rc(job);
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
return job->ret;
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
@@ -855,10 +860,17 @@ static void job_completed_txn_success(Job *job)
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
|
|
|
1bdc94 |
-void job_completed(Job *job, int ret)
|
|
|
1bdc94 |
+void job_completed(Job *job, int ret, Error *error)
|
|
|
1bdc94 |
{
|
|
|
1bdc94 |
assert(job && job->txn && !job_is_completed(job));
|
|
|
1bdc94 |
+
|
|
|
1bdc94 |
job->ret = ret;
|
|
|
1bdc94 |
+ if (error) {
|
|
|
1bdc94 |
+ assert(job->ret < 0);
|
|
|
1bdc94 |
+ job->error = g_strdup(error_get_pretty(error));
|
|
|
1bdc94 |
+ error_free(error);
|
|
|
1bdc94 |
+ }
|
|
|
1bdc94 |
+
|
|
|
1bdc94 |
job_update_rc(job);
|
|
|
1bdc94 |
trace_job_completed(job, ret, job->ret);
|
|
|
1bdc94 |
if (job->ret) {
|
|
|
1bdc94 |
@@ -876,7 +888,7 @@ void job_cancel(Job *job, bool force)
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
job_cancel_async(job, force);
|
|
|
1bdc94 |
if (!job_started(job)) {
|
|
|
1bdc94 |
- job_completed(job, -ECANCELED);
|
|
|
1bdc94 |
+ job_completed(job, -ECANCELED, NULL);
|
|
|
1bdc94 |
} else if (job->deferred_to_main_loop) {
|
|
|
1bdc94 |
job_completed_txn_abort(job);
|
|
|
1bdc94 |
} else {
|
|
|
1bdc94 |
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
|
|
|
1bdc94 |
index 2cba63b..a11c4cf 100644
|
|
|
1bdc94 |
--- a/tests/test-bdrv-drain.c
|
|
|
1bdc94 |
+++ b/tests/test-bdrv-drain.c
|
|
|
1bdc94 |
@@ -498,7 +498,7 @@ typedef struct TestBlockJob {
|
|
|
1bdc94 |
|
|
|
1bdc94 |
static void test_job_completed(Job *job, void *opaque)
|
|
|
1bdc94 |
{
|
|
|
1bdc94 |
- job_completed(job, 0);
|
|
|
1bdc94 |
+ job_completed(job, 0, NULL);
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
|
|
|
1bdc94 |
static void coroutine_fn test_job_start(void *opaque)
|
|
|
1bdc94 |
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
|
|
|
1bdc94 |
index fce8366..58d9b87 100644
|
|
|
1bdc94 |
--- a/tests/test-blockjob-txn.c
|
|
|
1bdc94 |
+++ b/tests/test-blockjob-txn.c
|
|
|
1bdc94 |
@@ -34,7 +34,7 @@ static void test_block_job_complete(Job *job, void *opaque)
|
|
|
1bdc94 |
rc = -ECANCELED;
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
|
|
|
1bdc94 |
- job_completed(job, rc);
|
|
|
1bdc94 |
+ job_completed(job, rc, NULL);
|
|
|
1bdc94 |
bdrv_unref(bs);
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
|
|
|
1bdc94 |
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
|
|
|
1bdc94 |
index e408d52..cb42f06 100644
|
|
|
1bdc94 |
--- a/tests/test-blockjob.c
|
|
|
1bdc94 |
+++ b/tests/test-blockjob.c
|
|
|
1bdc94 |
@@ -167,7 +167,7 @@ static void cancel_job_completed(Job *job, void *opaque)
|
|
|
1bdc94 |
{
|
|
|
1bdc94 |
CancelJob *s = opaque;
|
|
|
1bdc94 |
s->completed = true;
|
|
|
1bdc94 |
- job_completed(job, 0);
|
|
|
1bdc94 |
+ job_completed(job, 0, NULL);
|
|
|
1bdc94 |
}
|
|
|
1bdc94 |
|
|
|
1bdc94 |
static void cancel_job_complete(Job *job, Error **errp)
|
|
|
1bdc94 |
--
|
|
|
1bdc94 |
1.8.3.1
|
|
|
1bdc94 |
|