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

[Fei Li]

在 2019/1/8 上午1:29, Markus Armbruster 写道:
> Fei Li <address@hidden> writes:
>
> > Utilize the existed errp to propagate the error instead of the
> > temporary &error_abort.
> >
> > Cc: Markus Armbruster <address@hidden>
> > Cc: Jiri Slaby <address@hidden>
> > Signed-off-by: Fei Li <address@hidden>
> > ---
> >   hw/misc/edu.c | 7 ++++---
> >   1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> > index 3f4ba7ded3..011fe6e0b7 100644
> > --- a/hw/misc/edu.c
> > +++ b/hw/misc/edu.c
> > @@ -356,9 +356,10 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
> >   
> >       qemu_mutex_init(&edu->thr_mutex);
> >       qemu_cond_init(&edu->thr_cond);
> > -    /* TODO: let the further caller handle the error instead of abort() here */
> > -    qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
> > -                       edu, QEMU_THREAD_JOINABLE, &error_abort);
> > +    if (!qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
> > +                            edu, QEMU_THREAD_JOINABLE, errp)) {
> > +        return;
> You need to clean up everything that got initialized so far.  You might
> want to call qemu_thread_create() earlier so you have less to clean up.

Just to make sure about how to do the cleanup. I notice that in 
device_set_realized(),
the current code does not call "dc->unrealize(dev, NULL);" when dc->realize() 
fails.

        if (dc->realize) {
            dc->realize(dev, &local_err);
        }

        if (local_err != NULL) {
            goto fail;
        }

Is this on purpose? (Maybe due to some devices' realize() do their own cleanup
when fails? Sorry for the unsure, it is such a common function that I did not
check all. :( ) Or else, I prefer to do the cleanup in a unified manner, e.g. call 
"dc->unrealize(dev, NULL);" which is the pci_qdev_unrealize() for pci devices.

Have a nice day
Fei