bug fixs for memory backend

This patch reintroduces memory_region_init_ram() and memory_region_init_ram_ptr() which are almost the same as the renamed ones in the previous patch, except that an errp parameter is introduced to let callers handle error.

In hostmem-ram.c we call memory_region_init_ram() now rather than memory_region_init_ram_nofail() so that error can be handled.

This patch solves a problem that qemu just exits when using monitor command object_add to add a memory backend whose size is way too large. In the case we'd better give an error message and keep guest running.

The problem can be reproduced as follows:

1. run qemu
2. (monitor)object_add memory-backend-ram,size=100000G,id=ram0

Signed-off-by Hu Tao <[email protected]>

---
 backends/hostmem-ram.c  |  4 ++--
 exec.c                  | 32 ++++++++++++++++++++---------
 hw/block/pflash_cfi01.c |  5 ++++-
 hw/block/pflash_cfi02.c |  5 ++++-
 include/exec/memory.h   | 39 +++++++++++++++++++++++++++++++++++-
 include/exec/ram_addr.h |  4 ++--
 memory.c                | 53 +++++++++++++++++++++++++++++++++++++++++++++----
 7 files changed, 122 insertions(+), 20 deletions(-)

backends/hostmem-ram.c +2 -2

index 9d4960b..a67a134 100644
@@ -26,8 +26,8 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     }
 
     path = object_get_canonical_path_component(OBJECT(backend));
-    memory_region_init_ram_nofail(&backend->mr, OBJECT(backend), path,
-                                  backend->size);
+    memory_region_init_ram(&backend->mr, OBJECT(backend), path,
+                           backend->size, errp);
     g_free(path);
 }
 

exec.c +23 -9

index 5a2a25e..ca7741b 100644
@@ -1224,7 +1224,7 @@ static int memory_try_enable_merging(void *addr, size_t len)
     return qemu_madvise(addr, len, QEMU_MADV_MERGEABLE);
 }
 
-static ram_addr_t ram_block_add(RAMBlock *new_block)
+static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
 {
     RAMBlock *block;
     ram_addr_t old_ram_size, new_ram_size;

@@ -1241,9 +1241,11 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
         } else {
             new_block->host = phys_mem_alloc(new_block->length);
             if (!new_block->host) {
-                fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
-                        new_block->mr->name, strerror(errno));
-                exit(1);
+                error_setg_errno(errp, errno,
+                                 "cannot set up guest memory '%s'",
+                                 new_block->mr->name);
+                qemu_mutex_unlock_ramlist();
+                return -1;
             }
             memory_try_enable_merging(new_block->host, new_block->length);
         }

@@ -1294,6 +1296,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                     Error **errp)
 {
     RAMBlock *new_block;
+    ram_addr_t addr;
 
     if (xen_enabled()) {
         error_setg(errp, "-mem-path not supported with Xen");

@@ -1323,14 +1326,20 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
         return -1;
     }
 
-    return ram_block_add(new_block);
+    addr = ram_block_add(new_block, errp);
+    if (errp && *errp) {
+        g_free(new_block);
+        return -1;
+    }
+    return addr;
 }
 #endif
 
 ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
-                                   MemoryRegion *mr)
+                                   MemoryRegion *mr, Error **errp)
 {
     RAMBlock *new_block;
+    ram_addr_t addr;
 
     size = TARGET_PAGE_ALIGN(size);
     new_block = g_malloc0(sizeof(*new_block));

@@ -1341,12 +1350,17 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
     if (host) {
         new_block->flags |= RAM_PREALLOC;
     }
-    return ram_block_add(new_block);
+    addr = ram_block_add(new_block, errp);
+    if (errp && *errp) {
+        g_free(new_block);
+        return -1;
+    }
+    return addr;
 }
 
-ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr)
+ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp)
 {
-    return qemu_ram_alloc_from_ptr(size, NULL, mr);
+    return qemu_ram_alloc_from_ptr(size, NULL, mr, errp);
 }
 
 void qemu_ram_free_from_ptr(ram_addr_t addr)

hw/block/pflash_cfi01.c +4 -1

