Re: [Qemu-devel] [PATCH for-4.0 v9 10/16] qemu_thread: supplement error handling for h_resize_hpt_prepare

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2314 bytes --]

On Tue, Dec 25, 2018 at 10:04:43PM +0800, Fei Li wrote:
> Add a local_err to hold the error, and return the corresponding
> error code to replace the temporary &error_abort.
> 
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Fei Li <fli@suse.com>

This looks like a good change, but it no longer applies due to a
change in the qemu_thread_create() signature.

> ---
>  hw/ppc/spapr_hcall.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 5bc2cf4540..7c16ade04a 100644
> --- 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,10 +539,13 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
>      pending->shift = shift;
>      pending->ret = H_HARDWARE;
>  
> -    /* TODO: let the further caller handle the error instead of abort() here */
> -    qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
> -                       hpt_prepare_thread, pending,
> -                       QEMU_THREAD_DETACHED, &error_abort);
> +    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;

I also think H_HARDWARE would be a better choice here.  Although the
failure is due to a resource constraint, it's not because the guest
asked for too much, just because the host is in dire straits.  From
the guest's point of view it's basically a hardware failure.

> +    }
>  
>      spapr->pending_hpt = pending;
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]