From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50267) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VipB3-0004Lv-Hb for qemu-devel@nongnu.org; Tue, 19 Nov 2013 12:33:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VipAy-0002fV-1L for qemu-devel@nongnu.org; Tue, 19 Nov 2013 12:33:29 -0500 Message-ID: <528BA0DD.3040902@suse.de> Date: Tue, 19 Nov 2013 18:33:17 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <528B5EEE.5000705@suse.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-1.7] pci: unregister vmstate_pcibus on unplug List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bandan Das , Paolo Bonzini Cc: qemu-stable , qemu-devel@nongnu.org, Anthony Liguori , Michael Tsirkin Am 19.11.2013 18:03, schrieb Bandan Das: > Andreas F=C3=A4rber writes: >=20 >> Am 06.11.2013 23:52, schrieb Bandan Das: >>> >>> PCIBus registers a vmstate during init. Unregister it upon >>> removal/unplug. >>> >>> Signed-off-by: Bandan Das >> >> Michael, this patch looks good for 1.7 to me, are you planning to stil= l >> pick it up? Only one small comment below. >> >> Cc: qemu-stable@nongnu.org >> >>> --- >>> Note that I didn't add a instance_init to register vmstate (yet)=20 >>> due to concerns expressed by Andreas that we shouldn't be registering= =20 >>> global state there. >> >> What's happening here is the following: instance_init does in fact not >> register anything, but vmstate_unregister() becomes a no-op loop if th= e >> vmsd+opaque combo is not registered, so it is safe. The registration >> happens in pci_bus_new() / pci_bus_new_inplace(), which I believe all >> PCI buses to date inside QEMU use, i.e. after instance_init, so in >> practice unregistering will not be no-op. >=20 > Ok, thanks! Based on your explanation, I think it should be safe to mov= e > vmstate_register to instance_init as Paolo had suggested. Why? I still think that would be wrong. We had previously discussed with Paolo that VMState is global state, which according to Anthony should not be registered before realization. So far we have a mix of PCI host bridges instantiating PCIBus before or after realization depending on whether the bus name needs to depend on the device id or not (with trend towards instantiating the PCIBus during instance_init), at which point in time the state should not be registered yet. The sketched solution was to implement a "realized" property for BusState, so that we can decouple vmstate_register() from instantation time rather than moving it into instance_init. Andreas > If Michael=20 > and rest of the folks agree, I am inclined to send in a new version=20 > (which also fixes the issue you noted below). --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg