From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43834) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8I7n-0001eI-0C for qemu-devel@nongnu.org; Sun, 13 Dec 2015 20:40:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a8I7j-0004So-LN for qemu-devel@nongnu.org; Sun, 13 Dec 2015 20:40:26 -0500 Date: Mon, 14 Dec 2015 12:01:40 +1100 From: David Gibson Message-ID: <20151214010140.GC22783@voom.fritz.box> References: <1449792685-17000-1-git-send-email-david@gibson.dropbear.id.au> <1449792685-17000-3-git-send-email-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Y5rl02BVI9TCfPar" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 02/11] pseries: Cleanup error handling of spapr_cpu_init() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: lvivier@redhat.com, thuth@redhat.com, "qemu-devel@nongnu.org" , Michael Roth , aik@ozlabs.ru, armbru@redhat.com, Alexander Graf , "qemu-ppc@nongnu.org" --Y5rl02BVI9TCfPar Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Dec 11, 2015 at 02:15:38PM +0530, Bharata B Rao wrote: > On Fri, Dec 11, 2015 at 5:41 AM, David Gibson > wrote: > > Currently spapr_cpu_init() is hardcoded to handle any errors as fatal. > > That works for now, since it's only called from initial setup where an > > error here means we really can't proceed. > > > > However, we'll want to handle this more flexibly for cpu hotplug in fut= ure > > so generalize this using the error reporting infrastructure. While we'= re > > at it make a small cleanup in a related part of ppc_spapr_init() to use > > the error infrastructure instead of an old-style explicit fprintf / exi= t. > > > > Signed-off-by: David Gibson > > --- > > hw/ppc/spapr.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 1a5500f..91396cc 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1615,7 +1615,8 @@ static void spapr_boot_set(void *opaque, const ch= ar *boot_device, > > machine->boot_order =3D g_strdup(boot_device); > > } > > > > -static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu) > > +static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, > > + Error **errp) > > { > > CPUPPCState *env =3D &cpu->env; > > > > @@ -1633,7 +1634,7 @@ static void spapr_cpu_init(sPAPRMachineState *spa= pr, PowerPCCPU *cpu) > > } > > > > if (cpu->max_compat) { > > - ppc_set_compat(cpu, cpu->max_compat, &error_fatal); > > + ppc_set_compat(cpu, cpu->max_compat, errp); > > } > > > > xics_cpu_setup(spapr->icp, cpu); > > @@ -1802,10 +1803,9 @@ static void ppc_spapr_init(MachineState *machine) > > for (i =3D 0; i < smp_cpus; i++) { > > cpu =3D cpu_ppc_init(machine->cpu_model); > > if (cpu =3D=3D NULL) { > > - fprintf(stderr, "Unable to find PowerPC CPU definition\n"); > > - exit(1); > > + error_setg(&error_fatal, "Unable to find PowerPC CPU defin= ition"); > > } > > - spapr_cpu_init(spapr, cpu); > > + spapr_cpu_init(spapr, cpu, &error_fatal); >=20 > Using error_fatal is fine here for now, but just want to sound out > that with CPU hotplug, spapr_cpu_init() would move to ->plug() where > we can't be calling this unconditionally with error_fatal as we need a > graceful recovery for hotplugged CPUs and termination for boot CPUs. Right... part of the point of the patch is that the &error_fatal is moved out of spapr_cpu_init() into the caller. Hotplug will add a new caller which can use a different error handler. > Otherwise, Reviewed-by: Bharata B Rao >=20 > Regards, > Bharata. >=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 --Y5rl02BVI9TCfPar Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWbhT0AAoJEGw4ysog2bOS4qsP/i3OFG/tQ7oexlV3stuXuu/U AmcdXAoN3OnmM0Nz8t0DhBuDAnCzG//7kkABtRlC09OCtXNVCR1etKMTy6Iz3dn1 0Id/ippqi/cs08DsCJqmOdC0JQk1PBhGFB5M7O0SyGwhb4lqrRVU4PRKQlMT6hs7 0dAJ/JZrj/02RdY+51/7lRs6ER5DhI2Q23fFPnNO2qfDhjRs0Q1ScMU9FexmQb6J UrEIej7kG++9SLcr+PJBg8Je4tzBa1oMR6chNmBG6STbTtXXj58NyySrRlrxYs15 p44JXCLvx19jdLl3Pf6UhN1Kt9noWZf0HXbzWLmML9tY8ysaq98VL02TPoVik+tE P95PZQOcRQhUBE0CUnQxSjcrTHo8houTh0PMXAhysAv8fhq8DaDxLjRq4+KnQ9Fa 8lDOFuyjtRbrZR2ege5atGmkbgIJkbC8xcGvvMayRdICDbdJfkt5E6YGYU1B2E38 EXtsRvVxn2iPrJ3Q9F848ZOTimgFCzbODb3HXi5GwHUFRVgii0WiHSeEVujwIjrV 1dTjNXfrYiCWqUr1S8QWoUXAcXRunBbwGVvBU4aINd8PRDN6Jb1DpNF6OZdAe3RS xIc/wjnDzWO1yqmpuXnAAXZZkTZvNLq8LqqXRKdddS071J7FmOojPcIpgVvdq3EP 5oKt6D6ybC6Nsnay9VGK =gwtj -----END PGP SIGNATURE----- --Y5rl02BVI9TCfPar--