From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56966) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UkFU8-0006WA-AO for qemu-devel@nongnu.org; Wed, 05 Jun 2013 11:18:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UkFU3-0003Em-CN for qemu-devel@nongnu.org; Wed, 05 Jun 2013 11:18:48 -0400 Received: from ns232118.ovh.net ([178.33.234.66]:33591) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UkFU3-0003Ec-1A for qemu-devel@nongnu.org; Wed, 05 Jun 2013 11:18:43 -0400 Message-ID: <51AF56CB.4030002@greensocs.com> Date: Wed, 05 Jun 2013 17:18:35 +0200 From: Frederic Konrad MIME-Version: 1.0 References: <1370362965-3937-1-git-send-email-jlarrew@linux.vnet.ibm.com> <1370362965-3937-2-git-send-email-jlarrew@linux.vnet.ibm.com> <51AE2564.4080705@suse.de> <51AE47D3.6080709@linux.vnet.ibm.com> In-Reply-To: <51AE47D3.6080709@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/3] virtio: add bus_plugged() callback to VirtioDeviceClass List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jesse Larrew Cc: "Michael S. Tsirkin" , Stefan Hajnoczi , jasowang@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, Anthony Liguori , =?ISO-8859-15?Q?Andreas_F=E4rber?= On 04/06/2013 22:02, Jesse Larrew wrote: > On 06/04/2013 12:35 PM, Andreas F=E4rber wrote: >> Hi, >> > Hi Andreas! > > Thanks for the review. :) > >> Am 04.06.2013 18:22, schrieb Jesse Larrew: >>> Virtio devices are initialized prior to plugging them into a bus. How= ever, >>> other initializations (such as host_features) don't occur until after= the >>> device is plugged into the bus. If a device needs to modify it's >>> configuration based on host_features, then it needs to be notified wh= en the >>> bus is attached and host_features is available for use. >>> >>> This patch extends struct VirtioDeviceClass to add a bus_plugged() me= thod. >>> If implemented by a device, it will be called after the device is att= ached >>> to a bus. >>> >>> Signed-off-by: Jesse Larrew >> I think this is backwards... >> >> First of all, why is host_features not available before? >> >> A hook on the bus makes sense because it allows central handling for a= ny >> devices on that bus. >> However for a device, first TypeInfo::instance_init is run, then >> qdev_set_parent_bus() connects the bus and finally DeviceClass::realiz= e >> is run - and we want to postpone realize further in the future. >> > Yes! This would do perfectly. > >> So why can't this be in VirtioDevice's or VirtIONet's realize method? = At >> realize time we should definitely be on the bus in this case. I.e., >> create vdev->config only after we know how large it needs to be rather >> than creating and later resizing it, which might fail. >> > It appears that virtio hasn't been completely converted to use realize(= ) yet. > Currently, device_realize() in virtio.c simply calls virtio_device_init= (), > which looks like this: > > static int virtio_device_init(DeviceState *qdev) > { > VirtIODevice *vdev =3D VIRTIO_DEVICE(qdev); > VirtioDeviceClass *k =3D VIRTIO_DEVICE_GET_CLASS(qdev); > assert(k->init !=3D NULL); > if (k->init(vdev) < 0) { > return -1; > } > virtio_bus_plug_device(vdev); > return 0; > } > > VirtioDeviceClass::init() calls virtio_init(), which allocates the conf= ig > struct. Then virtio_bus_plug_device() is called to attach the bus (and = to > populate host_features). I wonder if it's safe to call > virtio_bus_plug_device() sooner... Hi Jesse, Maybe with little change. virtio_pci_device_plugged need the virtio-device to be initialized. Fred > >> Regards, >> Andreas >> > Sincerely, > > Jesse Larrew > Software Engineer, KVM Team > IBM Linux Technology Center > Phone: (512) 973-2052 (T/L: 363-2052) > jlarrew@linux.vnet.ibm.com >