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

PATCH Marcelo Tosatti 3 participants 6 messages Feb 4, 2014 – Feb 28, 2014 KVM

lore

Share & tools

lore

Marcelo Tosatti

author

2014-02-04 18:42 UTC #0 lore raw

-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 <[email protected]>

exec.c +10 -4

index 9ad0a4b..1da1ba7 100644
@@ -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,

Marcelo Tosatti

author

2014-02-04 18:43 UTC #1 lore raw

Quoted 14 lines · 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.

Quoted 5 lines

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 <[email protected]>

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

Paolo Bonzini

maintainer Reviewed-by

2014-02-07 08:39 UTC #2 lore raw

In reply to Marcelo Tosatti

Il 04/02/2014 19:41, Marcelo Tosatti ha scritto:

Quoted 64 lines

-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 <[email protected]>

exec.c +4 -4

index 9ad0a4b..1da1ba7 100644
@@ -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;

exec.c static void *file_ram_alloc(RAMBlock *block, · line 1098

Quoted 10 lines

exec.c +6 -0

     return area;
+
+error:
+    if (mem_prealloc) {
+        exit(1);
+    }
+    return NULL;
 }
 #else
 static void *file_ram_alloc(RAMBlock *block,

Reviewed-by Paolo Bonzini <[email protected]>

Will apply soon.

Paolo Bonzini

maintainer

2014-02-27 18:30 UTC #3 lore raw

In reply to Marcelo Tosatti

Il 04/02/2014 19:41, Marcelo Tosatti ha scritto:

Quoted 14 lines

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

Quoted 5 lines

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 <[email protected]>

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

Quoted 54 lines

exec.c +10 -4

index 9ad0a4b..1da1ba7 100644
@@ -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,

Igor Mammedov

2014-02-28 12:55 UTC #4 lore raw

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

Quoted 15 lines · On Thu, 27 Feb 2014 19:30:26 +0100 Paolo Bonzini <[email protected]> wrote:

Il 04/02/2014 19:41, Marcelo Tosatti ha scritto:
Quoted 14 lines
-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.

Quoted 8 lines

Quoted 5 lines
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 <[email protected]>
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.

Quoted 60 lines

For 2.0, I'm applying the patch to uq/master.

Paolo

Quoted 54 lines

exec.c +10 -4

index 9ad0a4b..1da1ba7 100644
@@ -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,

Marcelo Tosatti

author

2014-02-28 14:41 UTC #5 lore raw

In reply to Paolo Bonzini

Quoted 15 lines · Paolo Bonzini

Il 04/02/2014 19:41, Marcelo Tosatti ha scritto:
Quoted 14 lines
-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.

Quoted 8 lines

Quoted 5 lines
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 <[email protected]>
Once we introduce memdev, I believe -mem-path should always exit, and never fall back to malloc/MAP_ANON.

Agree.