From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58235) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZhPp-0001kv-4V for qemu-devel@nongnu.org; Tue, 07 May 2013 08:54:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZhPn-0001Wm-Ni for qemu-devel@nongnu.org; Tue, 07 May 2013 08:54:45 -0400 Received: from greensocs.com ([87.106.252.221]:58015 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZhPn-0001WZ-Gx for qemu-devel@nongnu.org; Tue, 07 May 2013 08:54:43 -0400 Message-ID: <5188F98E.9080203@greensocs.com> Date: Tue, 07 May 2013 14:54:38 +0200 From: =?ISO-8859-1?Q?KONRAD_Fr=E9d=E9ric?= MIME-Version: 1.0 References: <20130507102223.GA16878@redhat.com> <5188F3B2.2040109@greensocs.com> <20130507123313.GC21361@redhat.com> In-Reply-To: <20130507123313.GC21361@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: "Michael S. Tsirkin" Cc: Peter Maydell , Anthony Liguori , Jason Wang , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini 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. Why? virtio_net_set_config_size is called by each "proxy" devices no? > >> 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=20 proxy. And if we want to move it to the devices, we must have a kind of=20 property forwarding mechanism. Anthony said he had something about that. >> On 07/05/2013 12:22, Michael S. Tsirkin wrote: >>> 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 *o= bj) >>> 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 {