Re: [Qemu-devel] [PATCH v11 for-4.0 06/11] qemu_thread: supplement error handling for emulated_realize

[Markus Armbruster]

Fei Li <address@hidden> writes:

> 在 2019/2/1 下午9:04, Markus Armbruster 写道:
>> Fei Li <address@hidden> writes:
>>
>>> From: Fei Li <address@hidden>
>>>
>>> Utilize the existed errp to propagate the error and do the
>>> corresponding cleanup to replace the temporary &error_abort.
>>>
>>> Cc: Markus Armbruster <address@hidden>
>>> Cc: Gerd Hoffmann <address@hidden>
>>> Signed-off-by: Fei Li <address@hidden>
>>> ---
>>>   hw/usb/ccid-card-emulated.c | 15 ++++++++++-----
>>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
>>> index 0b170f6328..19b4b9a8fa 100644
>>> --- a/hw/usb/ccid-card-emulated.c
>>> +++ b/hw/usb/ccid-card-emulated.c
>>> @@ -544,11 +544,16 @@ static void emulated_realize(CCIDCardState *base, 
>>> Error **errp)
>>>           error_setg(errp, "%s: failed to initialize vcard", 
>>> TYPE_EMULATED_CCID);
>>>           goto out2;
>>>       }
>>> -    /* TODO: let the further caller handle the error instead of abort() 
>>> here */
>>> -    qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
>>> -                       card, QEMU_THREAD_JOINABLE, &error_abort);
>>> -    qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", 
>>> handle_apdu_thread,
>>> -                       card, QEMU_THREAD_JOINABLE, &error_abort);
>>> +    if (qemu_thread_create(&card->event_thread_id, "ccid/event", 
>>> event_thread,
>>> +                           card, QEMU_THREAD_JOINABLE, errp) < 0) {
>>> +        goto out2;
>>> +    }
>>> +    if (qemu_thread_create(&card->apdu_thread_id, "ccid/apdu",
>>> +                           handle_apdu_thread, card,
>>> +                           QEMU_THREAD_JOINABLE, errp) < 0) {
>>> +        qemu_thread_join(&card->event_thread_id);
>> What makes event_thread terminate here?
>>
>> I'm asking because if it doesn't, the join will hang.
> Oops, my neglect..  Could we add a
> `qemu_thread_cancel(card->event_thread_id);` here
> before the join()?

pthread_cancel() is difficult to use correctly, and we don't use it in
QEMU so far.  Instead, we tell threads to stop, e.g. by setting a flag
the thread checks in its main loop, and making sure the thread actually
loops in bounded time.  How to best achieve that for this thread I don't
know.  Christophe, Marc-André, can you help?