diff mbox

[RFC,v0,2/2] spapr: Abort when HTAB of requested size isn't allocated

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

Commit Message

Bharata B Rao Sept. 22, 2015, 3:39 a.m. UTC
Terminate the guest when HTAB of requested size isn't allocated by
the host.

When memory hotplug is attempted on a guest that has booted with
less than requested HTAB size, the guest kernel will not be able
to gracefully fail the hotplug request. This patch will ensure that
we never end up in a situation where memory hotplug fails due to
less than requested HTAB size.

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

Comments

David Gibson Sept. 23, 2015, 3:29 a.m. UTC | #1
On Tue, Sep 22, 2015 at 09:09:49AM +0530, Bharata B Rao wrote:
> Terminate the guest when HTAB of requested size isn't allocated by
> the host.
> 
> When memory hotplug is attempted on a guest that has booted with
> less than requested HTAB size, the guest kernel will not be able
> to gracefully fail the hotplug request. This patch will ensure that
> we never end up in a situation where memory hotplug fails due to
> less than requested HTAB size.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/spapr.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4692122..66446af 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -992,6 +992,10 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
>  
>      if (shift > 0) {
>          /* Kernel handles htab, we don't need to allocate one */
> +        if (shift != spapr->htab_shift) {
> +            error_setg(&error_abort, "Failed to allocated requested HTAB size");
> +        }
> +
>          spapr->htab_shift = shift;
>          kvmppc_kern_htab = true;
>
Bharata B Rao Sept. 24, 2015, 3:43 a.m. UTC | #2
On Wed, Sep 23, 2015 at 01:29:51PM +1000, David Gibson wrote:
> On Tue, Sep 22, 2015 at 09:09:49AM +0530, Bharata B Rao wrote:
> > Terminate the guest when HTAB of requested size isn't allocated by
> > the host.
> > 
> > When memory hotplug is attempted on a guest that has booted with
> > less than requested HTAB size, the guest kernel will not be able
> > to gracefully fail the hotplug request. This patch will ensure that
> > we never end up in a situation where memory hotplug fails due to
> > less than requested HTAB size.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> > ---
> >  hw/ppc/spapr.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 4692122..66446af 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -992,6 +992,10 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
> >  
> >      if (shift > 0) {
> >          /* Kernel handles htab, we don't need to allocate one */
> > +        if (shift != spapr->htab_shift) {
> > +            error_setg(&error_abort, "Failed to allocated requested HTAB size");

Do you think we should have a bit more verbose message here so that
use knows what action to take after this abortion ? Like

"Failed to allocate requested HTAB size, try with lower maxmem"

Regards,
Bharata.
David Gibson Sept. 24, 2015, 4:12 a.m. UTC | #3
On Thu, Sep 24, 2015 at 09:13:58AM +0530, Bharata B Rao wrote:
> On Wed, Sep 23, 2015 at 01:29:51PM +1000, David Gibson wrote:
> > On Tue, Sep 22, 2015 at 09:09:49AM +0530, Bharata B Rao wrote:
> > > Terminate the guest when HTAB of requested size isn't allocated by
> > > the host.
> > > 
> > > When memory hotplug is attempted on a guest that has booted with
> > > less than requested HTAB size, the guest kernel will not be able
> > > to gracefully fail the hotplug request. This patch will ensure that
> > > we never end up in a situation where memory hotplug fails due to
> > > less than requested HTAB size.
> > > 
> > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > 
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > > ---
> > >  hw/ppc/spapr.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 4692122..66446af 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -992,6 +992,10 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
> > >  
> > >      if (shift > 0) {
> > >          /* Kernel handles htab, we don't need to allocate one */
> > > +        if (shift != spapr->htab_shift) {
> > > +            error_setg(&error_abort, "Failed to allocated requested HTAB size");
> 
> Do you think we should have a bit more verbose message here so that
> use knows what action to take after this abortion ? Like
> 
> "Failed to allocate requested HTAB size, try with lower maxmem"

Uh, yeah, I guess that's better.
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4692122..66446af 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -992,6 +992,10 @@  static void spapr_alloc_htab(sPAPRMachineState *spapr)
 
     if (shift > 0) {
         /* Kernel handles htab, we don't need to allocate one */
+        if (shift != spapr->htab_shift) {
+            error_setg(&error_abort, "Failed to allocated requested HTAB size");
+        }
+
         spapr->htab_shift = shift;
         kvmppc_kern_htab = true;