[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate th
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle |
Date: |
Wed, 19 Dec 2018 10:29:41 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
David Gibson <address@hidden> writes:
> On Thu, 13 Dec 2018 08:26:48 +0100
> Markus Armbruster <address@hidden> 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.
>>
>> Still missing from the commit message then: how you update the callers.
>> Let's see below.
>
> [snip]
>> > --- a/hw/ppc/spapr_hcall.c
>> > +++ b/hw/ppc/spapr_hcall.c
>> > @@ -478,6 +478,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU
>> > *cpu,
>> > sPAPRPendingHPT *pending = spapr->pending_hpt;
>> > uint64_t current_ram_size;
>> > int rc;
>> > + Error *local_err = NULL;
>> >
>> > if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) {
>> > return H_AUTHORITY;
>> > @@ -538,8 +539,13 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU
>> > *cpu,
>> > pending->shift = shift;
>> > pending->ret = H_HARDWARE;
>> >
>> > - qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
>> > - hpt_prepare_thread, pending, QEMU_THREAD_DETACHED);
>> > + if (!qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
>> > + hpt_prepare_thread, pending,
>> > + QEMU_THREAD_DETACHED, &local_err)) {
>> > + error_reportf_err(local_err, "failed to create
>> > hpt_prepare_thread: ");
>> > + g_free(pending);
>> > + return H_RESOURCE;
>> > + }
>> >
>> > spapr->pending_hpt = pending;
>> >
>>
>> This is a caller that returns an error code on failure. You change it
>> to report the error, then return failure. The return failure part looks
>> fine. Whether reporting the error is appropriate I can't say for sure.
>> No other failure mode reports anything. David, what do you think?
>
> I think it's reasonable here. In this context error returns and
> reported errors are for different audiences. The error returns are for
> the guest, the reported errors are for the guest administrator or
> management layers. This particularly failure is essentially a host
> side fault that is mostly relevant to the VM management. We have to
> say *something* to the guest to explain that the action couldn't go
> forward and H_RESOURCE makes as much sense as anything.
Double-checking: is it okay to report some failures of this function
(one of two H_RESOURCE failures, to be precise), but not others?
- [Qemu-devel] [PATCH for-4.0 v8 2/7] qemu_init_vcpu: add a new Error parameter to propagate, (continued)
[Qemu-devel] [PATCH for-4.0 v8 4/7] migration: remove unused &local_err parameter in multifd_save_cleanup, Fei Li, 2018/12/11
[Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle, Fei Li, 2018/12/11
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle, Markus Armbruster, 2018/12/19
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle, Fei Li, 2018/12/19
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle, Eric Blake, 2018/12/19
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle, Fei Li, 2018/12/19
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle, Fei Li, 2018/12/21
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle, Peter Xu, 2018/12/23
Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle, Fei Li, 2018/12/24