From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45194) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aKmSc-0007QO-2U for qemu-devel@nongnu.org; Sun, 17 Jan 2016 07:29:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aKmSY-00018f-Sz for qemu-devel@nongnu.org; Sun, 17 Jan 2016 07:29:34 -0500 Date: Sun, 17 Jan 2016 20:32:45 +1100 From: David Gibson Message-ID: <20160117093245.GC9301@voom.fritz.box> References: <1452859244-9500-1-git-send-email-david@gibson.dropbear.id.au> <1452859244-9500-2-git-send-email-david@gibson.dropbear.id.au> <87io2uzx8p.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="pZs/OQEoSSbxGlYw" Content-Disposition: inline In-Reply-To: <87io2uzx8p.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH 01/10] ppc: Cleanup error handling in ppc_set_compat() 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 --pZs/OQEoSSbxGlYw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 15, 2016 at 04:19:18PM +0100, Markus Armbruster wrote: > David Gibson writes: >=20 > > Current ppc_set_compat() returns -1 for errors, and also (unconditional= ly) > > reports an error message. The caller in h_client_architecture_support() > > may then report it again using an outdated fprintf(). > > > > Clean this up by using the modern error reporting mechanisms. > > > > Signed-off-by: David Gibson > > Reviewed-by: Thomas Huth > > --- > > hw/ppc/spapr.c | 4 +--- > > hw/ppc/spapr_hcall.c | 10 +++++----- > > target-ppc/cpu.h | 2 +- > > target-ppc/translate_init.c | 13 +++++++------ > > 4 files changed, 14 insertions(+), 15 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 50e5a26..fa7a7f4 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1635,9 +1635,7 @@ static void spapr_cpu_init(sPAPRMachineState *spa= pr, PowerPCCPU *cpu) > > } > > =20 > > if (cpu->max_compat) { > > - if (ppc_set_compat(cpu, cpu->max_compat) < 0) { > > - exit(1); > > - } > > + ppc_set_compat(cpu, cpu->max_compat, &error_fatal); > > } > > =20 > > xics_cpu_setup(spapr->icp, cpu); > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > index cebceea..8b0fcb3 100644 > > --- a/hw/ppc/spapr_hcall.c > > +++ b/hw/ppc/spapr_hcall.c > > @@ -837,7 +837,7 @@ static target_ulong cas_get_option_vector(int vecto= r, target_ulong table) > > typedef struct { > > PowerPCCPU *cpu; > > uint32_t cpu_version; > > - int ret; > > + Error *err; > > } SetCompatState; > > =20 > > static void do_set_compat(void *arg) > > @@ -845,7 +845,7 @@ static void do_set_compat(void *arg) > > SetCompatState *s =3D arg; > > =20 > > cpu_synchronize_state(CPU(s->cpu)); > > - s->ret =3D ppc_set_compat(s->cpu, s->cpu_version); > > + ppc_set_compat(s->cpu, s->cpu_version, &s->err); > > } > > =20 > > #define get_compat_level(cpuver) ( \ > > @@ -929,13 +929,13 @@ static target_ulong h_client_architecture_support= (PowerPCCPU *cpu_, > > SetCompatState s =3D { > > .cpu =3D POWERPC_CPU(cs), > > .cpu_version =3D cpu_version, > > - .ret =3D 0 > > + .err =3D NULL, > > }; > > =20 > > run_on_cpu(cs, do_set_compat, &s); > > =20 > > - if (s.ret < 0) { > > - fprintf(stderr, "Unable to set compatibility mode\n"); > > + if (s.err) { > > + error_report_err(s.err); > > return H_HARDWARE; > > } > > } > > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > > index 9706000..b3b89e6 100644 > > --- a/target-ppc/cpu.h > > +++ b/target-ppc/cpu.h > > @@ -1210,7 +1210,7 @@ void ppc_store_msr (CPUPPCState *env, target_ulon= g value); > > =20 > > void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf); > > int ppc_get_compat_smt_threads(PowerPCCPU *cpu); > > -int ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version); > > +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **err= p); > > =20 > > /* Time-base and decrementer management */ > > #ifndef NO_CPU_IO_DEFS > > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > > index e88dc7f..d0feed0 100644 > > --- a/target-ppc/translate_init.c > > +++ b/target-ppc/translate_init.c > > @@ -9186,7 +9186,7 @@ int ppc_get_compat_smt_threads(PowerPCCPU *cpu) > > return ret; > > } > > =20 > > -int ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version) > > +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **err= p) > > { > > int ret =3D 0; > > CPUPPCState *env =3D &cpu->env; > > @@ -9208,12 +9208,13 @@ int ppc_set_compat(PowerPCCPU *cpu, uint32_t cp= u_version) > > break; > > } > > =20 > > - if (kvm_enabled() && kvmppc_set_compat(cpu, cpu->cpu_version) < 0)= { > > - error_report("Unable to set compatibility mode in KVM"); > > - ret =3D -1; > > + if (kvm_enabled()) { > > + ret =3D kvmppc_set_compat(cpu, cpu->cpu_version); > > + if (ret < 0) { > > + error_setg_errno(errp, -ret, > > + "Unable to set CPU compatibility mode in = KVM"); > > + } > > } > > - > > - return ret; > > } > > =20 > > static gint ppc_cpu_compare_class_pvr(gconstpointer a, gconstpointer b) >=20 > Error message now includes strerror() of the ioctl's errno. Suggest to > mention that in the commit message. Done. --=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 --pZs/OQEoSSbxGlYw Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWm1+9AAoJEGw4ysog2bOScyEP/j0vYZ9cFH07tjvCqlByiQTN +NvmX2slnA/j9yNzTB6Lr+O6j/w7xnplsmG5fw8zaRjUnoW15INUWTEcpd9T6Edw qQY0WPxOcae4VjTtHDQ279jC0ZGyPooLiXVJfRPf1b2OaRx7+4HokCJMUnG80TL5 b/OkXfvLOY0izX6qZKnHBrnCdv9aACYYk7tBTqUHMoju8Ijnq4H4UecsWvYkv+zF RsE6rZ2BAgNCOKo8czRRPJ8H6B/R+3JbMYNooEA2H9dtv2ooOmoDQ+BAKdhheQUi exCmL0LeoyEeQhpjX4ze/HkFpGIwvoq+DjLXt9B9qNfEaeNCxVlCsHCc/t55zyMD e74EJKTWnhmk3aU94bDwNnegzYhUEdT6PxUvd7PKa7QQ5i/h2L6hIx0MWItngTqT b6ghRJhZvrHALLuDQM9GHFC7+zB4JgxsxVDnQfCKx6DOW6guwQnV2xUiKwNPhCkf mkpvl6V0s3LJiZjZDOj3U44awSfA6LaHHKZW1irbW7yyiD/paISaJal0L5j8to50 piZWVHyiRVW4QGtRX3z/Vf6M46segGH4etBBMu+nDYRsZw679gq4YRCTwGMNa7dr /49rC3sFcO7vdEQHQcTwV9dtIMVTHqb9vjhEBiC5ygVvAQTjnqK1vi6rWJvcZdZA 8D/8ACkAPsBkIdm39sFK =qbpt -----END PGP SIGNATURE----- --pZs/OQEoSSbxGlYw--