From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54542) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZjpp-000105-L1 for qemu-devel@nongnu.org; Tue, 07 May 2013 11:29:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZjpk-0001Hc-GT for qemu-devel@nongnu.org; Tue, 07 May 2013 11:29:45 -0400 Received: from greensocs.com ([87.106.252.221]:38672 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZjpk-0001HT-5U for qemu-devel@nongnu.org; Tue, 07 May 2013 11:29:40 -0400 Message-ID: <51891DDE.2070106@greensocs.com> Date: Tue, 07 May 2013 17:29:34 +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> <5188F98E.9080203@greensocs.com> <20130507140056.GA21686@redhat.com> In-Reply-To: <20130507140056.GA21686@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 16:00, Michael S. Tsirkin wrote: > 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. >> 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. True but as I told you, we made this refactoring to be able to hot-plug VirtIODevice on a virtio-bus, so calling this function won't be possible. But you are right this must be fixed. > >>>> 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. Yes, where do you think I can add this? > >> 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 >>>>> --- >>>>> >>>>> 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(VirtIODevic= e *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_confi= g_size. >>>>> */ >>>>> - n->config_size =3D sizeof(struct virtio_net_config); >>>>> + n->config_size =3D (size_t)-1; >>>>> } >>>>> static Property virtio_net_properties[] =3D {