From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51245) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZiS6-0007Kv-KI for qemu-devel@nongnu.org; Tue, 07 May 2013 10:01:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZiS4-0008Qi-37 for qemu-devel@nongnu.org; Tue, 07 May 2013 10:01:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19452) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZiS3-0008QO-M9 for qemu-devel@nongnu.org; Tue, 07 May 2013 10:01:08 -0400 Date: Tue, 7 May 2013 17:00:56 +0300 From: "Michael S. Tsirkin" Message-ID: <20130507140056.GA21686@redhat.com> References: <20130507102223.GA16878@redhat.com> <5188F3B2.2040109@greensocs.com> <20130507123313.GC21361@redhat.com> <5188F98E.9080203@greensocs.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <5188F98E.9080203@greensocs.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] virtio-net: require that config size is initialized List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: KONRAD =?iso-8859-1?Q?Fr=E9d=E9ric?= Cc: Peter Maydell , Anthony Liguori , Jason Wang , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini On Tue, May 07, 2013 at 02:54:38PM +0200, KONRAD Fr=E9d=E9ric wrote: > On 07/05/2013 14:33, Michael S. Tsirkin wrote: > >On Tue, May 07, 2013 at 02:29:38PM +0200, KONRAD Fr=E9d=E9ric 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. >=20 > Why? >=20 > 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? > > >=20 > 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. >=20 > 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 > >>>--- > >>> > >>>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 =3D DEVICE(vdev); > >>> VirtIONet *n =3D VIRTIO_NET(vdev); > >>>+ /* Verify that config size has been initialized */ > >>>+ assert(n->config_size !=3D (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 =3D 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 =3D sizeof(struct virtio_net_config); > >>>+ n->config_size =3D (size_t)-1; > >>> } > >>> static Property virtio_net_properties[] =3D {