index f9507b4..92b8b87 100644
@@ -770,7 +770,10 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     memory_region_init_rom_device(
         &pfl->mem, OBJECT(dev),
         pfl->be ? &pflash_cfi01_ops_be : &pflash_cfi01_ops_le, pfl,
-        pfl->name, total_len);
+        pfl->name, total_len, errp);
+    if (errp && *errp) {
+        return;
+    }
     vmstate_register_ram(&pfl->mem, DEVICE(pfl));
     pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);

hw/block/pflash_cfi02.c +4 -1

index 8d4b828..b773f19 100644
@@ -608,7 +608,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
 
     memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), pfl->be ?
                                   &pflash_cfi02_ops_be : &pflash_cfi02_ops_le,
-                                  pfl, pfl->name, chip_len);
+                                  pfl, pfl->name, chip_len, errp);
+    if (errp && *errp) {
+        return;
+    }
     vmstate_register_ram(&pfl->orig_mem, DEVICE(pfl));
     pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
     pfl->chip_len = chip_len;

include/exec/memory.h +38 -1

index 28c86ee..279239d 100644
@@ -304,6 +304,22 @@ void memory_region_init_io(MemoryRegion *mr,
                            uint64_t size);
 
 /**
+ * memory_region_init_ram:  Initialize RAM memory region.  Accesses into the
+ *                          region will modify memory directly.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: size of the region.
+ * @errp: pointer to Error*, to store an error if it happens.
+ */
+void memory_region_init_ram(MemoryRegion *mr,
+                            struct Object *owner,
+                            const char *name,
+                            uint64_t size,
+                            Error **errp);
+
+/**
  * memory_region_init_ram_nofail:  Initialize RAM memory region.  Accesses into
  *                                 the region will modify memory directly.
  *

@@ -340,6 +356,25 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
 #endif
 
 /**
+ * memory_region_init_ram_ptr:  Initialize RAM memory region from a
+ *                              user-provided pointer.  Accesses into the
+ *                              region will modify memory directly.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: size of the region.
+ * @ptr: memory to be mapped; must contain at least @size bytes.
+ * @errp: pointer to Error*, to store an error if it happens.
+ */
+void memory_region_init_ram_ptr(MemoryRegion *mr,
+                                struct Object *owner,
+                                const char *name,
+                                uint64_t size,
+                                void *ptr,
+                                Error **errp);
+
+/**
  * memory_region_init_ram_ptr_nofail:  Initialize RAM memory region from a
  *                                     user-provided pointer.  Accesses into the
  *                                     region will modify memory directly.

@@ -384,13 +419,15 @@ void memory_region_init_alias(MemoryRegion *mr,
  * @ops: callbacks for write access handling.
  * @name: the name of the region.
  * @size: size of the region.
+ * @errp: pointer to Error*, to store an error if it happens.
  */
 void memory_region_init_rom_device(MemoryRegion *mr,
                                    struct Object *owner,
                                    const MemoryRegionOps *ops,
                                    void *opaque,
                                    const char *name,
-                                   uint64_t size);
+                                   uint64_t size,
+                                   Error **errp);
 
 /**
  * memory_region_init_reservation: Initialize a memory region that reserves

include/exec/ram_addr.h +2 -2

index e9eb831..998ac4f 100644
@@ -26,8 +26,8 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                     bool share, const char *mem_path,
                                     Error **errp);
 ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
-                                   MemoryRegion *mr);
-ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr);
+                                   MemoryRegion *mr, Error **errp);
+ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
 int qemu_get_ram_fd(ram_addr_t addr);
 void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
 void *qemu_get_ram_ptr(ram_addr_t addr);

memory.c +49 -4

index 5f8a9f5..dc24c53 100644
@@ -25,6 +25,7 @@
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
 #include "sysemu/sysemu.h"
+#include "qemu/error-report.h"
 
 //#define DEBUG_UNASSIGNED
 

@@ -1160,16 +1161,37 @@ void memory_region_init_io(MemoryRegion *mr,
     mr->ram_addr = ~(ram_addr_t)0;
 }
 
+void memory_region_init_ram(MemoryRegion *mr,
+                            Object *owner,
+                            const char *name,
+                            uint64_t size,
+                            Error **errp)
+{
+    memory_region_init(mr, owner, name, size);
+    mr->ram = true;
+    mr->terminates = true;
+    mr->destructor = memory_region_destructor_ram;
+    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+}
+
 void memory_region_init_ram_nofail(MemoryRegion *mr,
                                    Object *owner,
                                    const char *name,
                                    uint64_t size)
 {
+    Error *local_err = NULL;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
-    mr->ram_addr = qemu_ram_alloc(size, mr);
+    mr->ram_addr = qemu_ram_alloc(size, mr, &local_err);
+
+    if (local_err) {
+        error_report("%s", error_get_pretty(local_err));
+        error_free(local_err);
+        exit(EXIT_FAILURE);
+    }
 }
 
 #ifdef __linux__

@@ -1189,17 +1211,39 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
 }
 #endif
 
+void memory_region_init_ram_ptr(MemoryRegion *mr,
+                                Object *owner,
+                                const char *name,
+                                uint64_t size,
+                                void *ptr,
+                                Error **errp)
+{
+    memory_region_init(mr, owner, name, size);
+    mr->ram = true;
+    mr->terminates = true;
+    mr->destructor = memory_region_destructor_ram_from_ptr;
+    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, errp);
+}
+
 void memory_region_init_ram_ptr_nofail(MemoryRegion *mr,
                                        Object *owner,
                                        const char *name,
                                        uint64_t size,
                                        void *ptr)
 {
+    Error *local_err = NULL;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram_from_ptr;
-    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr);
+    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, &local_err);
+
+    if (local_err) {
+        error_report("%s", error_get_pretty(local_err));
+        error_free(local_err);
+        exit(EXIT_FAILURE);
+    }
 }
 
 void memory_region_init_alias(MemoryRegion *mr,

@@ -1221,7 +1265,8 @@ void memory_region_init_rom_device(MemoryRegion *mr,
                                    const MemoryRegionOps *ops,
                                    void *opaque,
                                    const char *name,
-                                   uint64_t size)
+                                   uint64_t size,
+                                   Error **errp)
 {
     memory_region_init(mr, owner, name, size);
     mr->ops = ops;

@@ -1229,7 +1274,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
     mr->terminates = true;
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
-    mr->ram_addr = qemu_ram_alloc(size, mr);
+    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
 }
 
 void memory_region_init_iommu(MemoryRegion *mr,

Signature · 1 line

1.9.3

This patch reintroduces memory_region_init_ram() and memory_region_init_ram_ptr() which are almost the same as the renamed ones in the previous patch, except that an errp parameter is introduced to let callers handle error.

In hostmem-ram.c we call memory_region_init_ram() now rather than memory_region_init_ram_nofail() so that error can be handled.

This patch solves a problem that qemu just exits when using monitor command object_add to add a memory backend whose size is way too large. In the case we'd better give an error message and keep guest running.

The problem can be reproduced as follows:

1. run qemu
2. (monitor)object_add memory-backend-ram,size=100000G,id=ram0

Signed-off-by Hu Tao <[email protected]>

---
 backends/hostmem-ram.c  |  4 ++--
 exec.c                  | 32 ++++++++++++++++++++---------
 hw/block/pflash_cfi01.c |  5 ++++-
 hw/block/pflash_cfi02.c |  5 ++++-
 include/exec/memory.h   | 39 +++++++++++++++++++++++++++++++++++-
 include/exec/ram_addr.h |  4 ++--
 memory.c                | 53 +++++++++++++++++++++++++++++++++++++++++++++----
 7 files changed, 122 insertions(+), 20 deletions(-)

backends/hostmem-ram.c +2 -2

index 9d4960b..a67a134 100644
@@ -26,8 +26,8 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     }
 
     path = object_get_canonical_path_component(OBJECT(backend));
-    memory_region_init_ram_nofail(&backend->mr, OBJECT(backend), path,
-                                  backend->size);
+    memory_region_init_ram(&backend->mr, OBJECT(backend), path,
+                           backend->size, errp);
     g_free(path);
 }
 

exec.c +23 -9

index 5a2a25e..ca7741b 100644
@@ -1224,7 +1224,7 @@ static int memory_try_enable_merging(void *addr, size_t len)
     return qemu_madvise(addr, len, QEMU_MADV_MERGEABLE);
 }
 
-static ram_addr_t ram_block_add(RAMBlock *new_block)
+static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
 {
     RAMBlock *block;
     ram_addr_t old_ram_size, new_ram_size;

@@ -1241,9 +1241,11 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
         } else {
             new_block->host = phys_mem_alloc(new_block->length);
             if (!new_block->host) {
-                fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
-                        new_block->mr->name, strerror(errno));
-                exit(1);
+                error_setg_errno(errp, errno,
+                                 "cannot set up guest memory '%s'",
+                                 new_block->mr->name);
+                qemu_mutex_unlock_ramlist();
+                return -1;
             }
             memory_try_enable_merging(new_block->host, new_block->length);
         }

@@ -1294,6 +1296,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                     Error **errp)
 {
     RAMBlock *new_block;
+    ram_addr_t addr;
 
     if (xen_enabled()) {
         error_setg(errp, "-mem-path not supported with Xen");

@@ -1323,14 +1326,20 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
         return -1;
     }
 
-    return ram_block_add(new_block);
+    addr = ram_block_add(new_block, errp);
+    if (errp && *errp) {
+        g_free(new_block);
+        return -1;
+    }
+    return addr;
 }
 #endif
 
 ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
-                                   MemoryRegion *mr)
+                                   MemoryRegion *mr, Error **errp)
 {
     RAMBlock *new_block;
+    ram_addr_t addr;
 
     size = TARGET_PAGE_ALIGN(size);
     new_block = g_malloc0(sizeof(*new_block));

@@ -1341,12 +1350,17 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
     if (host) {
         new_block->flags |= RAM_PREALLOC;
     }
-    return ram_block_add(new_block);
+    addr = ram_block_add(new_block, errp);
+    if (errp && *errp) {
+        g_free(new_block);
+        return -1;
+    }
+    return addr;
 }
 
-ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr)
+ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp)
 {
-    return qemu_ram_alloc_from_ptr(size, NULL, mr);
+    return qemu_ram_alloc_from_ptr(size, NULL, mr, errp);
 }
 
 void qemu_ram_free_from_ptr(ram_addr_t addr)

hw/block/pflash_cfi01.c +4 -1

index f9507b4..92b8b87 100644
@@ -770,7 +770,10 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     memory_region_init_rom_device(
         &pfl->mem, OBJECT(dev),
         pfl->be ? &pflash_cfi01_ops_be : &pflash_cfi01_ops_le, pfl,
-        pfl->name, total_len);
+        pfl->name, total_len, errp);
+    if (errp && *errp) {
+        return;
+    }
     vmstate_register_ram(&pfl->mem, DEVICE(pfl));
     pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);

hw/block/pflash_cfi02.c +4 -1

index 8d4b828..b773f19 100644
@@ -608,7 +608,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
 
     memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), pfl->be ?
                                   &pflash_cfi02_ops_be : &pflash_cfi02_ops_le,
-                                  pfl, pfl->name, chip_len);
+                                  pfl, pfl->name, chip_len, errp);
+    if (errp && *errp) {
+        return;
+    }
     vmstate_register_ram(&pfl->orig_mem, DEVICE(pfl));
     pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
     pfl->chip_len = chip_len;

include/exec/memory.h +38 -1

index 28c86ee..279239d 100644
@@ -304,6 +304,22 @@ void memory_region_init_io(MemoryRegion *mr,
                            uint64_t size);
 
 /**
+ * memory_region_init_ram:  Initialize RAM memory region.  Accesses into the
+ *                          region will modify memory directly.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: size of the region.
+ * @errp: pointer to Error*, to store an error if it happens.
+ */
+void memory_region_init_ram(MemoryRegion *mr,
+                            struct Object *owner,
+                            const char *name,
+                            uint64_t size,
+                            Error **errp);
+
+/**
  * memory_region_init_ram_nofail:  Initialize RAM memory region.  Accesses into
  *                                 the region will modify memory directly.
  *

@@ -340,6 +356,25 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
 #endif
 
 /**
+ * memory_region_init_ram_ptr:  Initialize RAM memory region from a
+ *                              user-provided pointer.  Accesses into the
+ *                              region will modify memory directly.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: size of the region.
+ * @ptr: memory to be mapped; must contain at least @size bytes.
+ * @errp: pointer to Error*, to store an error if it happens.
+ */
+void memory_region_init_ram_ptr(MemoryRegion *mr,
+                                struct Object *owner,
+                                const char *name,
+                                uint64_t size,
+                                void *ptr,
+                                Error **errp);
+
+/**
  * memory_region_init_ram_ptr_nofail:  Initialize RAM memory region from a
  *                                     user-provided pointer.  Accesses into the
  *                                     region will modify memory directly.

@@ -384,13 +419,15 @@ void memory_region_init_alias(MemoryRegion *mr,
  * @ops: callbacks for write access handling.
  * @name: the name of the region.
  * @size: size of the region.
+ * @errp: pointer to Error*, to store an error if it happens.
  */
 void memory_region_init_rom_device(MemoryRegion *mr,
                                    struct Object *owner,
                                    const MemoryRegionOps *ops,
                                    void *opaque,
                                    const char *name,
-                                   uint64_t size);
+                                   uint64_t size,
+                                   Error **errp);
 
 /**
  * memory_region_init_reservation: Initialize a memory region that reserves

include/exec/ram_addr.h +2 -2

index e9eb831..998ac4f 100644
@@ -26,8 +26,8 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                     bool share, const char *mem_path,
                                     Error **errp);
 ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
-                                   MemoryRegion *mr);
-ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr);
+                                   MemoryRegion *mr, Error **errp);
+ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
 int qemu_get_ram_fd(ram_addr_t addr);
 void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
 void *qemu_get_ram_ptr(ram_addr_t addr);

memory.c +16 -0

index 5f8a9f5..dc24c53 100644
@@ -25,6 +25,7 @@
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
 #include "sysemu/sysemu.h"
+#include "qemu/error-report.h"
 
 //#define DEBUG_UNASSIGNED
 

@@ -1160,16 +1161,37 @@ void memory_region_init_io(MemoryRegion *mr,
     mr->ram_addr = ~(ram_addr_t)0;
 }
 
+void memory_region_init_ram(MemoryRegion *mr,
+                            Object *owner,
+                            const char *name,
+                            uint64_t size,
+                            Error **errp)
+{
+    memory_region_init(mr, owner, name, size);
+    mr->ram = true;
+    mr->terminates = true;
+    mr->destructor = memory_region_destructor_ram;
+    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+}
+
 void memory_region_init_ram_nofail(MemoryRegion *mr,
                                    Object *owner,
                                    const char *name,
                                    uint64_t size)
 {
+    Error *local_err = NULL;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;

memory.c +26 -3

 }
 
 #ifdef __linux__

@@ -1189,17 +1211,39 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
 }
 #endif
 
+void memory_region_init_ram_ptr(MemoryRegion *mr,
+                                Object *owner,
+                                const char *name,
+                                uint64_t size,
+                                void *ptr,
+                                Error **errp)
+{
+    memory_region_init(mr, owner, name, size);
+    mr->ram = true;
+    mr->terminates = true;
+    mr->destructor = memory_region_destructor_ram_from_ptr;
+    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, errp);
+}
+
 void memory_region_init_ram_ptr_nofail(MemoryRegion *mr,
                                        Object *owner,
                                        const char *name,
                                        uint64_t size,
                                        void *ptr)
 {
+    Error *local_err = NULL;
+
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram_from_ptr;
-    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr);
+    mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, &local_err);
+
+    if (local_err) {
+        error_report("%s", error_get_pretty(local_err));
+        error_free(local_err);
+        exit(EXIT_FAILURE);
+    }
 }
 
 void memory_region_init_alias(MemoryRegion *mr,

@@ -1221,7 +1265,8 @@ void memory_region_init_rom_device(MemoryRegion *mr,
                                    const MemoryRegionOps *ops,
                                    void *opaque,
                                    const char *name,
-                                   uint64_t size)
+                                   uint64_t size,
+                                   Error **errp)
 {
     memory_region_init(mr, owner, name, size);
     mr->ops = ops;

@@ -1229,7 +1274,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
     mr->terminates = true;
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
-    mr->ram_addr = qemu_ram_alloc(size, mr);
+    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
 }
 
 void memory_region_init_iommu(MemoryRegion *mr,

Signature · 1 line

1.9.3

This patch fixes two problems of memory-backend-file:

1. If user adds a memory-backend-file object using object_add command, specifying a non-existing directory for property mem-path, qemu will core dump with message:

/nonexistingdir: No such file or directory
Bad ram offset fffffffffffff000
Aborted (core dumped)

with this patch, qemu reports error message like:

qemu-system-x86_64: -object memory-backend-file,mem-path=/nonexistingdir,id=mem-file0,size=128M:
failed to stat file /nonexistingdir: No such file or directory

2. If user adds a memory-backend-file object using object_add command, specifying a size that is less than huge page size, qemu will core dump with message:

Bad ram offset fffffffffffff000
Aborted (core dumped)

with this patch, qemu reports error message like:

qemu-system-x86_64: -object memory-backend-file,mem-path=/hugepages,id=mem-file0,size=1M: memory size 0x100000 should be euqal or larger than huge page size 0x200000

Signed-off-by Hu Tao <[email protected]>

---
 exec.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

exec.c +1 -1

index ca7741b..9ecd9cf 100644
@@ -996,7 +996,7 @@ void qemu_mutex_unlock_ramlist(void)
 
 #define HUGETLBFS_MAGIC       0x958458f6
 
-static long gethugepagesize(const char *path)
+static long gethugepagesize(const char *path, Error **errp)
 {
     struct statfs fs;
     int ret;

exec.c +1 -1

@@ -1024,17 +1024,20 @@ static void *file_ram_alloc(RAMBlock *block,
     char *filename;
     char *sanitized_name;
     char *c;
-    void *area;
+    void *area = NULL;
     int fd;
     unsigned long hpagesize;
 

exec.c +5 -2

+                   "or larger than huge page size 0x%" PRIx64,
+                   memory, hpagesize);
+        goto error;
     }
 
     if (kvm_enabled() && !kvm_has_sync_mmu()) {

@@ -1094,8 +1097,8 @@ static void *file_ram_alloc(RAMBlock *block,
     return area;
 
 error:
-    if (mem_prealloc) {
-        exit(1);
+    if (area && area != MAP_FAILED) {
+        munmap(area, memory);
     }
     return NULL;
 }

Signature · 1 line

1.9.3

This series includes three patches to fix bugs of memory backend. Patch 1 prepares for next patches, patch 2 and patch 3 fix two bugs respectively, see each patch for the bugs and how to reproduce them.

changes to v1:

- split patch 1 in v1 into 2 patches
- don't rely on ram_block_add to return -1
- error message tweak in file_ram_alloc
- add error messages reported by qemu to commit message of patch 3

Hu Tao (3):
memory: rename memory_region_init_ram() and
memory_region_init_ram_ptr()
memory: add errp parameter to memory_region_init_ram() and
memory_region_init_ram_ptr()
exec: improve error handling and reporting in file_ram_alloc() and
gethugepagesize()

 backends/hostmem-ram.c   |  2 +-
 exec.c                   | 51 ++++++++++++++++++++++++++++---------------
 hw/block/pflash_cfi01.c  |  5 ++++-
 hw/block/pflash_cfi02.c  |  5 ++++-
 hw/core/loader.c         |  2 +-
 hw/display/vga.c         |  2 +-
 hw/display/vmware_vga.c  |  3 ++-
 hw/i386/kvm/pci-assign.c |  9 ++++----
 hw/i386/pc.c             |  2 +-
 hw/i386/pc_sysfw.c       |  4 ++--
 hw/misc/ivshmem.c        |  9 ++++----
 hw/misc/vfio.c           |  3 ++-
 hw/pci/pci.c             |  2 +-
 include/exec/memory.h    | 43 +++++++++++++++++++++++++++++++++---
 include/exec/ram_addr.h  |  4 ++--
 memory.c                 | 57 +++++++++++++++++++++++++++++++++++++++++++-----
 numa.c                   |  4 ++--
 17 files changed, 158 insertions(+), 49 deletions(-)

Signature · 1 line

1.9.3

Quoted 11 lines · Hu Tao

This series includes three patches to fix bugs of memory backend. Patch 1 prepares for next patches, patch 2 and patch 3 fix two bugs respectively, see each patch for the bugs and how to reproduce them.

changes to v1:

- split patch 1 in v1 into 2 patches
- don't rely on ram_block_add to return -1
- error message tweak in file_ram_alloc
- add error messages reported by qemu to commit message of patch 3

Quoted 7 lines

Hu Tao (3):
memory: rename memory_region_init_ram() and
memory_region_init_ram_ptr()
memory: add errp parameter to memory_region_init_ram() and
memory_region_init_ram_ptr()
exec: improve error handling and reporting in file_ram_alloc() and
gethugepagesize()

I fixed up some minor issues and applied this, thanks.

Quoted 21 lines

 backends/hostmem-ram.c   |  2 +-
 exec.c                   | 51 ++++++++++++++++++++++++++++---------------
 hw/block/pflash_cfi01.c  |  5 ++++-
 hw/block/pflash_cfi02.c  |  5 ++++-
 hw/core/loader.c         |  2 +-
 hw/display/vga.c         |  2 +-
 hw/display/vmware_vga.c  |  3 ++-
 hw/i386/kvm/pci-assign.c |  9 ++++----
 hw/i386/pc.c             |  2 +-
 hw/i386/pc_sysfw.c       |  4 ++--
 hw/misc/ivshmem.c        |  9 ++++----
 hw/misc/vfio.c           |  3 ++-
 hw/pci/pci.c             |  2 +-
 include/exec/memory.h    | 43 +++++++++++++++++++++++++++++++++---
 include/exec/ram_addr.h  |  4 ++--
 memory.c                 | 57 +++++++++++++++++++++++++++++++++++++++++++-----
 numa.c                   |  4 ++--
 17 files changed, 158 insertions(+), 49 deletions(-)

Signature · 1 line

1.9.3

Quoted 20 lines · Michael S. Tsirkin

Quoted 18 lines · Hu Tao

This series includes three patches to fix bugs of memory backend. Patch 1 prepares for next patches, patch 2 and patch 3 fix two bugs respectively, see each patch for the bugs and how to reproduce them.

changes to v1:

- split patch 1 in v1 into 2 patches
- don't rely on ram_block_add to return -1
- error message tweak in file_ram_alloc
- add error messages reported by qemu to commit message of patch 3

Hu Tao (3):
memory: rename memory_region_init_ram() and
memory_region_init_ram_ptr()
memory: add errp parameter to memory_region_init_ram() and
memory_region_init_ram_ptr()
exec: improve error handling and reporting in file_ram_alloc() and
gethugepagesize()

Quoted 1 line

I fixed up some minor issues and applied this, thanks.

And reverted.

Build fails, and a simple check after applying patch 1 gives me:
git grep memory_region_init_ram |grep -v nofail|wc -l 132

Apparently you fixed up about 10% of the files using this function. So forget about me merging patch 1.

Add a new
memory_region_init_ram_may_fail
and
memory_region_init_ram_ptr_may_fail

and use it specifically for the new stuff.

Do the rename on top in two steps:
memory_region_init_ram -> memory_region_init_ram_nofail memory_region_init_ram_may_fail -> memory_region_init_ram

Paolo can then merge it when he prefers, though I'd say 2.2 is more reasonable.

Quoted 21 lines

Quoted 21 lines

 backends/hostmem-ram.c   |  2 +-
 exec.c                   | 51 ++++++++++++++++++++++++++++---------------
 hw/block/pflash_cfi01.c  |  5 ++++-
 hw/block/pflash_cfi02.c  |  5 ++++-
 hw/core/loader.c         |  2 +-
 hw/display/vga.c         |  2 +-
 hw/display/vmware_vga.c  |  3 ++-
 hw/i386/kvm/pci-assign.c |  9 ++++----
 hw/i386/pc.c             |  2 +-
 hw/i386/pc_sysfw.c       |  4 ++--
 hw/misc/ivshmem.c        |  9 ++++----
 hw/misc/vfio.c           |  3 ++-
 hw/pci/pci.c             |  2 +-
 include/exec/memory.h    | 43 +++++++++++++++++++++++++++++++++---
 include/exec/ram_addr.h  |  4 ++--
 memory.c                 | 57 +++++++++++++++++++++++++++++++++++++++++++-----
 numa.c                   |  4 ++--
 17 files changed, 158 insertions(+), 49 deletions(-)

Signature · 1 line

1.9.3

Quoted 21 lines · Michael S. Tsirkin

Quoted 20 lines · Michael S. Tsirkin

Quoted 18 lines · Hu Tao

This series includes three patches to fix bugs of memory backend. Patch 1 prepares for next patches, patch 2 and patch 3 fix two bugs respectively, see each patch for the bugs and how to reproduce them.

changes to v1:

- split patch 1 in v1 into 2 patches
- don't rely on ram_block_add to return -1
- error message tweak in file_ram_alloc
- add error messages reported by qemu to commit message of patch 3

Hu Tao (3):
memory: rename memory_region_init_ram() and
memory_region_init_ram_ptr()
memory: add errp parameter to memory_region_init_ram() and
memory_region_init_ram_ptr()
exec: improve error handling and reporting in file_ram_alloc() and
gethugepagesize()

Quoted 7 lines

Quoted 1 line

I fixed up some minor issues and applied this, thanks.

And reverted.

Build fails, and a simple check after applying patch 1 gives me:
git grep memory_region_init_ram |grep -v nofail|wc -l 132

Thanks for catching this! I should have built all targets.

Quoted 4 lines

Apparently you fixed up about 10% of the files using this function. So forget about me merging patch 1.

Quoted 6 lines

Add a new
memory_region_init_ram_may_fail
and
memory_region_init_ram_ptr_may_fail

and use it specifically for the new stuff.

Thanks for the change!

Quoted 30 lines

Do the rename on top in two steps:
memory_region_init_ram -> memory_region_init_ram_nofail memory_region_init_ram_may_fail -> memory_region_init_ram

Paolo can then merge it when he prefers, though I'd say 2.2 is more reasonable.

Quoted 21 lines

Quoted 21 lines

 backends/hostmem-ram.c   |  2 +-
 exec.c                   | 51 ++++++++++++++++++++++++++++---------------
 hw/block/pflash_cfi01.c  |  5 ++++-
 hw/block/pflash_cfi02.c  |  5 ++++-
 hw/core/loader.c         |  2 +-
 hw/display/vga.c         |  2 +-
 hw/display/vmware_vga.c  |  3 ++-
 hw/i386/kvm/pci-assign.c |  9 ++++----
 hw/i386/pc.c             |  2 +-
 hw/i386/pc_sysfw.c       |  4 ++--
 hw/misc/ivshmem.c        |  9 ++++----
 hw/misc/vfio.c           |  3 ++-
 hw/pci/pci.c             |  2 +-
 include/exec/memory.h    | 43 +++++++++++++++++++++++++++++++++---
 include/exec/ram_addr.h  |  4 ++--
 memory.c                 | 57 +++++++++++++++++++++++++++++++++++++++++++-----
 numa.c                   |  4 ++--
 17 files changed, 158 insertions(+), 49 deletions(-)

Signature · 1 line

1.9.3