file_ram_alloc: unify mem-path,mem-prealloc error handling

file_ram_alloc: unify mem-path,mem-prealloc error handling

[Marcelo Tosatti]

-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>

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,

[Qemu-devel] file_ram_alloc: unify mem-path, mem-prealloc error handling

[Marcelo Tosatti]

-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>

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,

Re: file_ram_alloc: unify mem-path,mem-prealloc error handling

[Marcelo Tosatti]

On Tue, Feb 04, 2014 at 01:41:53PM -0500, Marcelo Tosatti wrote:
> 
> -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>

Forgot uq/master on the subject (but its aimed for that tree).

Re: [Qemu-devel] file_ram_alloc: unify mem-path, mem-prealloc error handling

[Marcelo Tosatti]

On Tue, Feb 04, 2014 at 01:41:53PM -0500, Marcelo Tosatti wrote:
> 
> -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>

Forgot uq/master on the subject (but its aimed for that tree).

Re: file_ram_alloc: unify mem-path,mem-prealloc error handling

[Paolo Bonzini]

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>
>
> 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,
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Will apply soon.

Re: [Qemu-devel] file_ram_alloc: unify mem-path, mem-prealloc error handling

[Paolo Bonzini]

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>
>
> 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,
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Will apply soon.

Re: file_ram_alloc: unify mem-path,mem-prealloc error handling

[Paolo Bonzini]

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.

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,
>

Re: [Qemu-devel] file_ram_alloc: unify mem-path, mem-prealloc error handling

[Paolo Bonzini]

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.

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,
>

Re: file_ram_alloc: unify mem-path,mem-prealloc error handling

[Marcelo Tosatti]

On Thu, Feb 27, 2014 at 07:30:26PM +0100, Paolo Bonzini 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.

Agree.

Re: [Qemu-devel] file_ram_alloc: unify mem-path, mem-prealloc error handling

[Marcelo Tosatti]

On Thu, Feb 27, 2014 at 07:30:26PM +0100, Paolo Bonzini 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.

Agree.

Re: [Qemu-devel] file_ram_alloc: unify mem-path, mem-prealloc error handling

[Igor Mammedov]

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,
> >
> 
>