qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend
@ 2014-07-07 10:55 Hu Tao
  2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 1/2] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail() Hu Tao
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Hu Tao @ 2014-07-07 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yasunori Goto, Michael S. Tsirkin, Paolo Bonzini, Igor Mammedov

This series includes two patches to fix bugs of memory backend. See each
patch for the bugs and how to reproduce them.

changes to v2:

- introduce memory_region_init_ram_may_fail and
  memory_region_init_ram_ptr_may_fail

- address comments by MST

- missing the functions renaming. will send later.

Hu Tao (2):
  memory: add memory_region_init_ram_may_fail() and    
    memory_region_init_ram_ptr_may_fail()
  exec: improve error handling and reporting in file_ram_alloc() and
    gethugepagesize()

 backends/hostmem-ram.c  |  4 ++--
 exec.c                  | 51 +++++++++++++++++++++++++++--------------
 hw/block/pflash_cfi01.c |  5 +++-
 hw/block/pflash_cfi02.c |  5 +++-
 include/exec/memory.h   | 40 +++++++++++++++++++++++++++++++-
 include/exec/ram_addr.h |  4 ++--
 memory.c                | 61 +++++++++++++++++++++++++++++++++++++++----------
 7 files changed, 134 insertions(+), 36 deletions(-)

-- 
1.9.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH V3 for 2.1 1/2] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail()
  2014-07-07 10:55 [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend Hu Tao
@ 2014-07-07 10:55 ` Hu Tao
  2014-07-07 12:27   ` Michael S. Tsirkin
  2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 2/2] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize() Hu Tao
  2014-07-07 12:39 ` [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend Michael S. Tsirkin
  2 siblings, 1 reply; 7+ messages in thread
From: Hu Tao @ 2014-07-07 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yasunori Goto, Michael S. Tsirkin, Paolo Bonzini, Igor Mammedov

These two are almost the same as memory_region_init_ram() and
memory_region_init_ram_ptr() except that they have an extra errp
parameter to let callers handle error.

In hostmem-ram.c we call memory_region_init_ram_may_fail() now rather
than memory_region_init_ram() 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 <hutao@cn.fujitsu.com>
---
 backends/hostmem-ram.c  |  4 ++--
 exec.c                  | 32 ++++++++++++++++++--------
 hw/block/pflash_cfi01.c |  5 +++-
 hw/block/pflash_cfi02.c |  5 +++-
 include/exec/memory.h   | 40 +++++++++++++++++++++++++++++++-
 include/exec/ram_addr.h |  4 ++--
 memory.c                | 61 +++++++++++++++++++++++++++++++++++++++----------
 7 files changed, 123 insertions(+), 28 deletions(-)

diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index d9a8290..d0e92ad 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -26,8 +26,8 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     }
 
     path = object_get_canonical_path_component(OBJECT(backend));
-    memory_region_init_ram(&backend->mr, OBJECT(backend), path,
-                           backend->size);
+    memory_region_init_ram_may_fail(&backend->mr, OBJECT(backend), path,
+                                    backend->size, errp);
     g_free(path);
 }
 
diff --git a/exec.c b/exec.c
index 5a2a25e..ca7741b 100644
--- a/exec.c
+++ b/exec.c
@@ -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)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index f9507b4..92b8b87 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -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);
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 8d4b828..b773f19 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -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;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e2c8e3e..c720d43 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -304,6 +304,23 @@ void memory_region_init_io(MemoryRegion *mr,
                            uint64_t size);
 
 /**
+ * memory_region_init_ram_may_fail:  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_may_fail(MemoryRegion *mr,
+                                     struct Object *owner,
+                                     const char *name,
+                                     uint64_t size,
+                                     Error **errp);
+
+/**
  * memory_region_init_ram:  Initialize RAM memory region.  Accesses into the
  *                          region will modify memory directly.
  *
@@ -340,6 +357,25 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
 #endif
 
 /**
+ * memory_region_init_ram_ptr_may_fail:  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_may_fail(MemoryRegion *mr,
+                                         struct Object *owner,
+                                         const char *name,
+                                         uint64_t size,
+                                         void *ptr,
+                                         Error **errp);
+
+/**
  * memory_region_init_ram_ptr:  Initialize RAM memory region from a
  *                              user-provided pointer.  Accesses into the
  *                              region will modify memory directly.
@@ -384,13 +420,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
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index e9eb831..998ac4f 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -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);
diff --git a/memory.c b/memory.c
index 64d7176..6b1f6c3 100644
--- a/memory.c
+++ b/memory.c
@@ -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,33 @@ 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)
+void memory_region_init_ram_may_fail(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);
+    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+}
+
+void memory_region_init_ram(MemoryRegion *mr,
+                            Object *owner,
+                            const char *name,
+                            uint64_t size)
+{
+    Error *local_err = NULL;
+
+    memory_region_init_ram_may_fail(mr, owner, name, size, &local_err);
+
+    if (local_err) {
+        error_report("%s", error_get_pretty(local_err));
+        error_free(local_err);
+        exit(EXIT_FAILURE);
+    }
 }
 
 #ifdef __linux__
@@ -1189,17 +1207,35 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
 }
 #endif
 
+void memory_region_init_ram_ptr_may_fail(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(MemoryRegion *mr,
                                 Object *owner,
                                 const char *name,
                                 uint64_t size,
                                 void *ptr)
 {
-    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);
+    Error *local_err = NULL;
+
+    memory_region_init_ram_ptr_may_fail(mr, owner, name, size, ptr, &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 +1257,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 +1266,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,
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH V3 for 2.1 2/2] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize()
  2014-07-07 10:55 [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend Hu Tao
  2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 1/2] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail() Hu Tao
@ 2014-07-07 10:55 ` Hu Tao
  2014-07-07 12:38   ` Michael S. Tsirkin
  2014-07-07 12:39 ` [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend Michael S. Tsirkin
  2 siblings, 1 reply; 7+ messages in thread
From: Hu Tao @ 2014-07-07 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yasunori Goto, Michael S. Tsirkin, Paolo Bonzini, Igor Mammedov

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 <hutao@cn.fujitsu.com>
---
 exec.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/exec.c b/exec.c
index ca7741b..bb97b15 100644
--- a/exec.c
+++ b/exec.c
@@ -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;
@@ -1006,7 +1006,7 @@ static long gethugepagesize(const char *path)
     } while (ret != 0 && errno == EINTR);
 
     if (ret != 0) {
-        perror(path);
+        error_setg_errno(errp, errno, "failed to get size of file %s", path);
         return 0;
     }
 
