From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60883) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aL5Hr-0007Vg-SJ for qemu-devel@nongnu.org; Mon, 18 Jan 2016 03:35:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aL5Hq-00018m-Nu for qemu-devel@nongnu.org; Mon, 18 Jan 2016 03:35:43 -0500 Date: Mon, 18 Jan 2016 19:17:35 +1100 From: David Gibson Message-ID: <20160118081735.GO9301@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> <20160118053539.GM9301@voom.fritz.box> <569C8087.20407@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="MWF3YmTHhoLNIVQC" Content-Disposition: inline In-Reply-To: <569C8087.20407@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: mdroth@linux.vnet.ibm.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, armbru@redhat.com --MWF3YmTHhoLNIVQC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jan 18, 2016 at 05:04:55PM +1100, Alexey Kardashevskiy wrote: > On 01/18/2016 04:35 PM, David Gibson wrote: > >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 ha= ndle > >>>>>all errors with error_setg(&error_abort, ...). > >>>>> > >>>>>But really, the callers are really better placed to decide on the er= ror > >>>>>handling. So, instead make the functions use the error propagation > >>>>>infrastructure. > >>>>> > >>>>>In the callers we change to &error_fatal instead of &error_abort, si= nce > >>>>>this can be triggered by a bad configuration or kernel error rather = than > >>>>>indicating a programming error in qemu. > >>>>> > >>>>>While we're at it improve the messages themselves a bit, and clean u= p 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(~H= PTE64_V_HPTE_DIRTY)) > >>>>> #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |=3D tswap64(HP= TE64_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 reques= ted > >>>>> * HTAB size can't be allocated. > >>>>> */ > >>>>>- error_setg(&error_abort, "Failed to allocate HTAB of reques= ted size, try with smaller maxmem"); > >>>>>+ error_setg_errno(errp, -shift, > >>>>>+ "Error allocating KVM hash page table, try= smaller maxmem"); > >>>>> } else if (shift > 0) { > >>>>> /* > >>>>> * Kernel handles htab, we don't need to allocate one > >>>>>@@ -1040,7 +1041,10 @@ static void spapr_alloc_htab(sPAPRMachineStat= e *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 re= quested 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 obj= ecting > >>>>the very good kernel's "never break user-visible strings" rule or rej= ecting > >>>>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. > >> > >> > >>"KVM hash page table.*smaller maxmem" stopped working. Not a big deal b= ut 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 >=20 > checkpatch.pl reports it as "WARNING: line over 80 characters", not an > ERROR, so I'd say the hook has a problem. Well, maybe, but it's not a problem I'm really inclined to tackle at this time. --=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 --MWF3YmTHhoLNIVQC Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWnJ+fAAoJEGw4ysog2bOSFIcQAKFGVqGSEQgdd1Pk7efz+jRe Bt+dqM5bviWQCJcY577enlLUOX7cGWsinKUuuoo/63k7hd+FxHiG/KV0YezZOokn 4hoCO+nmR4y0uChqipK6alb5ZEsow9NJax51jfIWtq/nDwVHb2I+G7p4hrrSdtAT zrctSsrEGAfyQNN82XJu4IXs4gj2PvPCDeUkbBdpn/cBW6uP5MQK+48j9g7B4B4a wcyr3sSjB5HBrOm0cgQJnEz+5r7g9H1tuEAzIwrQ8PyRs7R3U/SMipN+VmjMj/og FKKzqLpPVUAXkaU3Uk3sYgrT0bJDd9GJLP3QpnZUoIeF59rpf8Z5CNLWMleWLlTH efqIl8QHuuTRIgZvSl9529MpjBSjBiCnfD+dZlSCNHf6s9zTG8BzuTgrswNncyBU fmN65JplczA3i8YIOajWLF2yqFxcy+ZeE0jSHwHlRz9TlRYYLrOk2eUYuhCGzkDH p7mV56So81UziOlwsGidyWE276T0aNRfnEEXOF+Ms7hUxb80PmL/jajETN6AWV2W 6ZbS0wK3ZAPPy2/uXee5hcUj5CLxmM/ZK4SPHGeW40+tGOISFBVo5LijT5+0x6fS BtTGPzR2Rpgq9dqXFX+PCgMK5GMGGQyzw7SJhNO7MiAXiiC/8sLGqHO3kyffg74y uDi8SVLYqTGzLyao4Uk5 =Tk86 -----END PGP SIGNATURE----- --MWF3YmTHhoLNIVQC--