On Tue, May 07, 2013 at 02:54:38PM +0200, KONRAD Frédéric wrote:
> On 07/05/2013 14:33, Michael S. Tsirkin wrote:
> >On Tue, May 07, 2013 at 02:29:38PM +0200, KONRAD Frédéric wrote:
> >>Hi,
> >>
> >>I think it is not a good idea, as we wanted to make VirtIODevice
> >>hot-pluggable for virtio-mmio.
> >And then this hack will break cross version migration.
>
> Why?
>
> virtio_net_set_config_size is called by each "proxy" devices no?
If it's called then there's no need for a default,
so this patch should be fine.
> >
> >>Maybe we can use a callback which is called by virtio-bus before
> >>plugging, to ensure that this
> >>config size is computed?
> >OK, will you post such a patch?
> >
>
> The issue is as we said in the last thread, host_feature is part of
> the proxy.
Maybe you could add documentation on how initialization works?
If I could fiure it out I would maybe suggest some solutions.
> And if we want to move it to the devices, we must have a kind of
> property forwarding mechanism.
>
> Anthony said he had something about that.
> >>>All buses do this, and if they don't they get subtle
> >>>bugs related to cross version migration.
> >>>Let's add an assert to catch these bugs early.
> >>>
> >>>Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> >>>---
> >>>
> >>>Just a cleanup so not 1.5 material.
> >>>Seems to work fine here - any opinions?
> >>>
> >>> hw/net/virtio-net.c | 7 ++++---
> >>> 1 file changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>>diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>>index 908e7b8..f0a9272 100644
> >>>--- a/hw/net/virtio-net.c
> >>>+++ b/hw/net/virtio-net.c
> >>>@@ -1282,6 +1282,8 @@ static int virtio_net_device_init(VirtIODevice *vdev)
> >>> DeviceState *qdev = DEVICE(vdev);
> >>> VirtIONet *n = VIRTIO_NET(vdev);
> >>>+ /* Verify that config size has been initialized */
> >>>+ assert(n->config_size != (size_t)-1);
> >>> virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET,
> >>> n->config_size);
> >>>@@ -1386,10 +1388,9 @@ static void virtio_net_instance_init(Object *obj)
> >>> VirtIONet *n = VIRTIO_NET(obj);
> >>> /*
> >>>- * The default config_size is sizeof(struct virtio_net_config).
> >>>- * Can be overriden with virtio_net_set_config_size.
> >>>+ * The config_size must be set later with virtio_net_set_config_size.
> >>> */
> >>>- n->config_size = sizeof(struct virtio_net_config);
> >>>+ n->config_size = (size_t)-1;
> >>> }
> >>> static Property virtio_net_properties[] = {
