From 864c5b5a3146aac264065f9ef5cf397451747440 Mon Sep 17 00:00:00 2001

From: Kevin Wolf <kwolf@redhat.com>

Date: Tue, 26 Jun 2018 09:48:39 +0200

Subject: [PATCH 70/89] job: Add error message for failing jobs

RH-Author: Kevin Wolf <kwolf@redhat.com>

Message-id: <20180626094856.6924-57-kwolf@redhat.com>

Patchwork-id: 81110

O-Subject: [RHV-7.6 qemu-kvm-rhev PATCH v2 56/73] job: Add error message for failing jobs

Bugzilla: 1513543

RH-Acked-by: Jeffrey Cody <jcody@redhat.com>

RH-Acked-by: Max Reitz <mreitz@redhat.com>

RH-Acked-by: Fam Zheng <famz@redhat.com>

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 <kwolf@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Jeff Cody <jcody@redhat.com>

(cherry picked from commit 1266c9b9f5fa05877b979eece5963a2bd99c3bfd)

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 block/backup.c            |  2 +-

 block/commit.c            |  2 +-

 block/mirror.c            |  2 +-

 block/stream.c            |  2 +-

 include/qemu/job.h        |  7 ++++++-

 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/block/backup.c b/block/backup.c

index 4e228e9..5661435 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 6206661..e1814d9 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 dcb66ec..435268b 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 a5d6e0c..9264b68 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/include/qemu/job.h b/include/qemu/job.h

index 8c8badf..1d82053 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, and only 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/job-qmp.c b/job-qmp.c

index 7f38f63..410775d 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 f026661..84e1402 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);

     }

@@ -660,6 +661,9 @@ static void job_update_rc(Job *job)

         job->ret = -ECANCELED;

     }

     if (job->ret) {

+        if (!job->error) {

+            job->error = 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 == 0 && job->driver->prepare) {

         job->ret = job->driver->prepare(job);

+        job_update_rc(job);

     }

     return job->ret;

 }

@@ -855,10 +860,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 +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 2cba63b..a11c4cf 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);

+    job_completed(job, 0, NULL);

 }

 static void coroutine_fn test_job_start(void *opaque)

diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c

index fce8366..58d9b87 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);

+    job_completed(job, rc, NULL);

     bdrv_unref(bs);

 }

diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c

index e408d52..cb42f06 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);

+    job_completed(job, 0, NULL);

 }

 static void cancel_job_complete(Job *job, Error **errp)

-- 

1.8.3.1