[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] hw/net/virtio-net: Move DEFINE_VIRTIO_NE
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] hw/net/virtio-net: Move DEFINE_VIRTIO_NET_FEATURES to virtio-net |
Date: |
Mon, 20 Apr 2015 18:44:22 +0200 |
On Mon, 20 Apr 2015 15:34:06 +0100
Peter Maydell <address@hidden> wrote:
> 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...
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.
[Qemu-devel] [PATCH v2 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi, shannon . zhao, 2015/04/20