@@ -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;
 
-    hpagesize = gethugepagesize(path);
-    if (!hpagesize) {
+    hpagesize = gethugepagesize(path, errp);
+    if (errp && *errp) {
         goto error;
     }
 
     if (memory < hpagesize) {
-        return NULL;
+        error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be euqal to "
+                   "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;
 }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH V3 for 2.1 1/2] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail()
  2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 1/2] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail() Hu Tao
@ 2014-07-07 12:27   ` Michael S. Tsirkin
  2014-07-11  8:40     ` Hu Tao
  0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 12:27 UTC (permalink / raw)
  To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov

On Mon, Jul 07, 2014 at 06:55:27PM +0800, Hu Tao wrote:
> These two are almost the same as memory_region_init_ram() and
> memory_region_init_ram_ptr() except that they have an extra errp
> parameter to let callers handle error.
> 
> In hostmem-ram.c we call memory_region_init_ram_may_fail() now rather
> than memory_region_init_ram() 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 <hutao@cn.fujitsu.com>
> ---
>  backends/hostmem-ram.c  |  4 ++--
>  exec.c                  | 32 ++++++++++++++++++--------
>  hw/block/pflash_cfi01.c |  5 +++-
>  hw/block/pflash_cfi02.c |  5 +++-

Why do we need to patch pflash?
We can't trigger it with object_add, can we?

