[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] blockjob: report a better error message
From: |
Kevin Wolf |
Subject: |
Re: [PATCH] blockjob: report a better error message |
Date: |
Wed, 24 Feb 2021 15:36:20 +0100 |
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
window.
> 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.
> return info;
> }
>
> @@ -356,7 +357,7 @@ static void block_job_event_completed(Notifier *n, void
> *opaque)
> }
>
> if (job->job.ret < 0) {
> - msg = strerror(-job->job.ret);
> + msg = error_get_pretty(job->job.err);
> }
>
> qapi_event_send_block_job_completed(job_type(&job->job),
Kevin