From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37993) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aKzwv-0004UL-KB for qemu-devel@nongnu.org; Sun, 17 Jan 2016 21:53:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aKzws-0004aW-DX for qemu-devel@nongnu.org; Sun, 17 Jan 2016 21:53:45 -0500 Date: Mon, 18 Jan 2016 13:50:24 +1100 From: David Gibson Message-ID: <20160118025024.GI9301@voom.fritz.box> References: <1452859244-9500-1-git-send-email-david@gibson.dropbear.id.au> <1452859244-9500-7-git-send-email-david@gibson.dropbear.id.au> <87egdizw9j.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="iRjOs3ViPWHdlw/I" Content-Disposition: inline In-Reply-To: <87egdizw9j.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH 06/10] pseries: Improve error handling in find_unknown_sysbus_device() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aik@ozlabs.ru, qemu-ppc@nongnu.org, mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org --iRjOs3ViPWHdlw/I Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 15, 2016 at 04:40:24PM +0100, Markus Armbruster wrote: > David Gibson writes: >=20 > > Use error_setg() to return an error instead of using an explicit exit(). > > > > Signed-off-by: David Gibson > > --- > > hw/ppc/spapr.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index bb5eaa5..ddca6e6 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1106,6 +1106,7 @@ static void spapr_reset_htab(sPAPRMachineState *s= papr, Error **errp) > > =20 > > static int find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaqu= e) > > { > > + Error **errp =3D opaque; > > bool matched =3D false; > > =20 > > if (object_dynamic_cast(OBJECT(sbdev), TYPE_SPAPR_PCI_HOST_BRIDGE)= ) { > > @@ -1113,9 +1114,10 @@ static int find_unknown_sysbus_device(SysBusDevi= ce *sbdev, void *opaque) > > } > > =20 > > if (!matched) { > > - error_report("Device %s is not supported by this machine yet.", > > - qdev_fw_name(DEVICE(sbdev))); > > - exit(1); > > + error_setg(errp, > > + "Device %s is not supported by this machine yet", > > + qdev_fw_name(DEVICE(sbdev))); > > + return 1; /* Don't continue scanning devices */ >=20 > Re the comment: really? >=20 > find_unknown_sysbus_device() gets passed to > foreach_dynamic_sysbus_device(), which passes it on to > find_sysbus_device(). >=20 > find_sysbus_device() calls it directly for non-containers, ignoring the > function value. >=20 > For containers, it iterates over the container's contents with > object_child_foreach(). That function indeed stops when the callback > returns non-zero. However, the callback is find_sysbus_device(), not > find_unknown_sysbus_device(). >=20 > Am I confused? No, I am. I can't see a reasonable way to change this without assuming the error is fatal, so I think I'll just drop this patch from the series. >=20 > > } > > =20 > > return 0; > > @@ -1150,7 +1152,7 @@ static void ppc_spapr_reset(void) > > uint32_t rtas_limit; > > =20 > > /* Check for unknown sysbus devices */ > > - foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL); > > + foreach_dynamic_sysbus_device(find_unknown_sysbus_device, &error_f= atal); > > =20 > > /* Reset the hash table & recalc the RMA */ > > spapr_reset_htab(spapr, &error_fatal); >=20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --iRjOs3ViPWHdlw/I Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWnFLwAAoJEGw4ysog2bOS9b0QAOZJSiOq2B4iUGIes6sVkcmS IzxOWWIO8CdPHkJPHZqfLnaoBahQOe0jhvy9/rtu22q9bfIvY0cBrae4tPaiYaj/ HbKnAmcUz+AQ23O4UdC2SwRTdFqmOlvl8Vde3GPDqMB11AmD1+DqQ7vACL6MXU7r nFjnIbsTbNIuJzfNlz9mjWXw1wxoZXOtNB2Fuqprv+fGfI480tAg6zK/cFs1ggPi v/ZW7bSp+EYmp8NyQ/nOANCV9kYVl7Wf+vO3ndP0OV7kYhTGeVrZhOO9zBLwNY19 j68zhMyCyw1EE7OCYZc9phw+XSktLTqDOhcxO8yav0QRQuBJK4AUO6yx2sKtetFr NqdXxKoffMwGLN57yD5wimAHmbmtsnt4pRaPNSb301dAydUAYdVSCIjh1d9MiYKh txappMJL8Xy0OSAmRp7B5sk9bRE7Qc603Ab85fW7fYghzkL9ZPtqnBL8I40CFJr7 HeDxlDZbpZXmN/+d/BvN5+xKBBsYl9vZdGw2mjN6uIwX9CK6Ao/PZlH6/8lJ9rBe 9Xc93+GU1msHmJPCXCGNX1IAVeIzOqRR1TOd1GF8HwI4o+/6I2fYYunDvfOxTxeL rBIJtRlwlNJeJLUEZ083GjkY09iWB6THen4txH3LVz0q4hAaFQ+J+rXm0+sGmlEk T/v7vG2mMqEeniYCVWGc =2h2S -----END PGP SIGNATURE----- --iRjOs3ViPWHdlw/I--