>  include/exec/memory.h   | 40 +++++++++++++++++++++++++++++++-
>  include/exec/ram_addr.h |  4 ++--
>  memory.c                | 61 +++++++++++++++++++++++++++++++++++++++----------
>  7 files changed, 123 insertions(+), 28 deletions(-)
> 
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index d9a8290..d0e92ad 100644
> --- a/backends/hostmem-ram.c
> +++ b/backends/hostmem-ram.c
> @@ -26,8 +26,8 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>      }
>  
>      path = object_get_canonical_path_component(OBJECT(backend));
> -    memory_region_init_ram(&backend->mr, OBJECT(backend), path,
> -                           backend->size);
> +    memory_region_init_ram_may_fail(&backend->mr, OBJECT(backend), path,
> +                                    backend->size, errp);
>      g_free(path);
>  }
>  
> diff --git a/exec.c b/exec.c
> index 5a2a25e..ca7741b 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -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)
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index f9507b4..92b8b87 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -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);
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 8d4b828..b773f19 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -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;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index e2c8e3e..c720d43 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -304,6 +304,23 @@ void memory_region_init_io(MemoryRegion *mr,
>                             uint64_t size);
>  
>  /**
> + * memory_region_init_ram_may_fail:  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_may_fail(MemoryRegion *mr,
> +                                     struct Object *owner,
> +                                     const char *name,
> +                                     uint64_t size,
> +                                     Error **errp);
> +
> +/**
>   * memory_region_init_ram:  Initialize RAM memory region.  Accesses into the
>   *                          region will modify memory directly.
>   *
> @@ -340,6 +357,25 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
>  #endif
>  
>  /**
> + * memory_region_init_ram_ptr_may_fail:  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_may_fail(MemoryRegion *mr,
> +                                         struct Object *owner,
> +                                         const char *name,
> +                                         uint64_t size,
> +                                         void *ptr,
> +                                         Error **errp);
> +
> +/**
>   * memory_region_init_ram_ptr:  Initialize RAM memory region from a
>   *                              user-provided pointer.  Accesses into the
>   *                              region will modify memory directly.
> @@ -384,13 +420,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
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index e9eb831..998ac4f 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -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);
> diff --git a/memory.c b/memory.c
> index 64d7176..6b1f6c3 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -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,33 @@ 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)
> +void memory_region_init_ram_may_fail(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);
> +    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
> +}
> +
> +void memory_region_init_ram(MemoryRegion *mr,
> +                            Object *owner,
> +                            const char *name,
> +                            uint64_t size)
> +{
> +    Error *local_err = NULL;
> +
> +    memory_region_init_ram_may_fail(mr, owner, name, size, &local_err);
> +
> +    if (local_err) {
> +        error_report("%s", error_get_pretty(local_err));
> +        error_free(local_err);
> +        exit(EXIT_FAILURE);
> +    }
>  }
>  
>  #ifdef __linux__
> @@ -1189,17 +1207,35 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
>  }
>  #endif
>  
> +void memory_region_init_ram_ptr_may_fail(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(MemoryRegion *mr,
>                                  Object *owner,
>                                  const char *name,
>                                  uint64_t size,
>                                  void *ptr)
>  {
> -    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);
> +    Error *local_err = NULL;
> +
> +    memory_region_init_ram_ptr_may_fail(mr, owner, name, size, ptr, &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 +1257,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 +1266,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,
> -- 
> 1.9.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH V3 for 2.1 2/2] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize()
  2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 2/2] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize() Hu Tao
@ 2014-07-07 12:38   ` Michael S. Tsirkin
  0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 12:38 UTC (permalink / raw)
  To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov

On Mon, Jul 07, 2014 at 06:55:28PM +0800, Hu Tao wrote:
> 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 <hutao@cn.fujitsu.com>

Build fails on 32 bit host
/scm/qemu/exec.c:1037:9: error: format ‘%llx’ expects argument of type
‘long long unsigned int’, but argument 5 has type ‘long unsigned int’
[-Werror=format=]


> ---
>  exec.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index ca7741b..bb97b15 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -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;
> @@ -1006,7 +1006,7 @@ static long gethugepagesize(const char *path)
>      } while (ret != 0 && errno == EINTR);
>  
>      if (ret != 0) {
> -        perror(path);
> +        error_setg_errno(errp, errno, "failed to get size of file %s", path);
>          return 0;
>      }
>  
> @@ -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;
>  
> -    hpagesize = gethugepagesize(path);
> -    if (!hpagesize) {
> +    hpagesize = gethugepagesize(path, errp);
> +    if (errp && *errp) {
>          goto error;
>      }
>  
>      if (memory < hpagesize) {
> -        return NULL;
> +        error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be euqal to "
> +                   "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;
>  }
> -- 
> 1.9.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend
  2014-07-07 10:55 [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend Hu Tao
  2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 1/2] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail() Hu Tao
  2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 2/2] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize() Hu Tao
@ 2014-07-07 12:39 ` Michael S. Tsirkin
  2 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 12:39 UTC (permalink / raw)
  To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov

On Mon, Jul 07, 2014 at 06:55:26PM +0800, Hu Tao wrote:
> This series includes two patches to fix bugs of memory backend. See each
> patch for the bugs and how to reproduce them.
> 
> changes to v2:
> 
> - introduce memory_region_init_ram_may_fail and
>   memory_region_init_ram_ptr_may_fail
> 
> - address comments by MST
> 
> - missing the functions renaming. will send later.
> 
> Hu Tao (2):
>   memory: add memory_region_init_ram_may_fail() and    
>     memory_region_init_ram_ptr_may_fail()
>   exec: improve error handling and reporting in file_ram_alloc() and
>     gethugepagesize()

I'm not merging this for 2.1, sorry.
There's a simple workaround, we can fix this in 2.2.


>  backends/hostmem-ram.c  |  4 ++--
>  exec.c                  | 51 +++++++++++++++++++++++++++--------------
>  hw/block/pflash_cfi01.c |  5 +++-
>  hw/block/pflash_cfi02.c |  5 +++-
>  include/exec/memory.h   | 40 +++++++++++++++++++++++++++++++-
>  include/exec/ram_addr.h |  4 ++--
>  memory.c                | 61 +++++++++++++++++++++++++++++++++++++++----------
>  7 files changed, 134 insertions(+), 36 deletions(-)
> 
> -- 
> 1.9.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH V3 for 2.1 1/2] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail()
  2014-07-07 12:27   ` Michael S. Tsirkin
@ 2014-07-11  8:40     ` Hu Tao
  0 siblings, 0 replies; 7+ messages in thread
From: Hu Tao @ 2014-07-11  8:40 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov

On Mon, Jul 07, 2014 at 03:27:35PM +0300, Michael S. Tsirkin wrote:
> On Mon, Jul 07, 2014 at 06:55:27PM +0800, Hu Tao wrote:
> > These two are almost the same as memory_region_init_ram() and
> > memory_region_init_ram_ptr() except that they have an extra errp
> > parameter to let callers handle error.
> > 
> > In hostmem-ram.c we call memory_region_init_ram_may_fail() now rather
> > than memory_region_init_ram() 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 <hutao@cn.fujitsu.com>
> > ---
> >  backends/hostmem-ram.c  |  4 ++--
> >  exec.c                  | 32 ++++++++++++++++++--------
> >  hw/block/pflash_cfi01.c |  5 +++-
> >  hw/block/pflash_cfi02.c |  5 +++-
> 
> Why do we need to patch pflash?
> We can't trigger it with object_add, can we?

pflash calls memory_region_init_rom_device which calls qemu_ram_alloc
to which an errp is added. so I added an errp to
memory_region_init_rom_device too. The change is small so I don't think
it is worth splitting it into a single patch.

Or we can make memory_region_init_rom_device nofail thus avoid patching
pflash. Even if later we decide to add errp to
memory_region_init_rom_device the patch won't be big since there is
little users.

> 
> >  include/exec/memory.h   | 40 +++++++++++++++++++++++++++++++-
> >  include/exec/ram_addr.h |  4 ++--
> >  memory.c                | 61 +++++++++++++++++++++++++++++++++++++++----------
> >  7 files changed, 123 insertions(+), 28 deletions(-)
> > 
> > diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> > index d9a8290..d0e92ad 100644
> > --- a/backends/hostmem-ram.c
> > +++ b/backends/hostmem-ram.c
> > @@ -26,8 +26,8 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> >      }
> >  
> >      path = object_get_canonical_path_component(OBJECT(backend));
> > -    memory_region_init_ram(&backend->mr, OBJECT(backend), path,
> > -                           backend->size);
> > +    memory_region_init_ram_may_fail(&backend->mr, OBJECT(backend), path,
> > +                                    backend->size, errp);
> >      g_free(path);
> >  }
> >  
> > diff --git a/exec.c b/exec.c
> > index 5a2a25e..ca7741b 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -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)
> > diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> > index f9507b4..92b8b87 100644
> > --- a/hw/block/pflash_cfi01.c
> > +++ b/hw/block/pflash_cfi01.c
> > @@ -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);
> > diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> > index 8d4b828..b773f19 100644
> > --- a/hw/block/pflash_cfi02.c
> > +++ b/hw/block/pflash_cfi02.c
> > @@ -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;
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index e2c8e3e..c720d43 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -304,6 +304,23 @@ void memory_region_init_io(MemoryRegion *mr,
> >                             uint64_t size);
> >  
> >  /**
> > + * memory_region_init_ram_may_fail:  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_may_fail(MemoryRegion *mr,
> > +                                     struct Object *owner,
> > +                                     const char *name,
> > +                                     uint64_t size,
> > +                                     Error **errp);
> > +
> > +/**
> >   * memory_region_init_ram:  Initialize RAM memory region.  Accesses into the
> >   *                          region will modify memory directly.
> >   *
> > @@ -340,6 +357,25 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
> >  #endif
> >  
> >  /**
> > + * memory_region_init_ram_ptr_may_fail:  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_may_fail(MemoryRegion *mr,
> > +                                         struct Object *owner,
> > +                                         const char *name,
> > +                                         uint64_t size,
> > +                                         void *ptr,
> > +                                         Error **errp);
> > +
> > +/**
> >   * memory_region_init_ram_ptr:  Initialize RAM memory region from a
> >   *                              user-provided pointer.  Accesses into the
> >   *                              region will modify memory directly.
> > @@ -384,13 +420,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
> > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> > index e9eb831..998ac4f 100644
> > --- a/include/exec/ram_addr.h
> > +++ b/include/exec/ram_addr.h
> > @@ -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);
> > diff --git a/memory.c b/memory.c
> > index 64d7176..6b1f6c3 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -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,33 @@ 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)
> > +void memory_region_init_ram_may_fail(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);
> > +    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
> > +}
> > +
> > +void memory_region_init_ram(MemoryRegion *mr,
> > +                            Object *owner,
> > +                            const char *name,
> > +                            uint64_t size)
> > +{
> > +    Error *local_err = NULL;
> > +
> > +    memory_region_init_ram_may_fail(mr, owner, name, size, &local_err);
> > +
> > +    if (local_err) {
> > +        error_report("%s", error_get_pretty(local_err));
> > +        error_free(local_err);
> > +        exit(EXIT_FAILURE);
> > +    }
> >  }
> >  
> >  #ifdef __linux__
> > @@ -1189,17 +1207,35 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
> >  }
> >  #endif
> >  
> > +void memory_region_init_ram_ptr_may_fail(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(MemoryRegion *mr,
> >                                  Object *owner,
> >                                  const char *name,
> >                                  uint64_t size,
> >                                  void *ptr)
> >  {
> > -    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);
> > +    Error *local_err = NULL;
> > +
> > +    memory_region_init_ram_ptr_may_fail(mr, owner, name, size, ptr, &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 +1257,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 +1266,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,
> > -- 
> > 1.9.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-07-11  8:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-07 10:55 [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend Hu Tao
2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 1/2] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail() Hu Tao
2014-07-07 12:27   ` Michael S. Tsirkin
2014-07-11  8:40     ` Hu Tao
2014-07-07 10:55 ` [Qemu-devel] [PATCH V3 for 2.1 2/2] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize() Hu Tao
2014-07-07 12:38   ` Michael S. Tsirkin
2014-07-07 12:39 ` [Qemu-devel] [PATCH V3 for 2.1 0/2] bug fixs for memory backend Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).