qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: David Gibson
Subject: Re: [Qemu-devel] [PATCH for-4.0 v9 10/16] qemu_thread: supplement error handling for h_resize_hpt_prepare
Date: Thu, 3 Jan 2019 14:43:06 +1100
User-agent: Mutt/1.10.1 (2018-07-13)

On Wed, Jan 02, 2019 at 02:44:17PM +0800, 李菲 wrote:
> 
> 在 2019/1/2 上午10:36, David Gibson 写道:
> > 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 <address@hidden>
> > > Cc: David Gibson <address@hidden>
> > > Signed-off-by: Fei Li <address@hidden>
> > This looks like a good change, but it no longer applies due to a
> > change in the qemu_thread_create() signature.
> Sorry that I am not sure whether I understand. Do you mean using
> &error_abort is more suitable for this handling, rather than report
> the &local_err & return a failure reason?

No, I just mean that context has been altered by a global change and
the patch will need to be fixed up to cope with that.

> > 
> > > ---
> > >   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.
> 
> Ok, thanks. Will use H_HARDWARE instead.
> 
> Have a nice day, thanks for the review. :)
> Fei
> > 
> > > +    }
> > >       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: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]