From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43761) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8I7k-0001e1-Qh for qemu-devel@nongnu.org; Sun, 13 Dec 2015 20:40:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a8I7j-0004T1-ME for qemu-devel@nongnu.org; Sun, 13 Dec 2015 20:40:24 -0500 Date: Mon, 14 Dec 2015 12:11:13 +1100 From: David Gibson Message-ID: <20151214011113.GE22783@voom.fritz.box> References: <1449792685-17000-1-git-send-email-david@gibson.dropbear.id.au> <1449792685-17000-4-git-send-email-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="nHwqXXcoX0o6fKCv" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 03/11] pseries: Clean up hash page table allocation error handling 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" --nHwqXXcoX0o6fKCv Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Dec 11, 2015 at 02:10:40PM +0530, Bharata B Rao wrote: > On Fri, Dec 11, 2015 at 5:41 AM, David Gibson > wrote: > > The spapr_alloc_htab() and spapr_reset_htab() functions currently handle > > all errors with error_setg(&error_abort, ...). That's correct for > > spapr_reset_htab() - if anything goes wrong there, there's really nothi= ng > > we can do about it. For spapr_alloc_htab() &error_fatal would make more > > sense, since it occurs during initialization. > > > > But in any case the callers are really better placed to decide on the e= rror > > handling. So, instead make the functions use the error propagation > > infrastructure. > > > > While we're at it improve the messages themselves a bit, and clean up t= he > > indentation a little. > > > > Signed-off-by: David Gibson > > --- > > hw/ppc/spapr.c | 26 ++++++++++++++++++-------- > > 1 file changed, 18 insertions(+), 8 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 91396cc..85474ee 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1015,7 +1015,7 @@ static void emulate_spapr_hypercall(PowerPCCPU *c= pu) > > #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &=3D tswap64(~HPTE6= 4_V_HPTE_DIRTY)) > > #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |=3D tswap64(HPTE64= _V_HPTE_DIRTY)) > > > > -static void spapr_alloc_htab(sPAPRMachineState *spapr) > > +static void spapr_alloc_htab(sPAPRMachineState *spapr, Error **errp) > > { > > long shift; > > int index; > > @@ -1030,7 +1030,9 @@ static void spapr_alloc_htab(sPAPRMachineState *s= papr) > > * For HV KVM, host kernel will return -ENOMEM when requested > > * HTAB size can't be allocated. > > */ > > - error_setg(&error_abort, "Failed to allocate HTAB of requested= size, try with smaller maxmem"); > > + error_setg(errp, > > + "Error allocating KVM hash page table, try smaller maxmem:= %s", > > + strerror(-shift)); >=20 > Do you want to explicitly "return" from here and other places in this > patch like you do in 4/11 to improve readability ? Given we still need the if (shift > 0), I don't think it really makes any difference to readability. --=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 --nHwqXXcoX0o6fKCv Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWbhcxAAoJEGw4ysog2bOSsmEP/3JA0Hfcpm+hysEucKoUNAt4 HnWwk3cErXNgpBXbc+HOBpaGu8yqYN+eEuRWBqQe4c25j70Fb1hK25XICOtJ0/72 bCbpDO4PUyzj9NvsYArGQTAdhoNL2wYUmFLXc/3WxYosgGI3siSVbtFZ/9I9n7kt ShgCmKvhnxoIK/euMOQgPft0unLt5PqkD09sSrDa7R/U/c/ZfYbXAUVdN2fO0z/b VbUwqTMRy/A6FmBFQVQ5Tg0StUUEDwxD9dKCAgVTmQPLYoOCVFEOoTfSevN/aB26 OaNBsEUZjDmBt/gOZbfnxKGKU80r3u5pgT7u2vBakatw4HVkuxdAPCban8hcbog7 j5hvR9QP0lkiskysel/Ql8e3PnNfBMiaKPBgzg6V/1Fd9DRra+T1G9tS4jtwU1Rz O7wnAVsdv64OF5edptws6MWYGOBKy84B9Y0EUs4m4yHyWpMWMghhZQOKTgirjQDB JosVbB9Baatsy8m9Z5PYySXtSLnP2uGgQcsEJufztF8rHvryE4y1aLWeXKN8IDTG uAWz+ZGb1qt/TuVUdUqRDlOikB2snViZ0VY+G2DC2LuSJxxPepkYCmPM8LilJJGy b9L6AYG7CC4HmlUsj3v4GpXO4BSDOLS2dXs5R6shOxV1JGiMx4Mw4eLQ9EtP3gBz QSk9DCnuHRxT0L8VDtUJ =VjVw -----END PGP SIGNATURE----- --nHwqXXcoX0o6fKCv--