From mboxrd@z Thu Jan  1 00:00:00 1970
From: Igor Mammedov <imammedo@redhat.com>
Subject: Re: [Qemu-devel] file_ram_alloc: unify mem-path, mem-prealloc error
 handling
Date: Fri, 28 Feb 2014 13:55:38 +0100
Message-ID: <20140228135538.7bf66df8@nial.usersys.redhat.com>
References: <20140204184153.GA25368@amt.cnet>
	<530F8442.7090007@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	kvm-devel <kvm@vger.kernel.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:63670 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751157AbaB1Mzo (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 28 Feb 2014 07:55:44 -0500
In-Reply-To: <530F8442.7090007@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, 27 Feb 2014 19:30:26 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 04/02/2014 19:41, Marcelo Tosatti ha scritto:
> >
> > -mem-prealloc asks to preallocate memory residing on -mem-path path.
> >
> > Currently QEMU exits in case:
> >
> > - Memory file has been created but allocation via explicit write
> > fails.
> >
> > And it fallbacks to malloc in case:
> > - Querying huge page size fails.
> > - Lack of sync MMU support.
> > - Open fails.
> > - mmap fails.
> >
> > Have the same behaviour for all cases: fail in case -mem-path and
> > -mem-prealloc are specified for regions where the requested size is
> > suitable for hugepages.
> >
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> Once we introduce memdev, I believe -mem-path should always exit, and 
> never fall back to malloc/MAP_ANON.
perhaps it should return a error instead of exit.
Exit would be bad for hotplug case, hotplug op should fail and not crash machine.

> For 2.0, I'm applying the patch to uq/master.
> 
> Paolo
> 
> > diff --git a/exec.c b/exec.c
> > index 9ad0a4b..1da1ba7 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -996,7 +996,7 @@ static void *file_ram_alloc(RAMBlock *block,
> >
> >      hpagesize = gethugepagesize(path);
> >      if (!hpagesize) {
> > -        return NULL;
> > +        goto error;
> >      }
> >
> >      if (memory < hpagesize) {
> > @@ -1005,7 +1005,7 @@ static void *file_ram_alloc(RAMBlock *block,
> >
> >      if (kvm_enabled() && !kvm_has_sync_mmu()) {
> >          fprintf(stderr, "host lacks kvm mmu notifiers, -mem-path unsupported\n");
> > -        return NULL;
> > +        goto error;
> >      }
> >
> >      /* Make name safe to use with mkstemp by replacing '/' with '_'. */
> > @@ -1023,7 +1023,7 @@ static void *file_ram_alloc(RAMBlock *block,
> >      if (fd < 0) {
> >          perror("unable to create backing store for hugepages");
> >          g_free(filename);
> > -        return NULL;
> > +        goto error;
> >      }
> >      unlink(filename);
> >      g_free(filename);
> > @@ -1043,7 +1043,7 @@ static void *file_ram_alloc(RAMBlock *block,
> >      if (area == MAP_FAILED) {
> >          perror("file_ram_alloc: can't mmap RAM pages");
> >          close(fd);
> > -        return (NULL);
> > +        goto error;
> >      }
> >
> >      if (mem_prealloc) {
> > @@ -1087,6 +1087,12 @@ static void *file_ram_alloc(RAMBlock *block,
> >
> >      block->fd = fd;
> >      return area;
> > +
> > +error:
> > +    if (mem_prealloc) {
> > +        exit(1);
> > +    }
> > +    return NULL;
> >  }
> >  #else
> >  static void *file_ram_alloc(RAMBlock *block,
> >
> 
>