From: Fei Li <fli@suse.com>
Utilize the existed errp to propagate the error and do the
corresponding cleanup to replace the temporary &error_abort.
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
---
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);
+ goto out2;
+ }
return;
--
2.17.2 (Apple Git-113)
Fei Li <lifei1214@126.com> writes:
> From: Fei Li <fli@suse.com>
>
> Utilize the existed errp to propagate the error and do the
> corresponding cleanup to replace the temporary &error_abort.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Fei Li <fli@suse.com>
> ---
> 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.
> + goto out2;
> + }
>
> return;
在 2019/2/1 下午9:04, Markus Armbruster 写道:
> Fei Li <lifei1214@126.com> writes:
>
>> From: Fei Li <fli@suse.com>
>>
>> Utilize the existed errp to propagate the error and do the
>> corresponding cleanup to replace the temporary &error_abort.
>>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Fei Li <fli@suse.com>
>> ---
>> 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()?
Have a nice day, thanks
Fei
>
>> + goto out2;
>> + }
>>
>> return;
Fei Li <lifei1214@126.com> writes:
> 在 2019/2/1 下午9:04, Markus Armbruster 写道:
>> Fei Li <lifei1214@126.com> writes:
>>
>>> From: Fei Li <fli@suse.com>
>>>
>>> Utilize the existed errp to propagate the error and do the
>>> corresponding cleanup to replace the temporary &error_abort.
>>>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> Signed-off-by: Fei Li <fli@suse.com>
>>> ---
>>> 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?
在 2019/2/4 下午9:30, Markus Armbruster 写道:
> Fei Li <lifei1214@126.com> writes:
>
>> 在 2019/2/1 下午9:04, Markus Armbruster 写道:
>>> Fei Li <lifei1214@126.com> writes:
>>>
>>>> From: Fei Li <fli@suse.com>
>>>>
>>>> Utilize the existed errp to propagate the error and do the
>>>> corresponding cleanup to replace the temporary &error_abort.
>>>>
>>>> Cc: Markus Armbruster <armbru@redhat.com>
>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>> Signed-off-by: Fei Li <fli@suse.com>
>>>> ---
>>>> 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?
Hi, Christophe, Marc-André,
Would you like to share your views and give some suggestions? :)
That would be very helpful, thanks a lot!
Have a nice day
Fei
在 2019/2/15 下午8:35, Fei Li 写道:
>
> 在 2019/2/4 下午9:30, Markus Armbruster 写道:
>> Fei Li <lifei1214@126.com> writes:
>>
>>> 在 2019/2/1 下午9:04, Markus Armbruster 写道:
>>>> Fei Li <lifei1214@126.com> writes:
>>>>
>>>>> From: Fei Li <fli@suse.com>
>>>>>
>>>>> Utilize the existed errp to propagate the error and do the
>>>>> corresponding cleanup to replace the temporary &error_abort.
>>>>>
>>>>> Cc: Markus Armbruster <armbru@redhat.com>
>>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>>> Signed-off-by: Fei Li <fli@suse.com>
>>>>> ---
>>>>> 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?
> Hi, Christophe, Marc-André,
> Would you like to share your views and give some suggestions? :)
> That would be very helpful, thanks a lot!
>
> Have a nice day
> Fei
>
Hi all,
I refer to the method in emulated_unrealize(): terminate event_thread
by stopping vevent thread [1]. But I am not sure whether this is proper,
please share your views in [PATCH v12 for-4.1 06/11]. Thanks a lot! :)
[1]
VEvent *vevent = vevent_new(VEVENT_LAST, NULL, NULL);
vevent_queue_vevent(vevent); /* stop vevent thread */
qemu_thread_join(&card->event_thread_id);
Have a nice day, thanks
Fei
© 2016 - 2026 Red Hat, Inc.