diff mbox

[v1,2/2] spapr: Handle failure of KVM_PPC_ALLOCATE_HTAB ioctl

Message ID 1447133094-32676-3-git-send-email-bharata@linux.vnet.ibm.com
State New
Headers show

Commit Message

Bharata B Rao Nov. 10, 2015, 5:24 a.m. UTC
KVM_PPC_ALLOCATE_HTAB ioctl can return -ENOMEM for KVM guests and QEMU
never handled this correctly. But this didn't cause any problems till
now as KVM_PPC_ALLOCATE_HTAB ioctl returned with smaller than requested
HTAB when enough contiguous memory wasn't available in the host.
After the proposed kernel change: https://patchwork.ozlabs.org/patch/530501/,
KVM_PPC_ALLOCATE_HTAB ioctl will not fallback to lower sized HTAB
allocation and will fail if requested HTAB size can't be met.

Check for such failures in QEMU and abort appropriately. This will
prevent guest kernel from hanging/freezing during early boot by doing
graceful exit when host is unable to allocate requested HTAB.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Michael Roth Nov. 10, 2015, 2:38 p.m. UTC | #1
Quoting Bharata B Rao (2015-11-09 23:24:54)
> KVM_PPC_ALLOCATE_HTAB ioctl can return -ENOMEM for KVM guests and QEMU
> never handled this correctly. But this didn't cause any problems till
> now as KVM_PPC_ALLOCATE_HTAB ioctl returned with smaller than requested
> HTAB when enough contiguous memory wasn't available in the host.
> After the proposed kernel change: https://patchwork.ozlabs.org/patch/530501/,
> KVM_PPC_ALLOCATE_HTAB ioctl will not fallback to lower sized HTAB
> allocation and will fail if requested HTAB size can't be met.
> 
> Check for such failures in QEMU and abort appropriately. This will
> prevent guest kernel from hanging/freezing during early boot by doing
> graceful exit when host is unable to allocate requested HTAB.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e1202ce..a64a1b5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1021,9 +1021,19 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
>       * RAM */
> 
>      shift = kvmppc_reset_htab(spapr->htab_shift);
> -
> -    if (shift > 0) {
> -        /* Kernel handles htab, we don't need to allocate one */
> +    if (shift < 0) {
> +        /*
> +         * 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");

Wording is appropriate, but maybe just "Failed to allocate HTAB, try with
smaller maxmem" to distinguish it from the error below where the call succeeds
but there's a size mismatch?

> +    } else if (shift > 0) {
> +        /*
> +         * Kernel handles htab, we don't need to allocate one
> +         *
> +         * Older kernels can fall back to lower HTAB shift values,
> +         * but we don't allow booting of such guests.
> +         */
>          if (shift != spapr->htab_shift) {
>              error_setg(&error_abort, "Failed to allocate HTAB of requested size, try with smaller maxmem");
>          }
> @@ -1055,7 +1065,9 @@ static void spapr_reset_htab(sPAPRMachineState *spapr)
>      int index;
> 
>      shift = kvmppc_reset_htab(spapr->htab_shift);
> -    if (shift > 0) {
> +    if (shift < 0) {
> +        error_setg(&error_abort, "Failed to reset HTAB");
> +    } else if (shift > 0) {
>          if (shift != spapr->htab_shift) {
>              error_setg(&error_abort, "Requested HTAB allocation failed during reset");
>          }
> -- 
> 2.1.0
>
David Gibson Nov. 11, 2015, 12:07 a.m. UTC | #2
On Tue, Nov 10, 2015 at 08:38:57AM -0600, Michael Roth wrote:
> Quoting Bharata B Rao (2015-11-09 23:24:54)
> > KVM_PPC_ALLOCATE_HTAB ioctl can return -ENOMEM for KVM guests and QEMU
> > never handled this correctly. But this didn't cause any problems till
> > now as KVM_PPC_ALLOCATE_HTAB ioctl returned with smaller than requested
> > HTAB when enough contiguous memory wasn't available in the host.
> > After the proposed kernel change: https://patchwork.ozlabs.org/patch/530501/,
> > KVM_PPC_ALLOCATE_HTAB ioctl will not fallback to lower sized HTAB
> > allocation and will fail if requested HTAB size can't be met.
> > 
> > Check for such failures in QEMU and abort appropriately. This will
> > prevent guest kernel from hanging/freezing during early boot by doing
> > graceful exit when host is unable to allocate requested HTAB.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr.c | 20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index e1202ce..a64a1b5 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1021,9 +1021,19 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
> >       * RAM */
> > 
> >      shift = kvmppc_reset_htab(spapr->htab_shift);
> > -
> > -    if (shift > 0) {
> > -        /* Kernel handles htab, we don't need to allocate one */
> > +    if (shift < 0) {
> > +        /*
> > +         * 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");
> 
> Wording is appropriate, but maybe just "Failed to allocate HTAB, try with
> smaller maxmem" to distinguish it from the error below where the call succeeds
> but there's a size mismatch?

Possibly the message could be a little better, but I'm not going to
hold up the patches just for that.  I've applied tentatively to
spapr-next, and I hope to send a pull req with it soon, assuming it
survives my tests.
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e1202ce..a64a1b5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1021,9 +1021,19 @@  static void spapr_alloc_htab(sPAPRMachineState *spapr)
      * RAM */
 
     shift = kvmppc_reset_htab(spapr->htab_shift);
-
-    if (shift > 0) {
-        /* Kernel handles htab, we don't need to allocate one */
+    if (shift < 0) {
+        /*
+         * 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");
+    } else if (shift > 0) {
+        /*
+         * Kernel handles htab, we don't need to allocate one
+         *
+         * Older kernels can fall back to lower HTAB shift values,
+         * but we don't allow booting of such guests.
+         */
         if (shift != spapr->htab_shift) {
             error_setg(&error_abort, "Failed to allocate HTAB of requested size, try with smaller maxmem");
         }
@@ -1055,7 +1065,9 @@  static void spapr_reset_htab(sPAPRMachineState *spapr)
     int index;
 
     shift = kvmppc_reset_htab(spapr->htab_shift);
-    if (shift > 0) {
+    if (shift < 0) {
+        error_setg(&error_abort, "Failed to reset HTAB");
+    } else if (shift > 0) {
         if (shift != spapr->htab_shift) {
             error_setg(&error_abort, "Requested HTAB allocation failed during reset");
         }