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 <kw...@redhat.com> > Signed-off-by: Stefano Garzarella <sgarz...@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