* [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
* 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 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
* [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 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
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).