* [Qemu-devel] [PATCH V2 for 2.1 1/3] memory: rename memory_region_init_ram() and memory_region_init_ram_ptr()
2014-07-07 2:58 [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend Hu Tao
@ 2014-07-07 2:58 ` Hu Tao
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 2/3] memory: add errp parameter to " Hu Tao
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Hu Tao @ 2014-07-07 2:58 UTC (permalink / raw)
To: qemu-devel
Cc: Yasunori Goto, Michael S. Tsirkin, Paolo Bonzini, Igor Mammedov
Rename memory_region_init_ram() and memory_region_init_ram_ptr() to
memory_region_init_ram_nofail() and memory_region_init_ram_ptr_nofail().
These are nofail versions. We're going to introduce normal versions
with errp to let callers handle the error.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem-ram.c | 4 ++--
hw/core/loader.c | 2 +-
hw/display/vga.c | 2 +-
hw/display/vmware_vga.c | 3 ++-
hw/i386/kvm/pci-assign.c | 9 +++++----
hw/i386/pc.c | 2 +-
hw/i386/pc_sysfw.c | 4 ++--
hw/misc/ivshmem.c | 9 +++++----
hw/misc/vfio.c | 3 ++-
hw/pci/pci.c | 2 +-
include/exec/memory.h | 28 ++++++++++++++--------------
memory.c | 18 +++++++++---------
numa.c | 4 ++--
13 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index d9a8290..9d4960b 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_nofail(&backend->mr, OBJECT(backend), path,
+ backend->size);
g_free(path);
}
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..fdebf86 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -632,7 +632,7 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name)
void *data;
rom->mr = g_malloc(sizeof(*rom->mr));
- memory_region_init_ram(rom->mr, owner, name, rom->datasize);
+ memory_region_init_ram_nofail(rom->mr, owner, name, rom->datasize);
memory_region_set_readonly(rom->mr, true);
vmstate_register_ram_global(rom->mr);
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 4b089a3..f17a4b4 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2291,7 +2291,7 @@ void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate)
s->vram_size_mb = s->vram_size >> 20;
s->is_vbe_vmstate = 1;
- memory_region_init_ram(&s->vram, obj, "vga.vram", s->vram_size);
+ memory_region_init_ram_nofail(&s->vram, obj, "vga.vram", s->vram_size);
vmstate_register_ram(&s->vram, global_vmstate ? NULL : DEVICE(obj));
xen_register_framebuffer(&s->vram);
s->vram_ptr = memory_region_get_ram_ptr(&s->vram);
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 591b645..a3738be 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -1201,7 +1201,8 @@ static void vmsvga_init(DeviceState *dev, struct vmsvga_state_s *s,
s->vga.con = graphic_console_init(dev, 0, &vmsvga_ops, s);
s->fifo_size = SVGA_FIFO_SIZE;
- memory_region_init_ram(&s->fifo_ram, NULL, "vmsvga.fifo", s->fifo_size);
+ memory_region_init_ram_nofail(&s->fifo_ram, NULL, "vmsvga.fifo",
+ s->fifo_size);
vmstate_register_ram_global(&s->fifo_ram);
s->fifo_ptr = memory_region_get_ram_ptr(&s->fifo_ram);
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index de33657..52d0db0 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -454,9 +454,10 @@ static void assigned_dev_register_regions(PCIRegion *io_regions,
char name[32];
snprintf(name, sizeof(name), "%s.bar%d",
object_get_typename(OBJECT(pci_dev)), i);
- memory_region_init_ram_ptr(&pci_dev->v_addrs[i].real_iomem,
- OBJECT(pci_dev), name,
- cur_region->size, virtbase);
+ memory_region_init_ram_ptr_nofail(
+ &pci_dev->v_addrs[i].real_iomem,
+ OBJECT(pci_dev), name,
+ cur_region->size, virtbase);
vmstate_register_ram(&pci_dev->v_addrs[i].real_iomem,
&pci_dev->dev.qdev);
}
@@ -1943,7 +1944,7 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev)
snprintf(name, sizeof(name), "%s.rom",
object_get_typename(OBJECT(dev)));
- memory_region_init_ram(&dev->dev.rom, OBJECT(dev), name, st.st_size);
+ memory_region_init_ram_nofail(&dev->dev.rom, OBJECT(dev), name, st.st_size);
vmstate_register_ram(&dev->dev.rom, &dev->dev.qdev);
ptr = memory_region_get_ram_ptr(&dev->dev.rom);
memset(ptr, 0xff, st.st_size);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2cf22b1..f503b0e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1272,7 +1272,7 @@ FWCfgState *pc_memory_init(MachineState *machine,
pc_system_firmware_init(rom_memory, guest_info->isapc_ram_fw);
option_rom_mr = g_malloc(sizeof(*option_rom_mr));
- memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
+ memory_region_init_ram_nofail(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
vmstate_register_ram_global(option_rom_mr);
memory_region_add_subregion_overlap(rom_memory,
PC_ROM_MIN_VGA,
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 75a7ebb..a15963b 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -55,7 +55,7 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory,
/* map the last 128KB of the BIOS in ISA space */
isa_bios_size = MIN(flash_size, 128 * 1024);
isa_bios = g_malloc(sizeof(*isa_bios));
- memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size);
+ memory_region_init_ram_nofail(isa_bios, NULL, "isa-bios", isa_bios_size);
vmstate_register_ram_global(isa_bios);
memory_region_add_subregion_overlap(rom_memory,
0x100000 - isa_bios_size,
@@ -192,7 +192,7 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
goto bios_error;
}
bios = g_malloc(sizeof(*bios));
- memory_region_init_ram(bios, NULL, "pc.bios", bios_size);
+ memory_region_init_ram_nofail(bios, NULL, "pc.bios", bios_size);
vmstate_register_ram_global(bios);
if (!isapc_ram_fw) {
memory_region_set_readonly(bios, true);
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 768e528..e3b6e06 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -347,8 +347,8 @@ static void create_shared_memory_BAR(IVShmemState *s, int fd) {
ptr = mmap(0, s->ivshmem_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
- memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s), "ivshmem.bar2",
- s->ivshmem_size, ptr);
+ memory_region_init_ram_ptr_nofail(&s->ivshmem, OBJECT(s), "ivshmem.bar2",
+ s->ivshmem_size, ptr);
vmstate_register_ram(&s->ivshmem, DEVICE(s));
memory_region_add_subregion(&s->bar, 0, &s->ivshmem);
@@ -475,8 +475,9 @@ static void ivshmem_read(void *opaque, const uint8_t * buf, int flags)
/* mmap the region and map into the BAR2 */
map_ptr = mmap(0, s->ivshmem_size, PROT_READ|PROT_WRITE, MAP_SHARED,
incoming_fd, 0);
- memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s),
- "ivshmem.bar2", s->ivshmem_size, map_ptr);
+ memory_region_init_ram_ptr_nofail(&s->ivshmem, OBJECT(s),
+ "ivshmem.bar2", s->ivshmem_size,
+ map_ptr);
vmstate_register_ram(&s->ivshmem, DEVICE(s));
IVSHMEM_DPRINTF("guest h/w addr = %" PRIu64 ", size = %" PRIu64 "\n",
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index aef4c9c..5bdee3e 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -2894,7 +2894,8 @@ static int vfio_mmap_bar(VFIODevice *vdev, VFIOBAR *bar,
goto empty_region;
}
- memory_region_init_ram_ptr(submem, OBJECT(vdev), name, size, *map);
+ memory_region_init_ram_ptr_nofail(submem, OBJECT(vdev), name, size,
+ *map);
} else {
empty_region:
/* Create a zero sized sub-region to make cleanup easy. */
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 17ed510..ba39f08 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1974,7 +1974,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
snprintf(name, sizeof(name), "%s.rom", object_get_typename(OBJECT(pdev)));
}
pdev->has_rom = true;
- memory_region_init_ram(&pdev->rom, OBJECT(pdev), name, size);
+ memory_region_init_ram_nofail(&pdev->rom, OBJECT(pdev), name, size);
vmstate_register_ram(&pdev->rom, &pdev->qdev);
ptr = memory_region_get_ram_ptr(&pdev->rom);
load_image(path, ptr);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e2c8e3e..28c86ee 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -304,18 +304,18 @@ void memory_region_init_io(MemoryRegion *mr,
uint64_t size);
/**
- * memory_region_init_ram: Initialize RAM memory region. Accesses into the
- * region will modify memory directly.
+ * memory_region_init_ram_nofail: 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.
*/
-void memory_region_init_ram(MemoryRegion *mr,
- struct Object *owner,
- const char *name,
- uint64_t size);
+void memory_region_init_ram_nofail(MemoryRegion *mr,
+ struct Object *owner,
+ const char *name,
+ uint64_t size);
#ifdef __linux__
/**
@@ -340,9 +340,9 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
#endif
/**
- * memory_region_init_ram_ptr: Initialize RAM memory region from a
- * user-provided pointer. Accesses into the
- * region will modify memory directly.
+ * memory_region_init_ram_ptr_nofail: 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
@@ -350,11 +350,11 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
* @size: size of the region.
* @ptr: memory to be mapped; must contain at least @size bytes.
*/
-void memory_region_init_ram_ptr(MemoryRegion *mr,
- struct Object *owner,
- const char *name,
- uint64_t size,
- void *ptr);
+void memory_region_init_ram_ptr_nofail(MemoryRegion *mr,
+ struct Object *owner,
+ const char *name,
+ uint64_t size,
+ void *ptr);
/**
* memory_region_init_alias: Initialize a memory region that aliases all or a
diff --git a/memory.c b/memory.c
index 64d7176..5f8a9f5 100644
--- a/memory.c
+++ b/memory.c
@@ -1160,10 +1160,10 @@ 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_nofail(MemoryRegion *mr,
+ Object *owner,
+ const char *name,
+ uint64_t size)
{
memory_region_init(mr, owner, name, size);
mr->ram = true;
@@ -1189,11 +1189,11 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
}
#endif
-void memory_region_init_ram_ptr(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size,
- void *ptr)
+void memory_region_init_ram_ptr_nofail(MemoryRegion *mr,
+ Object *owner,
+ const char *name,
+ uint64_t size,
+ void *ptr)
{
memory_region_init(mr, owner, name, size);
mr->ram = true;
diff --git a/numa.c b/numa.c
index 2fde740..dabba4f 100644
--- a/numa.c
+++ b/numa.c
@@ -263,14 +263,14 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
if (err) {
qerror_report_err(err);
error_free(err);
- memory_region_init_ram(mr, owner, name, ram_size);
+ memory_region_init_ram_nofail(mr, owner, name, ram_size);
}
#else
fprintf(stderr, "-mem-path not supported on this host\n");
exit(1);
#endif
} else {
- memory_region_init_ram(mr, owner, name, ram_size);
+ memory_region_init_ram_nofail(mr, owner, name, ram_size);
}
vmstate_register_ram_global(mr);
}
--
1.9.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH V2 for 2.1 2/3] memory: add errp parameter to memory_region_init_ram() and memory_region_init_ram_ptr()
2014-07-07 2:58 [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend Hu Tao
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 1/3] memory: rename memory_region_init_ram() and memory_region_init_ram_ptr() Hu Tao
@ 2014-07-07 2:58 ` Hu Tao
2014-07-07 3:20 ` Michael S. Tsirkin
2014-07-07 5:39 ` Michael S. Tsirkin
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 3/3] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize() Hu Tao
2014-07-07 3:24 ` [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend Michael S. Tsirkin
3 siblings, 2 replies; 11+ messages in thread
From: Hu Tao @ 2014-07-07 2:58 UTC (permalink / raw)
To: qemu-devel
Cc: Yasunori Goto, Michael S. Tsirkin, Paolo Bonzini, Igor Mammedov
This patch reintroduces memory_region_init_ram() and
memory_region_init_ram_ptr() which are almost the same as the renamed
ones in the previous patch, except that an errp parameter is introduced
to let callers handle error.
In hostmem-ram.c we call memory_region_init_ram() now rather than
memory_region_init_ram_nofail() so that error can be handled.
This patch solves a problem that qemu just exits when using monitor
command object_add to add a memory backend whose size is way too large.
In the case we'd better give an error message and keep guest running.
The problem can be reproduced as follows:
1. run qemu
2. (monitor)object_add memory-backend-ram,size=100000G,id=ram0
Signed-off-by: Hu Tao <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 | 39 +++++++++++++++++++++++++++++++++++-
include/exec/ram_addr.h | 4 ++--
memory.c | 53 +++++++++++++++++++++++++++++++++++++++++++++----
7 files changed, 122 insertions(+), 20 deletions(-)
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 9d4960b..a67a134 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_nofail(&backend->mr, OBJECT(backend), path,
- backend->size);
+ memory_region_init_ram(&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 28c86ee..279239d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -304,6 +304,22 @@ void memory_region_init_io(MemoryRegion *mr,
uint64_t size);
/**
+ * memory_region_init_ram: Initialize RAM memory region. Accesses into the
+ * region will modify memory directly.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: size of the region.
+ * @errp: pointer to Error*, to store an error if it happens.
+ */
+void memory_region_init_ram(MemoryRegion *mr,
+ struct Object *owner,
+ const char *name,
+ uint64_t size,
+ Error **errp);
+
+/**
* memory_region_init_ram_nofail: Initialize RAM memory region. Accesses into
* the region will modify memory directly.
*
@@ -340,6 +356,25 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
#endif
/**
+ * memory_region_init_ram_ptr: Initialize RAM memory region from a
+ * user-provided pointer. Accesses into the
+ * region will modify memory directly.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: size of the region.
+ * @ptr: memory to be mapped; must contain at least @size bytes.
+ * @errp: pointer to Error*, to store an error if it happens.
+ */
+void memory_region_init_ram_ptr(MemoryRegion *mr,
+ struct Object *owner,
+ const char *name,
+ uint64_t size,
+ void *ptr,
+ Error **errp);
+
+/**
* memory_region_init_ram_ptr_nofail: Initialize RAM memory region from a
* user-provided pointer. Accesses into the
* region will modify memory directly.
@@ -384,13 +419,15 @@ void memory_region_init_alias(MemoryRegion *mr,
* @ops: callbacks for write access handling.
* @name: the name of the region.
* @size: size of the region.
+ * @errp: pointer to Error*, to store an error if it happens.
*/
void memory_region_init_rom_device(MemoryRegion *mr,
struct Object *owner,
const MemoryRegionOps *ops,
void *opaque,
const char *name,
- uint64_t size);
+ uint64_t size,
+ Error **errp);
/**
* memory_region_init_reservation: Initialize a memory region that reserves
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 5f8a9f5..dc24c53 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,37 @@ void memory_region_init_io(MemoryRegion *mr,
mr->ram_addr = ~(ram_addr_t)0;
}
+void memory_region_init_ram(MemoryRegion *mr,
+ Object *owner,
+ const char *name,
+ uint64_t size,
+ Error **errp)
+{
+ memory_region_init(mr, owner, name, size);
+ mr->ram = true;
+ mr->terminates = true;
+ mr->destructor = memory_region_destructor_ram;
+ mr->ram_addr = qemu_ram_alloc(size, mr, errp);
+}
+
void memory_region_init_ram_nofail(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size)
{
+ Error *local_err = NULL;
+
memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
- mr->ram_addr = qemu_ram_alloc(size, mr);
+ mr->ram_addr = qemu_ram_alloc(size, mr, &local_err);
+
+ if (local_err) {
+ error_report("%s", error_get_pretty(local_err));
+ error_free(local_err);
+ exit(EXIT_FAILURE);
+ }
}
#ifdef __linux__
@@ -1189,17 +1211,39 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
}
#endif
+void memory_region_init_ram_ptr(MemoryRegion *mr,
+ Object *owner,
+ const char *name,
+ uint64_t size,
+ void *ptr,
+ Error **errp)
+{
+ memory_region_init(mr, owner, name, size);
+ mr->ram = true;
+ mr->terminates = true;
+ mr->destructor = memory_region_destructor_ram_from_ptr;
+ mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, errp);
+}
+
void memory_region_init_ram_ptr_nofail(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size,
void *ptr)
{
+ Error *local_err = NULL;
+
memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram_from_ptr;
- mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr);
+ mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, &local_err);
+
+ if (local_err) {
+ error_report("%s", error_get_pretty(local_err));
+ error_free(local_err);
+ exit(EXIT_FAILURE);
+ }
}
void memory_region_init_alias(MemoryRegion *mr,
@@ -1221,7 +1265,8 @@ void memory_region_init_rom_device(MemoryRegion *mr,
const MemoryRegionOps *ops,
void *opaque,
const char *name,
- uint64_t size)
+ uint64_t size,
+ Error **errp)
{
memory_region_init(mr, owner, name, size);
mr->ops = ops;
@@ -1229,7 +1274,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
mr->terminates = true;
mr->rom_device = true;
mr->destructor = memory_region_destructor_rom_device;
- mr->ram_addr = qemu_ram_alloc(size, mr);
+ mr->ram_addr = qemu_ram_alloc(size, mr, errp);
}
void memory_region_init_iommu(MemoryRegion *mr,
--
1.9.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH V2 for 2.1 2/3] memory: add errp parameter to memory_region_init_ram() and memory_region_init_ram_ptr()
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 2/3] memory: add errp parameter to " Hu Tao
@ 2014-07-07 3:20 ` Michael S. Tsirkin
2014-07-07 5:39 ` Michael S. Tsirkin
1 sibling, 0 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 3:20 UTC (permalink / raw)
To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov
On Mon, Jul 07, 2014 at 10:58:07AM +0800, Hu Tao wrote:
> This patch reintroduces memory_region_init_ram() and
> memory_region_init_ram_ptr() which are almost the same as the renamed
> ones in the previous patch, except that an errp parameter is introduced
> to let callers handle error.
>
> In hostmem-ram.c we call memory_region_init_ram() now rather than
> memory_region_init_ram_nofail() so that error can be handled.
s/error/errors/ above.
I fixed it up.
> 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 | 39 +++++++++++++++++++++++++++++++++++-
> include/exec/ram_addr.h | 4 ++--
> memory.c | 53 +++++++++++++++++++++++++++++++++++++++++++++----
> 7 files changed, 122 insertions(+), 20 deletions(-)
>
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index 9d4960b..a67a134 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_nofail(&backend->mr, OBJECT(backend), path,
> - backend->size);
> + memory_region_init_ram(&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 28c86ee..279239d 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -304,6 +304,22 @@ void memory_region_init_io(MemoryRegion *mr,
> uint64_t size);
>
> /**
> + * memory_region_init_ram: Initialize RAM memory region. Accesses into the
> + * region will modify memory directly.
> + *
> + * @mr: the #MemoryRegion to be initialized.
> + * @owner: the object that tracks the region's reference count
> + * @name: the name of the region.
> + * @size: size of the region.
> + * @errp: pointer to Error*, to store an error if it happens.
> + */
> +void memory_region_init_ram(MemoryRegion *mr,
> + struct Object *owner,
> + const char *name,
> + uint64_t size,
> + Error **errp);
> +
> +/**
> * memory_region_init_ram_nofail: Initialize RAM memory region. Accesses into
> * the region will modify memory directly.
> *
> @@ -340,6 +356,25 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
> #endif
>
> /**
> + * memory_region_init_ram_ptr: Initialize RAM memory region from a
> + * user-provided pointer. Accesses into the
> + * region will modify memory directly.
> + *
> + * @mr: the #MemoryRegion to be initialized.
> + * @owner: the object that tracks the region's reference count
> + * @name: the name of the region.
> + * @size: size of the region.
> + * @ptr: memory to be mapped; must contain at least @size bytes.
> + * @errp: pointer to Error*, to store an error if it happens.
> + */
> +void memory_region_init_ram_ptr(MemoryRegion *mr,
> + struct Object *owner,
> + const char *name,
> + uint64_t size,
> + void *ptr,
> + Error **errp);
> +
> +/**
> * memory_region_init_ram_ptr_nofail: Initialize RAM memory region from a
> * user-provided pointer. Accesses into the
> * region will modify memory directly.
> @@ -384,13 +419,15 @@ void memory_region_init_alias(MemoryRegion *mr,
> * @ops: callbacks for write access handling.
> * @name: the name of the region.
> * @size: size of the region.
> + * @errp: pointer to Error*, to store an error if it happens.
> */
> void memory_region_init_rom_device(MemoryRegion *mr,
> struct Object *owner,
> const MemoryRegionOps *ops,
> void *opaque,
> const char *name,
> - uint64_t size);
> + uint64_t size,
> + Error **errp);
>
> /**
> * memory_region_init_reservation: Initialize a memory region that reserves
> 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 5f8a9f5..dc24c53 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,37 @@ void memory_region_init_io(MemoryRegion *mr,
> mr->ram_addr = ~(ram_addr_t)0;
> }
>
> +void memory_region_init_ram(MemoryRegion *mr,
> + Object *owner,
> + const char *name,
> + uint64_t size,
> + Error **errp)
> +{
> + memory_region_init(mr, owner, name, size);
> + mr->ram = true;
> + mr->terminates = true;
> + mr->destructor = memory_region_destructor_ram;
> + mr->ram_addr = qemu_ram_alloc(size, mr, errp);
> +}
> +
> void memory_region_init_ram_nofail(MemoryRegion *mr,
> Object *owner,
> const char *name,
> uint64_t size)
> {
> + Error *local_err = NULL;
> +
> memory_region_init(mr, owner, name, size);
> mr->ram = true;
> mr->terminates = true;
> mr->destructor = memory_region_destructor_ram;
> - mr->ram_addr = qemu_ram_alloc(size, mr);
> + mr->ram_addr = qemu_ram_alloc(size, mr, &local_err);
> +
> + if (local_err) {
> + error_report("%s", error_get_pretty(local_err));
> + error_free(local_err);
> + exit(EXIT_FAILURE);
> + }
> }
>
> #ifdef __linux__
> @@ -1189,17 +1211,39 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
> }
> #endif
>
> +void memory_region_init_ram_ptr(MemoryRegion *mr,
> + Object *owner,
> + const char *name,
> + uint64_t size,
> + void *ptr,
> + Error **errp)
> +{
> + memory_region_init(mr, owner, name, size);
> + mr->ram = true;
> + mr->terminates = true;
> + mr->destructor = memory_region_destructor_ram_from_ptr;
> + mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, errp);
> +}
> +
> void memory_region_init_ram_ptr_nofail(MemoryRegion *mr,
> Object *owner,
> const char *name,
> uint64_t size,
> void *ptr)
> {
> + Error *local_err = NULL;
> +
> memory_region_init(mr, owner, name, size);
> mr->ram = true;
> mr->terminates = true;
> mr->destructor = memory_region_destructor_ram_from_ptr;
> - mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr);
> + mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, &local_err);
> +
> + if (local_err) {
> + error_report("%s", error_get_pretty(local_err));
> + error_free(local_err);
> + exit(EXIT_FAILURE);
> + }
> }
>
> void memory_region_init_alias(MemoryRegion *mr,
> @@ -1221,7 +1265,8 @@ void memory_region_init_rom_device(MemoryRegion *mr,
> const MemoryRegionOps *ops,
> void *opaque,
> const char *name,
> - uint64_t size)
> + uint64_t size,
> + Error **errp)
> {
> memory_region_init(mr, owner, name, size);
> mr->ops = ops;
> @@ -1229,7 +1274,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
> mr->terminates = true;
> mr->rom_device = true;
> mr->destructor = memory_region_destructor_rom_device;
> - mr->ram_addr = qemu_ram_alloc(size, mr);
> + mr->ram_addr = qemu_ram_alloc(size, mr, errp);
> }
>
> void memory_region_init_iommu(MemoryRegion *mr,
> --
> 1.9.3
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH V2 for 2.1 2/3] memory: add errp parameter to memory_region_init_ram() and memory_region_init_ram_ptr()
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 2/3] memory: add errp parameter to " Hu Tao
2014-07-07 3:20 ` Michael S. Tsirkin
@ 2014-07-07 5:39 ` Michael S. Tsirkin
1 sibling, 0 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 5:39 UTC (permalink / raw)
To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov
On Mon, Jul 07, 2014 at 10:58:07AM +0800, Hu Tao wrote:
> This patch reintroduces memory_region_init_ram() and
> memory_region_init_ram_ptr() which are almost the same as the renamed
> ones in the previous patch, except that an errp parameter is introduced
> to let callers handle error.
>
> In hostmem-ram.c we call memory_region_init_ram() now rather than
> memory_region_init_ram_nofail() so that error can be handled.
>
> This patch solves a problem that qemu just exits when using monitor
> command object_add to add a memory backend whose size is way too large.
> In the case we'd better give an error message and keep guest running.
>
> The problem can be reproduced as follows:
>
> 1. run qemu
> 2. (monitor)object_add memory-backend-ram,size=100000G,id=ram0
>
> Signed-off-by: Hu Tao <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 | 39 +++++++++++++++++++++++++++++++++++-
> include/exec/ram_addr.h | 4 ++--
> memory.c | 53 +++++++++++++++++++++++++++++++++++++++++++++----
> 7 files changed, 122 insertions(+), 20 deletions(-)
>
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index 9d4960b..a67a134 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_nofail(&backend->mr, OBJECT(backend), path,
> - backend->size);
> + memory_region_init_ram(&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 28c86ee..279239d 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -304,6 +304,22 @@ void memory_region_init_io(MemoryRegion *mr,
> uint64_t size);
>
> /**
> + * memory_region_init_ram: Initialize RAM memory region. Accesses into the
> + * region will modify memory directly.
> + *
> + * @mr: the #MemoryRegion to be initialized.
> + * @owner: the object that tracks the region's reference count
> + * @name: the name of the region.
> + * @size: size of the region.
> + * @errp: pointer to Error*, to store an error if it happens.
> + */
> +void memory_region_init_ram(MemoryRegion *mr,
> + struct Object *owner,
> + const char *name,
> + uint64_t size,
> + Error **errp);
> +
> +/**
> * memory_region_init_ram_nofail: Initialize RAM memory region. Accesses into
> * the region will modify memory directly.
> *
> @@ -340,6 +356,25 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
> #endif
>
> /**
> + * memory_region_init_ram_ptr: Initialize RAM memory region from a
> + * user-provided pointer. Accesses into the
> + * region will modify memory directly.
> + *
> + * @mr: the #MemoryRegion to be initialized.
> + * @owner: the object that tracks the region's reference count
> + * @name: the name of the region.
> + * @size: size of the region.
> + * @ptr: memory to be mapped; must contain at least @size bytes.
> + * @errp: pointer to Error*, to store an error if it happens.
> + */
> +void memory_region_init_ram_ptr(MemoryRegion *mr,
> + struct Object *owner,
> + const char *name,
> + uint64_t size,
> + void *ptr,
> + Error **errp);
> +
> +/**
> * memory_region_init_ram_ptr_nofail: Initialize RAM memory region from a
> * user-provided pointer. Accesses into the
> * region will modify memory directly.
> @@ -384,13 +419,15 @@ void memory_region_init_alias(MemoryRegion *mr,
> * @ops: callbacks for write access handling.
> * @name: the name of the region.
> * @size: size of the region.
> + * @errp: pointer to Error*, to store an error if it happens.
> */
> void memory_region_init_rom_device(MemoryRegion *mr,
> struct Object *owner,
> const MemoryRegionOps *ops,
> void *opaque,
> const char *name,
> - uint64_t size);
> + uint64_t size,
> + Error **errp);
>
> /**
> * memory_region_init_reservation: Initialize a memory region that reserves
> 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 5f8a9f5..dc24c53 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,37 @@ void memory_region_init_io(MemoryRegion *mr,
> mr->ram_addr = ~(ram_addr_t)0;
> }
>
> +void memory_region_init_ram(MemoryRegion *mr,
> + Object *owner,
> + const char *name,
> + uint64_t size,
> + Error **errp)
> +{
> + memory_region_init(mr, owner, name, size);
> + mr->ram = true;
> + mr->terminates = true;
> + mr->destructor = memory_region_destructor_ram;
> + mr->ram_addr = qemu_ram_alloc(size, mr, errp);
> +}
> +
> void memory_region_init_ram_nofail(MemoryRegion *mr,
> Object *owner,
> const char *name,
> uint64_t size)
> {
> + Error *local_err = NULL;
> +
> memory_region_init(mr, owner, name, size);
> mr->ram = true;
> mr->terminates = true;
> mr->destructor = memory_region_destructor_ram;
> - mr->ram_addr = qemu_ram_alloc(size, mr);
> + mr->ram_addr = qemu_ram_alloc(size, mr, &local_err);
> +
> + if (local_err) {
> + error_report("%s", error_get_pretty(local_err));
> + error_free(local_err);
> + exit(EXIT_FAILURE);
> + }
This is not the best way to do it.
It's better to call memory_region_init_ram from
memory_region_init_ram_nofail.
> }
>
> #ifdef __linux__
> @@ -1189,17 +1211,39 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
> }
> #endif
>
> +void memory_region_init_ram_ptr(MemoryRegion *mr,
> + Object *owner,
> + const char *name,
> + uint64_t size,
> + void *ptr,
> + Error **errp)
> +{
> + memory_region_init(mr, owner, name, size);
> + mr->ram = true;
> + mr->terminates = true;
> + mr->destructor = memory_region_destructor_ram_from_ptr;
> + mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, errp);
> +}
> +
> void memory_region_init_ram_ptr_nofail(MemoryRegion *mr,
> Object *owner,
> const char *name,
> uint64_t size,
> void *ptr)
> {
> + Error *local_err = NULL;
> +
> memory_region_init(mr, owner, name, size);
> mr->ram = true;
> mr->terminates = true;
> mr->destructor = memory_region_destructor_ram_from_ptr;
> - mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr);
> + mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr, &local_err);
> +
> + if (local_err) {
> + error_report("%s", error_get_pretty(local_err));
> + error_free(local_err);
> + exit(EXIT_FAILURE);
> + }
> }
>
> void memory_region_init_alias(MemoryRegion *mr,
> @@ -1221,7 +1265,8 @@ void memory_region_init_rom_device(MemoryRegion *mr,
> const MemoryRegionOps *ops,
> void *opaque,
> const char *name,
> - uint64_t size)
> + uint64_t size,
> + Error **errp)
> {
> memory_region_init(mr, owner, name, size);
> mr->ops = ops;
> @@ -1229,7 +1274,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
> mr->terminates = true;
> mr->rom_device = true;
> mr->destructor = memory_region_destructor_rom_device;
> - mr->ram_addr = qemu_ram_alloc(size, mr);
> + mr->ram_addr = qemu_ram_alloc(size, mr, errp);
> }
>
> void memory_region_init_iommu(MemoryRegion *mr,
> --
> 1.9.3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH V2 for 2.1 3/3] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize()
2014-07-07 2:58 [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend Hu Tao
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 1/3] memory: rename memory_region_init_ram() and memory_region_init_ram_ptr() Hu Tao
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 2/3] memory: add errp parameter to " Hu Tao
@ 2014-07-07 2:58 ` Hu Tao
2014-07-07 3:24 ` Michael S. Tsirkin
2014-07-07 3:24 ` [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend Michael S. Tsirkin
3 siblings, 1 reply; 11+ messages in thread
From: Hu Tao @ 2014-07-07 2:58 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..9ecd9cf 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 stat 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 " should be euqal "
+ "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] 11+ messages in thread* Re: [Qemu-devel] [PATCH V2 for 2.1 3/3] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize()
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 3/3] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize() Hu Tao
@ 2014-07-07 3:24 ` Michael S. Tsirkin
0 siblings, 0 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 3:24 UTC (permalink / raw)
To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov
On Mon, Jul 07, 2014 at 10:58:08AM +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>
Fixed up some messages below.
> ---
> exec.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index ca7741b..9ecd9cf 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 stat file %s", path);
> return 0;
> }
>
s/to stat/to get size/
stat is an implementation detail.
> @@ -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 " should be euqal "
s/should be euqal/must be equal 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] 11+ messages in thread
* Re: [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend
2014-07-07 2:58 [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend Hu Tao
` (2 preceding siblings ...)
2014-07-07 2:58 ` [Qemu-devel] [PATCH V2 for 2.1 3/3] exec: improve error handling and reporting in file_ram_alloc() and gethugepagesize() Hu Tao
@ 2014-07-07 3:24 ` Michael S. Tsirkin
2014-07-07 3:35 ` Michael S. Tsirkin
3 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 3:24 UTC (permalink / raw)
To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov
On Mon, Jul 07, 2014 at 10:58:05AM +0800, Hu Tao wrote:
> This series includes three patches to fix bugs of memory backend. Patch
> 1 prepares for next patches, patch 2 and patch 3 fix two bugs
> respectively, see each patch for the bugs and how to reproduce them.
>
> changes to v1:
>
> - split patch 1 in v1 into 2 patches
> - don't rely on ram_block_add to return -1
> - error message tweak in file_ram_alloc
> - add error messages reported by qemu to commit message of patch 3
>
> Hu Tao (3):
> memory: rename memory_region_init_ram() and
> memory_region_init_ram_ptr()
> memory: add errp parameter to memory_region_init_ram() and
> memory_region_init_ram_ptr()
> exec: improve error handling and reporting in file_ram_alloc() and
> gethugepagesize()
I fixed up some minor issues and applied this, thanks.
> backends/hostmem-ram.c | 2 +-
> exec.c | 51 ++++++++++++++++++++++++++++---------------
> hw/block/pflash_cfi01.c | 5 ++++-
> hw/block/pflash_cfi02.c | 5 ++++-
> hw/core/loader.c | 2 +-
> hw/display/vga.c | 2 +-
> hw/display/vmware_vga.c | 3 ++-
> hw/i386/kvm/pci-assign.c | 9 ++++----
> hw/i386/pc.c | 2 +-
> hw/i386/pc_sysfw.c | 4 ++--
> hw/misc/ivshmem.c | 9 ++++----
> hw/misc/vfio.c | 3 ++-
> hw/pci/pci.c | 2 +-
> include/exec/memory.h | 43 +++++++++++++++++++++++++++++++++---
> include/exec/ram_addr.h | 4 ++--
> memory.c | 57 +++++++++++++++++++++++++++++++++++++++++++-----
> numa.c | 4 ++--
> 17 files changed, 158 insertions(+), 49 deletions(-)
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend
2014-07-07 3:24 ` [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend Michael S. Tsirkin
@ 2014-07-07 3:35 ` Michael S. Tsirkin
2014-07-07 8:17 ` Hu Tao
0 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 3:35 UTC (permalink / raw)
To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov
On Mon, Jul 07, 2014 at 06:24:59AM +0300, Michael S. Tsirkin wrote:
> On Mon, Jul 07, 2014 at 10:58:05AM +0800, Hu Tao wrote:
> > This series includes three patches to fix bugs of memory backend. Patch
> > 1 prepares for next patches, patch 2 and patch 3 fix two bugs
> > respectively, see each patch for the bugs and how to reproduce them.
> >
> > changes to v1:
> >
> > - split patch 1 in v1 into 2 patches
> > - don't rely on ram_block_add to return -1
> > - error message tweak in file_ram_alloc
> > - add error messages reported by qemu to commit message of patch 3
> >
> > Hu Tao (3):
> > memory: rename memory_region_init_ram() and
> > memory_region_init_ram_ptr()
> > memory: add errp parameter to memory_region_init_ram() and
> > memory_region_init_ram_ptr()
> > exec: improve error handling and reporting in file_ram_alloc() and
> > gethugepagesize()
>
> I fixed up some minor issues and applied this, thanks.
And reverted.
Build fails, and a simple check after applying patch 1 gives me:
git grep memory_region_init_ram |grep -v nofail|wc -l
132
Apparently you fixed up about 10% of the files using this function.
So forget about me merging patch 1.
Add a new
memory_region_init_ram_may_fail
and
memory_region_init_ram_ptr_may_fail
and use it specifically for the new stuff.
Do the rename on top in two steps:
memory_region_init_ram -> memory_region_init_ram_nofail
memory_region_init_ram_may_fail -> memory_region_init_ram
Paolo can then merge it when he prefers, though I'd say 2.2
is more reasonable.
> > backends/hostmem-ram.c | 2 +-
> > exec.c | 51 ++++++++++++++++++++++++++++---------------
> > hw/block/pflash_cfi01.c | 5 ++++-
> > hw/block/pflash_cfi02.c | 5 ++++-
> > hw/core/loader.c | 2 +-
> > hw/display/vga.c | 2 +-
> > hw/display/vmware_vga.c | 3 ++-
> > hw/i386/kvm/pci-assign.c | 9 ++++----
> > hw/i386/pc.c | 2 +-
> > hw/i386/pc_sysfw.c | 4 ++--
> > hw/misc/ivshmem.c | 9 ++++----
> > hw/misc/vfio.c | 3 ++-
> > hw/pci/pci.c | 2 +-
> > include/exec/memory.h | 43 +++++++++++++++++++++++++++++++++---
> > include/exec/ram_addr.h | 4 ++--
> > memory.c | 57 +++++++++++++++++++++++++++++++++++++++++++-----
> > numa.c | 4 ++--
> > 17 files changed, 158 insertions(+), 49 deletions(-)
> >
> > --
> > 1.9.3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend
2014-07-07 3:35 ` Michael S. Tsirkin
@ 2014-07-07 8:17 ` Hu Tao
2014-07-07 9:08 ` Michael S. Tsirkin
0 siblings, 1 reply; 11+ messages in thread
From: Hu Tao @ 2014-07-07 8:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov
On Mon, Jul 07, 2014 at 06:35:42AM +0300, Michael S. Tsirkin wrote:
> On Mon, Jul 07, 2014 at 06:24:59AM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jul 07, 2014 at 10:58:05AM +0800, Hu Tao wrote:
> > > This series includes three patches to fix bugs of memory backend. Patch
> > > 1 prepares for next patches, patch 2 and patch 3 fix two bugs
> > > respectively, see each patch for the bugs and how to reproduce them.
> > >
> > > changes to v1:
> > >
> > > - split patch 1 in v1 into 2 patches
> > > - don't rely on ram_block_add to return -1
> > > - error message tweak in file_ram_alloc
> > > - add error messages reported by qemu to commit message of patch 3
> > >
> > > Hu Tao (3):
> > > memory: rename memory_region_init_ram() and
> > > memory_region_init_ram_ptr()
> > > memory: add errp parameter to memory_region_init_ram() and
> > > memory_region_init_ram_ptr()
> > > exec: improve error handling and reporting in file_ram_alloc() and
> > > gethugepagesize()
> >
> > I fixed up some minor issues and applied this, thanks.
>
> And reverted.
>
> Build fails, and a simple check after applying patch 1 gives me:
> git grep memory_region_init_ram |grep -v nofail|wc -l
> 132
Thanks for catching this! I should have built all targets.
>
> Apparently you fixed up about 10% of the files using this function.
> So forget about me merging patch 1.
>
> Add a new
> memory_region_init_ram_may_fail
> and
> memory_region_init_ram_ptr_may_fail
>
> and use it specifically for the new stuff.
Thanks for the change!
>
> Do the rename on top in two steps:
> memory_region_init_ram -> memory_region_init_ram_nofail
> memory_region_init_ram_may_fail -> memory_region_init_ram
>
> Paolo can then merge it when he prefers, though I'd say 2.2
> is more reasonable.
>
>
> > > backends/hostmem-ram.c | 2 +-
> > > exec.c | 51 ++++++++++++++++++++++++++++---------------
> > > hw/block/pflash_cfi01.c | 5 ++++-
> > > hw/block/pflash_cfi02.c | 5 ++++-
> > > hw/core/loader.c | 2 +-
> > > hw/display/vga.c | 2 +-
> > > hw/display/vmware_vga.c | 3 ++-
> > > hw/i386/kvm/pci-assign.c | 9 ++++----
> > > hw/i386/pc.c | 2 +-
> > > hw/i386/pc_sysfw.c | 4 ++--
> > > hw/misc/ivshmem.c | 9 ++++----
> > > hw/misc/vfio.c | 3 ++-
> > > hw/pci/pci.c | 2 +-
> > > include/exec/memory.h | 43 +++++++++++++++++++++++++++++++++---
> > > include/exec/ram_addr.h | 4 ++--
> > > memory.c | 57 +++++++++++++++++++++++++++++++++++++++++++-----
> > > numa.c | 4 ++--
> > > 17 files changed, 158 insertions(+), 49 deletions(-)
> > >
> > > --
> > > 1.9.3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH V2 for 2.1 0/3] bug fixs for memory backend
2014-07-07 8:17 ` Hu Tao
@ 2014-07-07 9:08 ` Michael S. Tsirkin
0 siblings, 0 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2014-07-07 9:08 UTC (permalink / raw)
To: Hu Tao; +Cc: Yasunori Goto, Paolo Bonzini, qemu-devel, Igor Mammedov
On Mon, Jul 07, 2014 at 04:17:30PM +0800, Hu Tao wrote:
> On Mon, Jul 07, 2014 at 06:35:42AM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jul 07, 2014 at 06:24:59AM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Jul 07, 2014 at 10:58:05AM +0800, Hu Tao wrote:
> > > > This series includes three patches to fix bugs of memory backend. Patch
> > > > 1 prepares for next patches, patch 2 and patch 3 fix two bugs
> > > > respectively, see each patch for the bugs and how to reproduce them.
> > > >
> > > > changes to v1:
> > > >
> > > > - split patch 1 in v1 into 2 patches
> > > > - don't rely on ram_block_add to return -1
> > > > - error message tweak in file_ram_alloc
> > > > - add error messages reported by qemu to commit message of patch 3
> > > >
> > > > Hu Tao (3):
> > > > memory: rename memory_region_init_ram() and
> > > > memory_region_init_ram_ptr()
> > > > memory: add errp parameter to memory_region_init_ram() and
> > > > memory_region_init_ram_ptr()
> > > > exec: improve error handling and reporting in file_ram_alloc() and
> > > > gethugepagesize()
> > >
> > > I fixed up some minor issues and applied this, thanks.
> >
> > And reverted.
> >
> > Build fails, and a simple check after applying patch 1 gives me:
> > git grep memory_region_init_ram |grep -v nofail|wc -l
> > 132
>
> Thanks for catching this! I should have built all targets.
>
> >
> > Apparently you fixed up about 10% of the files using this function.
> > So forget about me merging patch 1.
> >
> > Add a new
> > memory_region_init_ram_may_fail
> > and
> > memory_region_init_ram_ptr_may_fail
> >
> > and use it specifically for the new stuff.
>
> Thanks for the change!
To clarify: I didn't apply this change. I simply reverted the whole
patchset. If you want this patchset applied please do it, at this point
I'm making no promises on whether it will get into 2.1.
> >
> > Do the rename on top in two steps:
> > memory_region_init_ram -> memory_region_init_ram_nofail
> > memory_region_init_ram_may_fail -> memory_region_init_ram
> >
> > Paolo can then merge it when he prefers, though I'd say 2.2
> > is more reasonable.
> >
> >
> > > > backends/hostmem-ram.c | 2 +-
> > > > exec.c | 51 ++++++++++++++++++++++++++++---------------
> > > > hw/block/pflash_cfi01.c | 5 ++++-
> > > > hw/block/pflash_cfi02.c | 5 ++++-
> > > > hw/core/loader.c | 2 +-
> > > > hw/display/vga.c | 2 +-
> > > > hw/display/vmware_vga.c | 3 ++-
> > > > hw/i386/kvm/pci-assign.c | 9 ++++----
> > > > hw/i386/pc.c | 2 +-
> > > > hw/i386/pc_sysfw.c | 4 ++--
> > > > hw/misc/ivshmem.c | 9 ++++----
> > > > hw/misc/vfio.c | 3 ++-
> > > > hw/pci/pci.c | 2 +-
> > > > include/exec/memory.h | 43 +++++++++++++++++++++++++++++++++---
> > > > include/exec/ram_addr.h | 4 ++--
> > > > memory.c | 57 +++++++++++++++++++++++++++++++++++++++++++-----
> > > > numa.c | 4 ++--
> > > > 17 files changed, 158 insertions(+), 49 deletions(-)
> > > >
> > > > --
> > > > 1.9.3
^ permalink raw reply [flat|nested] 11+ messages in thread