From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46306) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aL2Sq-0003f3-5h for qemu-devel@nongnu.org; Mon, 18 Jan 2016 00:34:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aL2Sm-00086A-Ud for qemu-devel@nongnu.org; Mon, 18 Jan 2016 00:34:52 -0500 Date: Mon, 18 Jan 2016 16:35:39 +1100 From: David Gibson Message-ID: <20160118053539.GM9301@voom.fritz.box> References: <1452859244-9500-1-git-send-email-david@gibson.dropbear.id.au> <1452859244-9500-4-git-send-email-david@gibson.dropbear.id.au> <569C5170.9000606@ozlabs.ru> <20160118044204.GK9301@voom.fritz.box> <569C7554.5070507@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="FX+Db2fp7WJhXKrW" Content-Disposition: inline In-Reply-To: <569C7554.5070507@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH 03/10] pseries: Clean up hash page table allocation error handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, armbru@redhat.com, mdroth@linux.vnet.ibm.com --FX+Db2fp7WJhXKrW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jan 18, 2016 at 04:17:08PM +1100, Alexey Kardashevskiy wrote: > On 01/18/2016 03:42 PM, David Gibson wrote: > >On Mon, Jan 18, 2016 at 01:44:00PM +1100, Alexey Kardashevskiy wrote: > >>On 01/15/2016 11:00 PM, David Gibson wrote: > >>>The spapr_alloc_htab() and spapr_reset_htab() functions currently hand= le > >>>all errors with error_setg(&error_abort, ...). > >>> > >>>But really, the callers are really better placed to decide on the error > >>>handling. So, instead make the functions use the error propagation > >>>infrastructure. > >>> > >>>In the callers we change to &error_fatal instead of &error_abort, since > >>>this can be triggered by a bad configuration or kernel error rather th= an > >>>indicating a programming error in qemu. > >>> > >>>While we're at it improve the messages themselves a bit, and clean up = the > >>>indentation a little. > >>> > >>>Signed-off-by: David Gibson > >>>--- > >>> hw/ppc/spapr.c | 24 ++++++++++++++++-------- > >>> 1 file changed, 16 insertions(+), 8 deletions(-) > >>> > >>>diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >>>index b7fd09a..d28e349 100644 > >>>--- a/hw/ppc/spapr.c > >>>+++ b/hw/ppc/spapr.c > >>>@@ -1016,7 +1016,7 @@ static void emulate_spapr_hypercall(PowerPCCPU *= cpu) > >>> #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &=3D tswap64(~HPT= E64_V_HPTE_DIRTY)) > >>> #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |=3D tswap64(HPTE= 64_V_HPTE_DIRTY)) > >>> > >>>-static void spapr_alloc_htab(sPAPRMachineState *spapr) > >>>+static void spapr_alloc_htab(sPAPRMachineState *spapr, Error **errp) > >>> { > >>> long shift; > >>> int index; > >>>@@ -1031,7 +1031,8 @@ static void spapr_alloc_htab(sPAPRMachineState *= spapr) > >>> * 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 requeste= d size, try with smaller maxmem"); > >>>+ error_setg_errno(errp, -shift, > >>>+ "Error allocating KVM hash page table, try s= maller maxmem"); > >>> } else if (shift > 0) { > >>> /* > >>> * Kernel handles htab, we don't need to allocate one > >>>@@ -1040,7 +1041,10 @@ static void spapr_alloc_htab(sPAPRMachineState = *spapr) > >>> * but we don't allow booting of such guests. > >>> */ > >>> if (shift !=3D spapr->htab_shift) { > >>>- error_setg(&error_abort, "Failed to allocate HTAB of requ= ested size, try with smaller maxmem"); > >>>+ error_setg(errp, > >>>+ "Small allocation for KVM hash page table (%ld < %" > >>>+ PRIu32 "), try smaller maxmem", > >> > >> > >> > >>Even though it is not in the CODING_STYLE, I have not seen anyone objec= ting > >>the very good kernel's "never break user-visible strings" rule or rejec= ting > >>patches with user-visible strings failing to fit 80 chars limit. > > > >I'm not. Or rather, the string is already broken by the PRIu32, so > >the newline doesn't make it any less greppable. >=20 >=20 > "KVM hash page table.*smaller maxmem" stopped working. Not a big deal but= I > do not see any win in breaking strings anyway. The problem is that the current pre-commit hooks don't agree with you. They seem to allow long unbroken strings, but if there's a break like the PRIu32, they won't permit the commit. --=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 --FX+Db2fp7WJhXKrW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWnHmrAAoJEGw4ysog2bOSL9EP/R/yr3Sq2DpaFvUGb1QpwOzE Q2iyfYo5LSH1UP954MTPL4VCurLxPJaaiVwn7k2KR479vlHydf+uj3mx5MZPuYiV qCdWC+3GqvhVwnyBfsPLLD9G9++3pyuU8nBHXgwEIe8Hb+j3cXcMWVjO9NdY09o5 1j1dzyG10GVMPAZWPSP/DPjVaiJAqBelXoJmD1Yjjx+TYZdkrqDw4DpSpqrjK+rG zB+U5I7R5FHwYVjjc13XCLdkOSOtJA2Wq6Kx448oNd/e4PTiOUn+LYQ9vItyG2eK DzmCArdsGZsOnRdv0hkn9UcgURXW5xe1foVxjyVU1Ix0tAC7hg40gWThUxUZY1S0 waw6glGtdIYeOb5971xJlh9gQ/8032cgltWtuywhmzZvPpC5EnbnEZZpLpI2AOff zlFceVPHl2TodX783B/6+jFlKpOv2dkYvKVGZyqXN8WbkbVRLBglc9c/dbGA0rrX iQ/SffW/hIcAPYShCIXiGQdprTFCs9QRJvThcLVJnJszAvEP+sVpmN/Jutg/Ef9z +uDxaGvbFAHwew8DSSjUdRAOdHIBXT9gALs8XqIZDeHzCXBNa6r9rALQrqjDGCSr zStW2zkATQ8inhIaCu3r9/GOuxRzgjMjEgwQY+fthtnRu5APTwWaT71nuGz5Hf7g +uUAD1MtcQ0ebwVj946e =Ajke -----END PGP SIGNATURE----- --FX+Db2fp7WJhXKrW--