Re: [Qemu-devel] [PATCH 23/25] qom: add realized property

[Paolo Bonzini]

Il 05/04/2012 15:31, Andreas Färber ha scritto:
> Here's how I see it:
> 
> * You add a realize callback to ObjectClass like I did, you add the
> Error** parameter that was requested as feedback to mine.
> * You add a static object_realize() method that clashes with my
> introducing it as a public wrapper function.
> * You introduce a function object_get_realized() like I did, only you
> defer your implementation to object_is_realized() which I didn't have
> and used a new bool realized instead of a state enum (since I left qdev
> unmodified).
> * You introduce a function object_set_realized() like I did, only you
> change the logic to also do unrealize.
> * You introduce additional stuff that I don't particularly care about.

Since we're nitpicking, I also do correct error propagation.

> So my point is, whether you've read some patch or not, I just can't
> understand why you couldn't wait a week for me to resend the updated
> version

Because a week is a long time 10 days before the feature freeze, and
(via object_is_realized and a few other small bits) the whole series
depends on the implementation of realized.

> While having unrealize and propagation is certainly nice, the most
> serious issue with yours I see is that it doesn't offer me a way to
> actually make use of it outside qdev, so that *I* am left with no
> benefit from your patch!

Can you explain?  I definitely would need to fix this.

> Some practical thoughts on how to align both approaches would be helpful
> here. For starters, should I name my function object_realize_nofail()
> instead?

Yes, that would be an idea.  I would hope that long-term there would be
only one object_realize call during in initial machine creation (i.e.
except for hot-plug), but it would be fine as a start.

> And could you prefer _one over _1 in your patch please?

Yes.

> If your problem is Signed-off-by specifically, feel free to invent some
> inofficial tag such as Inspired-by or Derived-from-commit-message-by or
> resort to a textual reference.

I can add the SoB, no problem.

Paolo