From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37352) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TkihJ-0004wx-TL for qemu-devel@nongnu.org; Mon, 17 Dec 2012 16:58:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TkihH-0001HK-V4 for qemu-devel@nongnu.org; Mon, 17 Dec 2012 16:58:05 -0500 Received: from cantor2.suse.de ([195.135.220.15]:48337 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TkihH-0001Gw-L4 for qemu-devel@nongnu.org; Mon, 17 Dec 2012 16:58:03 -0500 Message-ID: <50CF9560.2030300@suse.de> Date: Mon, 17 Dec 2012 22:57:52 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1355761490-10073-1-git-send-email-pbonzini@redhat.com> <1355761490-10073-2-git-send-email-pbonzini@redhat.com> In-Reply-To: <1355761490-10073-2-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 01/15] qdev: do not reset a device until the parent has been initialized List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Igor Mammedov , aliguori@us.ibm.com, qemu-devel@nongnu.org, Eduardo Habkost , mst@redhat.com Am 17.12.2012 17:24, schrieb Paolo Bonzini: > When a device creates a bus and more devices as part of its init callba= ck, the > child device could be reset while the parent is still only partly initi= alized. > In this case, the right thing to do is to delay resetting the child. D= o not > do it at all in qdev_init, instead use qdev_reset_all to reset already-= created > devices when the state goes from CREATED to INITIALIZED. >=20 > This happens when hotplugging a usb-storage device. Without this patch= , > initialization of a hotplugged usb-storage device would run in pre-orde= r. > Initialization of a coldplugged usb-storage device would run according = to > qdev_reset_all semantics (pre-order right now, post-order later in the > series). This patch makes things consistent. >=20 > Signed-off-by: Paolo Bonzini > --- > hw/qdev.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/qdev.c b/hw/qdev.c > index 599382c..2fdf4ac 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -176,8 +176,9 @@ int qdev_init(DeviceState *dev) > dev->alias_required_for_version= ); > } > dev->state =3D DEV_STATE_INITIALIZED; > - if (dev->hotplugged) { > - device_reset(dev); > + if (dev->hotplugged && > + dev->parent_bus->parent->state =3D=3D DEV_STATE_INITIALIZED) { This assumes that parent_bus !=3D NULL, when we are trying to let our fallback bus SysBus die out. The CPU in the current proposal is not a SysBus device, to avoid compiling SysBus unnecessarily into *-user. Your direct access into the parent's state also conflicts with my QOM realize series IIUC. So please at least add appropriate checks to do the below reset only when there is a bus and leave the old behavior otherwise. Andreas > + qdev_reset_all(dev); > } > return 0; > } >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg