On Mon, 20 Apr 2015 15:34:06 +0100 Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 20 April 2015 at 15:08, Cornelia Huck <cornelia.h...@de.ibm.com> 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... Indeed. > > > 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) That actually makes a lot of sense. I'll put checking if I can do something similar for virtio-ccw on my todo list. > (2) it implements a plugged method and a lot of work is done there I'm not sure how much we can actually do in a plugged method for virtio-ccw, but it's probably worth checking out. > (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 So if the features are propagated in the plugged method, virtio-pci should have the same problem? > > > 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. Let me see what makes sense. One of the problems is that we don't have a clean split between the hardware device (along the lines of a pci device) and the virtio proxy - which means that the virtio-ccw realize method does a lot of things that have more to do with channel devices than with virtio. Modelling on the old s390-virtio transport is still another problem, and I don't want to do anything there beyond the minimum changes to make it work.