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

[Fei Li]

在 2019/1/14 下午8:36, Markus Armbruster 写道:
> Fei Li <address@hidden> writes:
>
> > 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.
Sorry that I am still uncertain.. I guess the code below I pasted was 
misleading,
actually I want to stress the *dc->unrealize() is not called when 
dc->realize() fails*
and the incomplete below "goto fail" does not include the dc->unrealize(),
but instead the dc->unrealize() is included in later child_realize_fail: 
& post_realize_fail:.


Emm, IMHO, I think when dc->realize() fails, the dc->unrealize() is 
either should be
called in the common function: device_set_realized() in a unified way, 
that is

        if (local_err != NULL) {
+          if (dc->unrealize) {
+              dc->unrealize(dev, local_err);
+          }
            goto fail;
        }

or do the unrealize() locally for each device earlier when dc->realize() 
fails.
But I checked several dc->realize() function, they did not call unrealize()
when fails. Besides, it may mean verbose code if unrealize() locally.
Thus I think the above way is the right way to do the cleanup when 
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.
> Yes, this is on purpose.
>
> When a realize() method fails, it must revert everything it has done so
> far.  Results in sane "either succeed completely, or fail and do
> nothing" semantics.

Have a nice day, thanks

Fei