From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40085) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZU49J-000579-8O for qemu-devel@nongnu.org; Mon, 24 Aug 2015 22:39:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZU49F-0003z4-U7 for qemu-devel@nongnu.org; Mon, 24 Aug 2015 22:39:45 -0400 Received: from e19.ny.us.ibm.com ([129.33.205.209]:36735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZU49F-0003xy-QK for qemu-devel@nongnu.org; Mon, 24 Aug 2015 22:39:41 -0400 Received: from /spool/local by e19.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 24 Aug 2015 22:39:39 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1439967371-15870-3-git-send-email-bharata@linux.vnet.ibm.com> References: <1439967371-15870-1-git-send-email-bharata@linux.vnet.ibm.com> <1439967371-15870-3-git-send-email-bharata@linux.vnet.ibm.com> Message-ID: <20150825022628.11069.64154@loki> Date: Mon, 24 Aug 2015 21:26:28 -0500 Subject: Re: [Qemu-devel] [RFC PATCH v0 2/3] spapr-rtas: Enable rtas_set_indicator() to return correct error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao , qemu-devel@nongnu.org Cc: agraf@suse.de, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, imammedo@redhat.com, david@gibson.dropbear.id.au Quoting Bharata B Rao (2015-08-19 01:56:10) > drck->set_isolation_state() can return error. For such a case ensure > correct error is returned by rtas_set_indicator() instead of always > returning success. > = > TODO: rtas_st(, , uint32 val) =3D> the return value uint32, but > drck->set_[allocation/indicator/isolation]_state() is returning int. > Should we change this return value to uint32_t to match with rtas_st() > argument ? I wouldn't bother too much aligning the types unless we go to the extent of documenting these interfaces as returning rtas error codes. That's not really the case currently, and there's a lot of rtas errors that don't really need to be determined by DRC code so I think it's more trouble than it's worth. For now I think it's better to just check for ret !=3D 0 and set return values explicitly in rtas code based on what the drc errors entail. > = > Signed-off-by: Bharata B Rao > --- > hw/ppc/spapr_rtas.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > = > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index e99e25f..96729b4 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -374,6 +374,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPR= MachineState *spapr, > uint32_t sensor_state; > sPAPRDRConnector *drc; > sPAPRDRConnectorClass *drck; > + int ret; > = > if (nargs !=3D 3 || nret !=3D 1) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > @@ -413,19 +414,19 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPA= PRMachineState *spapr, > spapr_ccs_remove(spapr, ccs); > } > } > - drck->set_isolation_state(drc, sensor_state); > + ret =3D drck->set_isolation_state(drc, sensor_state); > break; > case RTAS_SENSOR_TYPE_DR: > - drck->set_indicator_state(drc, sensor_state); > + ret =3D drck->set_indicator_state(drc, sensor_state); > break; > case RTAS_SENSOR_TYPE_ALLOCATION_STATE: > - drck->set_allocation_state(drc, sensor_state); > + ret =3D drck->set_allocation_state(drc, sensor_state); > break; > default: > goto out_unimplemented; > } > = > - rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + rtas_st(rets, 0, ret); > return; > = > out_unimplemented: > -- = > 2.1.0 >=20