Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle

[Markus Armbruster]

Fei Li <address@hidden> writes:

> On 12/17/2018 03:29 PM, Fei Li wrote:
>>
>>
>> On 12/13/2018 03:26 PM, Markus Armbruster wrote:
>>> There's a question for David Gibson inline.  Please search for /ppc/.
>>>
>>> Fei Li <address@hidden> writes:
>>>
>>>> Make qemu_thread_create() return a Boolean to indicate if it succeeds
>>>> rather than failing with an error. And add an Error parameter to hold
>>>> the error message and let the callers handle it.
>>> The "rather than failing with an error" is misleading.  Before the
>>> patch, we report to stderr and abort().  What about:
>>>
>>>      qemu-thread: Make qemu_thread_create() handle errors properly
>>>
>>>      qemu_thread_create() abort()s on error.  Not nice.  Give it a
>>>      return value and an Error ** argument, so it can return success /
>>>      failure.
>> A nice commit-amend! Thanks!
>>> Still missing from the commit message then: how you update the callers.
>> Yes, agree. I think the-how should also be noted here, like
>> - propagating the err to callers whose call trace already have the
>> Error paramater;
>> - just add an &error_abort for qemu_thread_create() and make it a
>> "TODO: xxx";
>>> Let's see below.
> According to your below comment and suggestion, I make a summary for
> the second paragraph for the commit message, please help to review,
> thanks. :)
>
> /* ...The first paragraph and the middle blank... */
> And let's update qemu_thread_create()'s callers by
> - setting an error on qemu_thread_create() failure for callers that
>   set an error on failure;
> - reporting the error and returning failure for callers that return
>   an error code on failure;
> - reporting the error and setting some state for callers that just
>   report errors and choose not to continue on.
> - passing &error_abort for qemu_thread_create() for callers that
>   can't return failure, and marking a "TODO: " for further change.
>
> Have a nice day
> Fei

If you split the patch so that the first part makes all callers pass
&error_abort, the first part's commit message becomes much simpler, and
the subsequent parts' commit messages should be pretty simple to write,
too.