On 20 February 2014 19:09, Mario Smarduch <m.smard...@samsung.com> wrote: > Questionable in this patch - is cutting through several layers to set > proxy properties. They should be set from "device" instance init > before it's realized. The problem though is that unlike PCI > that sets proxy and virtio-net properties via its virtio_net_properites[], > the virtio-mmio version instantiates the proxy in the machine model, > so it doesn't appear to be good place to set virtio-net > host features since you don't know what virtio device will be plugged > in later.
I think this function is the right place to set these properties, yes. What I'm saying is that I don't see why you're doing it this way rather than using the existing per-backend hook. Maybe there's a reason not to use that hook, but you don't say. > It's virtio_net_properties[] can only set virtio-net > properites when it's instantiated. I think the proper way would > be to refactor virtio-mmio to similar structure PCI version uses then > one virtio_net_properties[] can be used selecting PCI or virtio-mmio at > compile time. But right now it's unclear to me how pci and virtio-mmio > versions intend to co-exist. This is the result of a refactoring last year to allow virtio-mmio. Basically the underlying structure now is: [some virtio transport device] <- virtio bus -> [virtio backend] where the transport could be mmio, PCI, CCW, etc, and the backend is net, blk, balloon, etc. We also provide devices which are "virtio PCI transport plus a specific backend" for (a) user convenience and (b) backwards compatibility, since the pre-refactoring structure put the transport and backend all in one lump. The all-in-one-lump arrangement is legacy: we shouldn't break it, but it's not the model for virtio-mmio to follow either. The transport shouldn't know anything specific about the backend it's plugged into (or vice-versa), but it's fine for there to be hook functions so the transport and backend can call each other at the appropriate times. thanks -- PMM
