Re: [Qemu-devel] [PATCH v2 1/2] hw/net/virtio-net: Move DEFINE_VIRTIO_NET_FEATURES to virtio-net

[Peter Maydell]

On 20 April 2015 at 15:08, Cornelia Huck <address@hidden> wrote:
> Hmm... isn't ->plugged() called after ->realize()?
>
> Maybe I'm just confused, so let's try to understand the callchain :)
>
> VirtIONetCcw is realized
>   -> feature bits are used
>   -> embedded VirtIODevice is realized
>   -> VirtioCcwDevice is realized
>      -> features are populated
>
> My understanding was that ->plugged() happened after step 3, but
> re-reading, it might already happen after step 2... very confusing.
> (This still would be too late for the feature bits, and we don't set up
> the parent bus before step 2.)

plugged gets called when the virtio backend device is realized
(from the hw/virtio/virtio.c base class realize method).
For virtio-ccw, your virtio_ccw_net_realize function does this
(by setting the 'realized' property on the backend vdev to true).
Since it does this before it calls virtio_ccw_device_realize()
you get the ordering:
 * virtio_ccw_net_realize early stuff
 * virtio-net backend realize
 * virtio_ccw plugged method called (if you had one)
 * virtio_ccw_device_realize called (manually by the subclass)

That's probably not a very helpful order...

> virtio-pci might be slightly different due to a different topology, I
> think.

virtio-pci has three differences:
 (1) its generic 'virtio_pci_realize' is a method on a common
base class, which then invokes the subclass realize
(rather than having the subclass realize call the parent
realize function as virtio-ccw does)
 (2) it implements a plugged method and a lot of work is done there
 (3) the virtio_net_pci_realize realizes the backend as its
final action, not in the middle of doing other things

So the order here is:
 * virtio_pci_realize (as base class realize method)
 * virtio_net_pci_realize
 * virtio-net backend realize
 * virtio_pci plugged method called

> I'm not opposed to moving setting up the features into ->plugged(), but
> I'm not sure it helps.

Conceptually I think if you have code which relies on the
backend existing, it is better placed in the plugged() method
rather than trying to implement the realize method as a sort
of two-stage thing with the backend-realize done in the middle
and manual calls from the subclass back into the base class
done afterwards.

You can probably fix the specific weirdnesses here by
being a bit more careful about what all the
virtio_ccw_net_realize &c functions do before realizing
the backend and what they do afterwards. But it might
be long term cleaner to structure things like virtio-pci.

thanks
-- PMM