[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] blockjob: report a better error message

From: Stefano Garzarella
Subject: Re: [PATCH] blockjob: report a better error message
Date: Wed, 24 Feb 2021 16:59:25 +0100

On Wed, Feb 24, 2021 at 03:36:20PM +0100, Kevin Wolf wrote:
Am 23.02.2021 um 14:11 hat Stefano Garzarella geschrieben:
When a block job fails, we report 'strerror(-job->job.ret)' error
message, also if the job set an error object.
Let's report a better error message using 'error_get_pretty(job->job.err)'.

If an error object was not set, strerror(-job->ret) is used as fallback,
as explained in include/qemu/job.h:

typedef struct Job {
     * 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;

This is true, but there is a short time where job->ret is already set,
but not turned into job->err yet if necessary. The latter is done in a
bottom half scheduled after the former has happened.

It doesn't matter for block_job_event_completed(), which is called only
after both, but block_job_query() could in theory be called in this

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
 blockjob.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index f2feff051d..a696f3408d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -319,7 +319,8 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
     info->auto_finalize = job->job.auto_finalize;
     info->auto_dismiss  = job->job.auto_dismiss;
     info->has_error = job->job.ret != 0;
-    info->error     = job->job.ret ? g_strdup(strerror(-job->job.ret)) : NULL;
+    info->error     = job->job.ret ?
+                        g_strdup(error_get_pretty(job->job.err)) : NULL;

So I think we can't rely on job->job.err being non-NULL here.

Do you think is better to leave it as it was or do something like this?

    if (job->job.ret) {
        info->has_error = true;
        info->error = job->job.err ? g_strdup(error_get_pretty(job->job.err)) :


     return info;

@@ -356,7 +357,7 @@ static void block_job_event_completed(Notifier *n, void 

     if (job->job.ret < 0) {
-        msg = strerror(-job->job.ret);
+        msg = error_get_pretty(job->job.err);



reply via email to

[Prev in Thread] Current Thread [Next in Thread]