* [Qemu-devel] [PATCH v5 01/16] fixup! NUMA: check if the total numa memory size is equal to ram_size
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 02/16] vl: redo -object parsing Hu Tao
` (15 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto, Wanlong Gao
From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
If the total number of the assigned numa nodes memory is not
equal to the assigned ram size, it will write the wrong data
to ACPI table, then the guest will ignore the wrong ACPI table
and recognize all memory to one node. It's buggy, we should
check it to ensure that we write the right data to ACPI table.
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Conflicts:
numa.c
---
numa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/numa.c b/numa.c
index 2c349ae..6ea267e 100644
--- a/numa.c
+++ b/numa.c
@@ -187,8 +187,8 @@ void set_numa_nodes(void)
numa_total += numa_info[i].node_mem;
}
if (numa_total != ram_size) {
- error_report("qemu: total memory size for NUMA nodes (%" PRIu64 ")"
- " should equal RAM size (" RAM_ADDR_FMT ")\n",
+ error_report("total memory for NUMA nodes (%" PRIu64 ")"
+ " should equal RAM size (" RAM_ADDR_FMT ")",
numa_total, ram_size);
exit(1);
}
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 02/16] vl: redo -object parsing
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 01/16] fixup! NUMA: check if the total numa memory size is equal to ram_size Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 03/16] fixup! qmp: improve error reporting for -object and object-add Hu Tao
` (14 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
From: Paolo Bonzini <pbonzini@redhat.com>
Follow the lines of the HMP implementation, using OptsVisitor
to parse the options. This gives access to OptsVisitor's
rich parsing of integer lists.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
vl.c | 65 ++++++++++++++++++++++++++++++++++++-----------------------------
1 file changed, 36 insertions(+), 29 deletions(-)
diff --git a/vl.c b/vl.c
index 730bd1e..1617013 100644
--- a/vl.c
+++ b/vl.c
@@ -116,7 +116,7 @@ int main(int argc, char **argv)
#include "ui/qemu-spice.h"
#include "qapi/string-input-visitor.h"
-#include "qom/object_interfaces.h"
+#include "qapi/opts-visitor.h"
#define DEFAULT_RAM_SIZE 128
@@ -2822,44 +2822,51 @@ static int object_set_property(const char *name, const char *value, void *opaque
static int object_create(QemuOpts *opts, void *opaque)
{
- const char *type = qemu_opt_get(opts, "qom-type");
- const char *id = qemu_opts_id(opts);
- Error *local_err = NULL;
- Object *obj;
-
- g_assert(type != NULL);
-
- if (id == NULL) {
- qerror_report(QERR_MISSING_PARAMETER, "id");
- return -1;
+ Error *err = NULL;
+ char *type = NULL;
+ char *id = NULL;
+ void *dummy = NULL;
+ OptsVisitor *ov;
+ QDict *pdict;
+
+ ov = opts_visitor_new(opts);
+ pdict = qemu_opts_to_qdict(opts, NULL);
+
+ visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
+ if (err) {
+ goto out;
}
- obj = object_new(type);
- if (qemu_opt_foreach(opts, object_set_property, obj, 1) < 0) {
- object_unref(obj);
- return -1;
+ qdict_del(pdict, "qom-type");
+ visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
+ if (err) {
+ goto out;
}
- if (!object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
- error_setg(&local_err, "object '%s' isn't supported by -object",
- id);
+ qdict_del(pdict, "id");
+ visit_type_str(opts_get_visitor(ov), &id, "id", &err);
+ if (err) {
goto out;
}
- object_property_add_child(container_get(object_get_root(), "/objects"),
- id, obj, &local_err);
-
- user_creatable_complete(obj, &local_err);
- if (local_err) {
- object_property_del(container_get(object_get_root(), "/objects"),
- id, &error_abort);
+ object_add(type, id, pdict, opts_get_visitor(ov), &err);
+ if (err) {
goto out;
}
+ visit_end_struct(opts_get_visitor(ov), &err);
+ if (err) {
+ qmp_object_del(id, NULL);
+ }
+
out:
- object_unref(obj);
- if (local_err) {
- qerror_report_err(local_err);
- error_free(local_err);
+ opts_visitor_cleanup(ov);
+
+ QDECREF(pdict);
+ g_free(id);
+ g_free(type);
+ g_free(dummy);
+ if (err) {
+ qerror_report_err(err);
return -1;
}
return 0;
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 03/16] fixup! qmp: improve error reporting for -object and object-add
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 01/16] fixup! NUMA: check if the total numa memory size is equal to ram_size Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 02/16] vl: redo -object parsing Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 04/16] pc: pass MachineState to pc_memory_init Hu Tao
` (13 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
From: Paolo Bonzini <pbonzini@redhat.com>
reword error message to make it more understandable.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Conflicts:
qmp.c
---
qmp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/qmp.c b/qmp.c
index cef60fb..c3c0229 100644
--- a/qmp.c
+++ b/qmp.c
@@ -540,8 +540,7 @@ void object_add(const char *type, const char *id, const QDict *qdict,
klass = object_class_by_name(type);
if (!klass) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE,
- "qom-type", "a valid class name");
+ error_setg(errp, "invalid object type: %s", type);
return;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 04/16] pc: pass MachineState to pc_memory_init
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (2 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 03/16] fixup! qmp: improve error reporting for -object and object-add Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 05/16] backend:hostmem: replace hostmemory with host_memory Hu Tao
` (12 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
From: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-By: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc.c | 24 ++++++++++++------------
hw/i386/pc_piix.c | 8 +++-----
hw/i386/pc_q35.c | 4 +---
include/hw/i386/pc.h | 7 +++----
4 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a035da6..37344ce 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1194,10 +1194,8 @@ void pc_acpi_init(const char *default_dsdt)
}
}
-FWCfgState *pc_memory_init(MemoryRegion *system_memory,
- const char *kernel_filename,
- const char *kernel_cmdline,
- const char *initrd_filename,
+FWCfgState *pc_memory_init(MachineState *machine,
+ MemoryRegion *system_memory,
ram_addr_t below_4g_mem_size,
ram_addr_t above_4g_mem_size,
MemoryRegion *rom_memory,
@@ -1208,18 +1206,19 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
MemoryRegion *ram, *option_rom_mr;
MemoryRegion *ram_below_4g, *ram_above_4g;
FWCfgState *fw_cfg;
- ram_addr_t ram_size = below_4g_mem_size + above_4g_mem_size;
- MachineState *machine = MACHINE(qdev_get_machine());
PCMachineState *pcms = PC_MACHINE(machine);
- linux_boot = (kernel_filename != NULL);
+ assert(machine->ram_size == below_4g_mem_size + above_4g_mem_size);
+
+ linux_boot = (machine->kernel_filename != NULL);
/* Allocate RAM. We allocate it as a single memory region and use
* aliases to address portions of it, mostly for backwards compatibility
* with older qemus that used qemu_ram_alloc().
*/
ram = g_malloc(sizeof(*ram));
- memory_region_allocate_system_memory(ram, NULL, "pc.ram", ram_size);
+ memory_region_allocate_system_memory(ram, NULL, "pc.ram",
+ machine->ram_size);
*ram_memory = ram;
ram_below_4g = g_malloc(sizeof(*ram_below_4g));
memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
@@ -1237,7 +1236,7 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
if (!guest_info->has_reserved_memory &&
(machine->ram_slots ||
- (machine->maxram_size > ram_size))) {
+ (machine->maxram_size > machine->ram_size))) {
MachineClass *mc = MACHINE_GET_CLASS(machine);
error_report("\"-memory 'slots|maxmem'\" is not supported by: %s",
@@ -1247,9 +1246,9 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
/* initialize hotplug memory address space */
if (guest_info->has_reserved_memory &&
- (ram_size < machine->maxram_size)) {
+ (machine->ram_size < machine->maxram_size)) {
ram_addr_t hotplug_mem_size =
- machine->maxram_size - ram_size;
+ machine->maxram_size - machine->ram_size;
if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
error_report("unsupported amount of memory slots: %"PRIu64,
@@ -1294,7 +1293,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
}
if (linux_boot) {
- load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
+ load_linux(fw_cfg, machine->kernel_filename, machine->initrd_filename,
+ machine->kernel_cmdline, below_4g_mem_size);
}
for (i = 0; i < nb_option_roms; i++) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a13e8d6..3e7524b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -156,11 +156,9 @@ static void pc_init1(MachineState *machine,
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
- fw_cfg = pc_memory_init(system_memory,
- machine->kernel_filename, machine->kernel_cmdline,
- machine->initrd_filename,
- below_4g_mem_size, above_4g_mem_size,
- rom_memory, &ram_memory, guest_info);
+ fw_cfg = pc_memory_init(machine, system_memory,
+ below_4g_mem_size, above_4g_mem_size,
+ rom_memory, &ram_memory, guest_info);
}
gsi_state = g_malloc0(sizeof(*gsi_state));
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 629eb2d..aa71332 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -143,9 +143,7 @@ static void pc_q35_init(MachineState *machine)
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
- pc_memory_init(get_system_memory(),
- machine->kernel_filename, machine->kernel_cmdline,
- machine->initrd_filename,
+ pc_memory_init(machine, get_system_memory(),
below_4g_mem_size, above_4g_mem_size,
rom_memory, &ram_memory, guest_info);
}
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index a2bf22c..2c3d7c4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -3,6 +3,7 @@
#include "qemu-common.h"
#include "exec/memory.h"
+#include "hw/boards.h"
#include "hw/isa/isa.h"
#include "hw/block/fdc.h"
#include "net/net.h"
@@ -183,10 +184,8 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
MemoryRegion *pci_address_space);
-FWCfgState *pc_memory_init(MemoryRegion *system_memory,
- const char *kernel_filename,
- const char *kernel_cmdline,
- const char *initrd_filename,
+FWCfgState *pc_memory_init(MachineState *machine,
+ MemoryRegion *system_memory,
ram_addr_t below_4g_mem_size,
ram_addr_t above_4g_mem_size,
MemoryRegion *rom_memory,
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 05/16] backend:hostmem: replace hostmemory with host_memory
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (3 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 04/16] pc: pass MachineState to pc_memory_init Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 06/16] hostmem: separate allocation from UserCreatable complete method Hu Tao
` (11 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
..to keep names consistant.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 2f578ac..2d9fd00 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -17,8 +17,8 @@
#include "qom/object_interfaces.h"
static void
-hostmemory_backend_get_size(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+host_memory_backend_get_size(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
HostMemoryBackend *backend = MEMORY_BACKEND(obj);
uint64_t value = backend->size;
@@ -27,8 +27,8 @@ hostmemory_backend_get_size(Object *obj, Visitor *v, void *opaque,
}
static void
-hostmemory_backend_set_size(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+host_memory_backend_set_size(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
HostMemoryBackend *backend = MEMORY_BACKEND(obj);
Error *local_err = NULL;
@@ -53,14 +53,14 @@ out:
error_propagate(errp, local_err);
}
-static void hostmemory_backend_init(Object *obj)
+static void host_memory_backend_init(Object *obj)
{
object_property_add(obj, "size", "int",
- hostmemory_backend_get_size,
- hostmemory_backend_set_size, NULL, NULL, NULL);
+ host_memory_backend_get_size,
+ host_memory_backend_set_size, NULL, NULL, NULL);
}
-static void hostmemory_backend_finalize(Object *obj)
+static void host_memory_backend_finalize(Object *obj)
{
HostMemoryBackend *backend = MEMORY_BACKEND(obj);
@@ -75,14 +75,14 @@ host_memory_backend_get_memory(HostMemoryBackend *backend, Error **errp)
return memory_region_size(&backend->mr) ? &backend->mr : NULL;
}
-static const TypeInfo hostmemory_backend_info = {
+static const TypeInfo host_memory_backend_info = {
.name = TYPE_MEMORY_BACKEND,
.parent = TYPE_OBJECT,
.abstract = true,
.class_size = sizeof(HostMemoryBackendClass),
.instance_size = sizeof(HostMemoryBackend),
- .instance_init = hostmemory_backend_init,
- .instance_finalize = hostmemory_backend_finalize,
+ .instance_init = host_memory_backend_init,
+ .instance_finalize = host_memory_backend_finalize,
.interfaces = (InterfaceInfo[]) {
{ TYPE_USER_CREATABLE },
{ }
@@ -91,7 +91,7 @@ static const TypeInfo hostmemory_backend_info = {
static void register_types(void)
{
- type_register_static(&hostmemory_backend_info);
+ type_register_static(&host_memory_backend_info);
}
type_init(register_types);
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 06/16] hostmem: separate allocation from UserCreatable complete method
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (4 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 05/16] backend:hostmem: replace hostmemory with host_memory Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 07/16] fixup! numa: add -numa node, memdev= option Hu Tao
` (10 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
This allows the superclass to set various policies on the memory
region that the subclass creates. Drops hostmem-ram's complete method
accordingly.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem-ram.c | 7 +++----
backends/hostmem.c | 20 ++++++++++++++++++++
include/sysemu/hostmem.h | 2 ++
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index bba2ebc..d9a8290 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -16,9 +16,8 @@
static void
-ram_backend_memory_init(UserCreatable *uc, Error **errp)
+ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
{
- HostMemoryBackend *backend = MEMORY_BACKEND(uc);
char *path;
if (!backend->size) {
@@ -35,9 +34,9 @@ ram_backend_memory_init(UserCreatable *uc, Error **errp)
static void
ram_backend_class_init(ObjectClass *oc, void *data)
{
- UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
- ucc->complete = ram_backend_memory_init;
+ bc->alloc = ram_backend_memory_alloc;
}
static const TypeInfo ram_backend_info = {
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 2d9fd00..1738774 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -75,11 +75,31 @@ host_memory_backend_get_memory(HostMemoryBackend *backend, Error **errp)
return memory_region_size(&backend->mr) ? &backend->mr : NULL;
}
+static void
+host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(uc);
+ HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
+
+ if (bc->alloc) {
+ bc->alloc(backend, errp);
+ }
+}
+
+static void
+host_memory_backend_class_init(ObjectClass *oc, void *data)
+{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+
+ ucc->complete = host_memory_backend_memory_complete;
+}
+
static const TypeInfo host_memory_backend_info = {
.name = TYPE_MEMORY_BACKEND,
.parent = TYPE_OBJECT,
.abstract = true,
.class_size = sizeof(HostMemoryBackendClass),
+ .class_init = host_memory_backend_class_init,
.instance_size = sizeof(HostMemoryBackend),
.instance_init = host_memory_backend_init,
.instance_finalize = host_memory_backend_finalize,
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 4fc081e..923f672 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -34,6 +34,8 @@ typedef struct HostMemoryBackendClass HostMemoryBackendClass;
*/
struct HostMemoryBackendClass {
ObjectClass parent_class;
+
+ void (*alloc)(HostMemoryBackend *backend, Error **errp);
};
/**
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 07/16] fixup! numa: add -numa node, memdev= option
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (5 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 06/16] hostmem: separate allocation from UserCreatable complete method Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:27 ` Michael S. Tsirkin
2014-06-10 11:30 ` Michael S. Tsirkin
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 08/16] hostmem: add file-based HostMemoryBackend Hu Tao
` (9 subsequent siblings)
16 siblings, 2 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
From: Paolo Bonzini <pbonzini@redhat.com>
This option provides the infrastructure for binding guest NUMA nodes
to host NUMA nodes. For example:
-object memory-ram,size=1024M,policy=bind,host-nodes=0,id=ram-node0 \
-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
-object memory-ram,size=1024M,policy=interleave,host-nodes=1-3,id=ram-node1 \
-numa node,nodeid=1,cpus=1,memdev=ram-node1
The option replaces "-numa node,mem=".
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Conflicts:
numa.c
qemu-options.hx
---
qemu-options.hx | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 5375a93..73b6f1f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -95,9 +95,11 @@ specifies the maximum number of hotpluggable CPUs.
ETEXI
DEF("numa", HAS_ARG, QEMU_OPTION_numa,
- "-numa node[,mem=size][,memdev=id][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
+ "-numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node]\n"
+ "-numa node[,memdev=id][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
STEXI
-@item -numa node[,mem=@var{size}][,memdev=@var{id}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
+@item -numa node[,mem=@var{size}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
+@item -numa node[,memdev=@var{id}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
@findex -numa
Simulate a multi node NUMA system. If @samp{mem}, @samp{memdev}
and @samp{cpus} are omitted, resources are split equally. Also, note
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH v5 07/16] fixup! numa: add -numa node, memdev= option
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 07/16] fixup! numa: add -numa node, memdev= option Hu Tao
@ 2014-06-10 11:27 ` Michael S. Tsirkin
2014-06-10 11:30 ` Michael S. Tsirkin
1 sibling, 0 replies; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-10 11:27 UTC (permalink / raw)
To: Hu Tao
Cc: Eduardo Habkost, qemu-devel, Paolo Bonzini, Igor Mammedov, Yasunori Goto
On Tue, Jun 10, 2014 at 07:15:20PM +0800, Hu Tao wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> This option provides the infrastructure for binding guest NUMA nodes
> to host NUMA nodes. For example:
>
> -object memory-ram,size=1024M,policy=bind,host-nodes=0,id=ram-node0 \
> -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
> -object memory-ram,size=1024M,policy=interleave,host-nodes=1-3,id=ram-node1 \
> -numa node,nodeid=1,cpus=1,memdev=ram-node1
>
> The option replaces "-numa node,mem=".
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>
> Conflicts:
> numa.c
> qemu-options.hx
Don't keep the Conflicts option around next time pls.
I dropped this for now.
> ---
> qemu-options.hx | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5375a93..73b6f1f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -95,9 +95,11 @@ specifies the maximum number of hotpluggable CPUs.
> ETEXI
>
> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> - "-numa node[,mem=size][,memdev=id][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
> + "-numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node]\n"
> + "-numa node[,memdev=id][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
> STEXI
> -@item -numa node[,mem=@var{size}][,memdev=@var{id}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
> +@item -numa node[,mem=@var{size}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
> +@item -numa node[,memdev=@var{id}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
> @findex -numa
> Simulate a multi node NUMA system. If @samp{mem}, @samp{memdev}
> and @samp{cpus} are omitted, resources are split equally. Also, note
> --
> 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH v5 07/16] fixup! numa: add -numa node, memdev= option
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 07/16] fixup! numa: add -numa node, memdev= option Hu Tao
2014-06-10 11:27 ` Michael S. Tsirkin
@ 2014-06-10 11:30 ` Michael S. Tsirkin
1 sibling, 0 replies; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-10 11:30 UTC (permalink / raw)
To: Hu Tao
Cc: Eduardo Habkost, qemu-devel, Paolo Bonzini, Igor Mammedov, Yasunori Goto
Original subject was:
numa: add -numa node, memdev= option
with space after ,
if you are sending fixups, please make subject match
verbatim.
I fixed this up for now.
On Tue, Jun 10, 2014 at 07:15:20PM +0800, Hu Tao wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> This option provides the infrastructure for binding guest NUMA nodes
> to host NUMA nodes. For example:
>
> -object memory-ram,size=1024M,policy=bind,host-nodes=0,id=ram-node0 \
> -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
> -object memory-ram,size=1024M,policy=interleave,host-nodes=1-3,id=ram-node1 \
> -numa node,nodeid=1,cpus=1,memdev=ram-node1
>
> The option replaces "-numa node,mem=".
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>
> Conflicts:
> numa.c
> qemu-options.hx
would be better to mention what's being changed here instead.
> ---
> qemu-options.hx | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5375a93..73b6f1f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -95,9 +95,11 @@ specifies the maximum number of hotpluggable CPUs.
> ETEXI
>
> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> - "-numa node[,mem=size][,memdev=id][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
> + "-numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node]\n"
> + "-numa node[,memdev=id][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
> STEXI
> -@item -numa node[,mem=@var{size}][,memdev=@var{id}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
> +@item -numa node[,mem=@var{size}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
> +@item -numa node[,memdev=@var{id}][,cpus=@var{cpu[-cpu]}][,nodeid=@var{node}]
> @findex -numa
> Simulate a multi node NUMA system. If @samp{mem}, @samp{memdev}
> and @samp{cpus} are omitted, resources are split equally. Also, note
> --
> 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH v5 08/16] hostmem: add file-based HostMemoryBackend
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (6 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 07/16] fixup! numa: add -numa node, memdev= option Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-11 8:03 ` Michael S. Tsirkin
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 09/16] hostmem: add merge and dump properties Hu Tao
` (8 subsequent siblings)
16 siblings, 1 reply; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
From: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/Makefile.objs | 1 +
backends/hostmem-file.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 108 insertions(+)
create mode 100644 backends/hostmem-file.c
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 7fb7acd..506a46c 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -8,3 +8,4 @@ baum.o-cflags := $(SDL_CFLAGS)
common-obj-$(CONFIG_TPM) += tpm.o
common-obj-y += hostmem.o hostmem-ram.o
+common-obj-$(CONFIG_LINUX) += hostmem-file.o
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
new file mode 100644
index 0000000..9b75314
--- /dev/null
+++ b/backends/hostmem-file.c
@@ -0,0 +1,107 @@
+/*
+ * QEMU Host Memory Backend for hugetlbfs
+ *
+ * Copyright (C) 2013 Red Hat Inc
+ *
+ * Authors:
+ * Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "sysemu/hostmem.h"
+#include "qom/object_interfaces.h"
+
+/* hostmem-file.c */
+/**
+ * @TYPE_MEMORY_BACKEND_FILE:
+ * name of backend that uses mmap on a file descriptor
+ */
+#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file"
+
+#define MEMORY_BACKEND_FILE(obj) \
+ OBJECT_CHECK(HostMemoryBackendFile, (obj), TYPE_MEMORY_BACKEND_FILE)
+
+typedef struct HostMemoryBackendFile HostMemoryBackendFile;
+
+struct HostMemoryBackendFile {
+ HostMemoryBackend parent_obj;
+ char *mem_path;
+};
+
+static void
+file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
+{
+ HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
+
+ if (!backend->size) {
+ error_setg(errp, "can't create backend with size 0");
+ return;
+ }
+ if (!fb->mem_path) {
+ error_setg(errp, "mem_path property not set");
+ return;
+ }
+#ifndef CONFIG_LINUX
+ error_setg(errp, "-mem-path not supported on this host");
+#else
+ if (!memory_region_size(&backend->mr)) {
+ memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
+ object_get_canonical_path(OBJECT(backend)),
+ backend->size,
+ fb->mem_path, errp);
+ }
+#endif
+}
+
+static void
+file_backend_class_init(ObjectClass *oc, void *data)
+{
+ HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
+
+ bc->alloc = file_backend_memory_alloc;
+}
+
+static char *get_mem_path(Object *o, Error **errp)
+{
+ HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
+
+ return g_strdup(fb->mem_path);
+}
+
+static void set_mem_path(Object *o, const char *str, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(o);
+ HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
+
+ if (memory_region_size(&backend->mr)) {
+ error_setg(errp, "cannot change property value");
+ return;
+ }
+ if (fb->mem_path) {
+ g_free(fb->mem_path);
+ }
+ fb->mem_path = g_strdup(str);
+}
+
+static void
+file_backend_instance_init(Object *o)
+{
+ object_property_add_str(o, "mem-path", get_mem_path,
+ set_mem_path, NULL);
+}
+
+static const TypeInfo file_backend_info = {
+ .name = TYPE_MEMORY_BACKEND_FILE,
+ .parent = TYPE_MEMORY_BACKEND,
+ .class_init = file_backend_class_init,
+ .instance_init = file_backend_instance_init,
+ .instance_size = sizeof(HostMemoryBackendFile),
+};
+
+static void register_types(void)
+{
+ type_register_static(&file_backend_info);
+}
+
+type_init(register_types);
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH v5 08/16] hostmem: add file-based HostMemoryBackend
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 08/16] hostmem: add file-based HostMemoryBackend Hu Tao
@ 2014-06-11 8:03 ` Michael S. Tsirkin
0 siblings, 0 replies; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-11 8:03 UTC (permalink / raw)
To: Hu Tao
Cc: Eduardo Habkost, qemu-devel, Paolo Bonzini, Igor Mammedov, Yasunori Goto
On Tue, Jun 10, 2014 at 07:15:21PM +0800, Hu Tao wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> backends/Makefile.objs | 1 +
> backends/hostmem-file.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 108 insertions(+)
> create mode 100644 backends/hostmem-file.c
>
> diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> index 7fb7acd..506a46c 100644
> --- a/backends/Makefile.objs
> +++ b/backends/Makefile.objs
> @@ -8,3 +8,4 @@ baum.o-cflags := $(SDL_CFLAGS)
> common-obj-$(CONFIG_TPM) += tpm.o
>
> common-obj-y += hostmem.o hostmem-ram.o
> +common-obj-$(CONFIG_LINUX) += hostmem-file.o
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> new file mode 100644
> index 0000000..9b75314
> --- /dev/null
> +++ b/backends/hostmem-file.c
> @@ -0,0 +1,107 @@
> +/*
> + * QEMU Host Memory Backend for hugetlbfs
> + *
> + * Copyright (C) 2013 Red Hat Inc
I tweaked copyright to 2013-2014 here.
Not too important since the full history is in the git log, but still.
> + *
> + * Authors:
> + * Paolo Bonzini <pbonzini@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include "sysemu/hostmem.h"
> +#include "qom/object_interfaces.h"
> +
> +/* hostmem-file.c */
> +/**
> + * @TYPE_MEMORY_BACKEND_FILE:
> + * name of backend that uses mmap on a file descriptor
> + */
> +#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file"
> +
> +#define MEMORY_BACKEND_FILE(obj) \
> + OBJECT_CHECK(HostMemoryBackendFile, (obj), TYPE_MEMORY_BACKEND_FILE)
> +
> +typedef struct HostMemoryBackendFile HostMemoryBackendFile;
> +
> +struct HostMemoryBackendFile {
> + HostMemoryBackend parent_obj;
> + char *mem_path;
> +};
> +
> +static void
> +file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> +{
> + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
> +
> + if (!backend->size) {
> + error_setg(errp, "can't create backend with size 0");
> + return;
> + }
> + if (!fb->mem_path) {
> + error_setg(errp, "mem_path property not set");
> + return;
> + }
> +#ifndef CONFIG_LINUX
> + error_setg(errp, "-mem-path not supported on this host");
> +#else
> + if (!memory_region_size(&backend->mr)) {
> + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> + object_get_canonical_path(OBJECT(backend)),
> + backend->size,
> + fb->mem_path, errp);
> + }
> +#endif
> +}
> +
> +static void
> +file_backend_class_init(ObjectClass *oc, void *data)
> +{
> + HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
> +
> + bc->alloc = file_backend_memory_alloc;
> +}
> +
> +static char *get_mem_path(Object *o, Error **errp)
> +{
> + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> +
> + return g_strdup(fb->mem_path);
> +}
> +
> +static void set_mem_path(Object *o, const char *str, Error **errp)
> +{
> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
> + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> +
> + if (memory_region_size(&backend->mr)) {
> + error_setg(errp, "cannot change property value");
> + return;
> + }
> + if (fb->mem_path) {
> + g_free(fb->mem_path);
> + }
> + fb->mem_path = g_strdup(str);
> +}
> +
> +static void
> +file_backend_instance_init(Object *o)
> +{
> + object_property_add_str(o, "mem-path", get_mem_path,
> + set_mem_path, NULL);
> +}
> +
> +static const TypeInfo file_backend_info = {
> + .name = TYPE_MEMORY_BACKEND_FILE,
> + .parent = TYPE_MEMORY_BACKEND,
> + .class_init = file_backend_class_init,
> + .instance_init = file_backend_instance_init,
> + .instance_size = sizeof(HostMemoryBackendFile),
> +};
> +
> +static void register_types(void)
> +{
> + type_register_static(&file_backend_info);
> +}
> +
> +type_init(register_types);
> --
> 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH v5 09/16] hostmem: add merge and dump properties
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (7 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 08/16] hostmem: add file-based HostMemoryBackend Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 10/16] hostmem: allow preallocation of any memory region Hu Tao
` (7 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
From: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-
include/qemu/osdep.h | 10 ++++++
include/sysemu/hostmem.h | 1 +
3 files changed, 94 insertions(+), 1 deletion(-)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 1738774..a2550fe 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -53,8 +53,73 @@ out:
error_propagate(errp, local_err);
}
+static bool host_memory_backend_get_merge(Object *obj, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+
+ return backend->merge;
+}
+
+static void host_memory_backend_set_merge(Object *obj, bool value, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+
+ if (!memory_region_size(&backend->mr)) {
+ backend->merge = value;
+ return;
+ }
+
+ if (value != backend->merge) {
+ void *ptr = memory_region_get_ram_ptr(&backend->mr);
+ uint64_t sz = memory_region_size(&backend->mr);
+
+ qemu_madvise(ptr, sz,
+ value ? QEMU_MADV_MERGEABLE : QEMU_MADV_UNMERGEABLE);
+ backend->merge = value;
+ }
+}
+
+static bool host_memory_backend_get_dump(Object *obj, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+
+ return backend->dump;
+}
+
+static void host_memory_backend_set_dump(Object *obj, bool value, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+
+ if (!memory_region_size(&backend->mr)) {
+ backend->dump = value;
+ return;
+ }
+
+ if (value != backend->dump) {
+ void *ptr = memory_region_get_ram_ptr(&backend->mr);
+ uint64_t sz = memory_region_size(&backend->mr);
+
+ qemu_madvise(ptr, sz,
+ value ? QEMU_MADV_DODUMP : QEMU_MADV_DONTDUMP);
+ backend->dump = value;
+ }
+}
+
static void host_memory_backend_init(Object *obj)
{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+
+ backend->merge = qemu_opt_get_bool(qemu_get_machine_opts(),
+ "mem-merge", true);
+ backend->dump = qemu_opt_get_bool(qemu_get_machine_opts(),
+ "dump-guest-core", true);
+
+ object_property_add_bool(obj, "merge",
+ host_memory_backend_get_merge,
+ host_memory_backend_set_merge, NULL);
+ object_property_add_bool(obj, "dump",
+ host_memory_backend_get_dump,
+ host_memory_backend_set_dump, NULL);
object_property_add(obj, "size", "int",
host_memory_backend_get_size,
host_memory_backend_set_size, NULL, NULL, NULL);
@@ -80,9 +145,26 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
{
HostMemoryBackend *backend = MEMORY_BACKEND(uc);
HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
+ Error *local_err = NULL;
+ void *ptr;
+ uint64_t sz;
if (bc->alloc) {
- bc->alloc(backend, errp);
+ bc->alloc(backend, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ ptr = memory_region_get_ram_ptr(&backend->mr);
+ sz = memory_region_size(&backend->mr);
+
+ if (backend->merge) {
+ qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE);
+ }
+ if (!backend->dump) {
+ qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP);
+ }
}
}
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 9c1a119..820c5d0 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -116,6 +116,16 @@ void qemu_anon_ram_free(void *ptr, size_t size);
#else
#define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID
#endif
+#ifdef MADV_UNMERGEABLE
+#define QEMU_MADV_UNMERGEABLE MADV_UNMERGEABLE
+#else
+#define QEMU_MADV_UNMERGEABLE QEMU_MADV_INVALID
+#endif
+#ifdef MADV_DODUMP
+#define QEMU_MADV_DODUMP MADV_DODUMP
+#else
+#define QEMU_MADV_DODUMP QEMU_MADV_INVALID
+#endif
#ifdef MADV_DONTDUMP
#define QEMU_MADV_DONTDUMP MADV_DONTDUMP
#else
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 923f672..ede5ec9 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -52,6 +52,7 @@ struct HostMemoryBackend {
/* protected */
uint64_t size;
+ bool merge, dump;
MemoryRegion mr;
};
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 10/16] hostmem: allow preallocation of any memory region
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (8 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 09/16] hostmem: add merge and dump properties Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 11/16] hostmem: add property to map memory with MAP_SHARED Hu Tao
` (6 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
From: Paolo Bonzini <pbonzini@redhat.com>
And allow preallocation of file-based memory even without -mem-prealloc.
Some care is necessary because -mem-prealloc does not allow disabling
preallocation for hostmem-file.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem-file.c | 3 +++
backends/hostmem.c | 42 ++++++++++++++++++++++++++++++++++++++++++
exec.c | 7 +++++++
include/exec/memory.h | 10 ++++++++++
include/exec/ram_addr.h | 1 +
include/sysemu/hostmem.h | 1 +
memory.c | 11 +++++++++++
7 files changed, 75 insertions(+)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 9b75314..35e32d1 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -9,7 +9,9 @@
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
*/
+#include "qemu-common.h"
#include "sysemu/hostmem.h"
+#include "sysemu/sysemu.h"
#include "qom/object_interfaces.h"
/* hostmem-file.c */
@@ -46,6 +48,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
error_setg(errp, "-mem-path not supported on this host");
#else
if (!memory_region_size(&backend->mr)) {
+ backend->force_prealloc = mem_prealloc;
memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
object_get_canonical_path(OBJECT(backend)),
backend->size,
diff --git a/backends/hostmem.c b/backends/hostmem.c
index a2550fe..ebef620 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -105,6 +105,41 @@ static void host_memory_backend_set_dump(Object *obj, bool value, Error **errp)
}
}
+static bool host_memory_backend_get_prealloc(Object *obj, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+
+ return backend->prealloc || backend->force_prealloc;
+}
+
+static void host_memory_backend_set_prealloc(Object *obj, bool value,
+ Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+
+ if (backend->force_prealloc) {
+ if (value) {
+ error_setg(errp,
+ "remove -mem-prealloc to use the prealloc property");
+ return;
+ }
+ }
+
+ if (!memory_region_size(&backend->mr)) {
+ backend->prealloc = value;
+ return;
+ }
+
+ if (value && !backend->prealloc) {
+ int fd = memory_region_get_fd(&backend->mr);
+ void *ptr = memory_region_get_ram_ptr(&backend->mr);
+ uint64_t sz = memory_region_size(&backend->mr);
+
+ os_mem_prealloc(fd, ptr, sz);
+ backend->prealloc = true;
+ }
+}
+
static void host_memory_backend_init(Object *obj)
{
HostMemoryBackend *backend = MEMORY_BACKEND(obj);
@@ -113,6 +148,7 @@ static void host_memory_backend_init(Object *obj)
"mem-merge", true);
backend->dump = qemu_opt_get_bool(qemu_get_machine_opts(),
"dump-guest-core", true);
+ backend->prealloc = mem_prealloc;
object_property_add_bool(obj, "merge",
host_memory_backend_get_merge,
@@ -120,6 +156,9 @@ static void host_memory_backend_init(Object *obj)
object_property_add_bool(obj, "dump",
host_memory_backend_get_dump,
host_memory_backend_set_dump, NULL);
+ object_property_add_bool(obj, "prealloc",
+ host_memory_backend_get_prealloc,
+ host_memory_backend_set_prealloc, NULL);
object_property_add(obj, "size", "int",
host_memory_backend_get_size,
host_memory_backend_set_size, NULL, NULL, NULL);
@@ -165,6 +204,9 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
if (!backend->dump) {
qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP);
}
+ if (backend->prealloc) {
+ os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
+ }
}
}
diff --git a/exec.c b/exec.c
index 739f0cf..520d673 100644
--- a/exec.c
+++ b/exec.c
@@ -1432,6 +1432,13 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
}
#endif /* !_WIN32 */
+int qemu_get_ram_fd(ram_addr_t addr)
+{
+ RAMBlock *block = qemu_get_ram_block(addr);
+
+ return block->fd;
+}
+
/* Return a host pointer to ram allocated with qemu_ram_alloc.
With the exception of the softmmu code in this file, this should
only be used for local memory (e.g. video ram) that the device owns,
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 82d7781..36226f7 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -534,6 +534,16 @@ bool memory_region_is_logging(MemoryRegion *mr);
bool memory_region_is_rom(MemoryRegion *mr);
/**
+ * memory_region_get_fd: Get a file descriptor backing a RAM memory region.
+ *
+ * Returns a file descriptor backing a file-based RAM memory region,
+ * or -1 if the region is not a file-based RAM memory region.
+ *
+ * @mr: the RAM or alias memory region being queried.
+ */
+int memory_region_get_fd(MemoryRegion *mr);
+
+/**
* memory_region_get_ram_ptr: Get a pointer into a RAM memory region.
*
* Returns a host pointer to a RAM memory region (created with
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index f9518a6..d352f60 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -27,6 +27,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
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);
+int qemu_get_ram_fd(ram_addr_t addr);
void *qemu_get_ram_ptr(ram_addr_t addr);
void qemu_ram_free(ram_addr_t addr);
void qemu_ram_free_from_ptr(ram_addr_t addr);
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index ede5ec9..4cae673 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -53,6 +53,7 @@ struct HostMemoryBackend {
/* protected */
uint64_t size;
bool merge, dump;
+ bool prealloc, force_prealloc;
MemoryRegion mr;
};
diff --git a/memory.c b/memory.c
index 310729a..bcef72b 100644
--- a/memory.c
+++ b/memory.c
@@ -1258,6 +1258,17 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client);
}
+int memory_region_get_fd(MemoryRegion *mr)
+{
+ if (mr->alias) {
+ return memory_region_get_fd(mr->alias);
+ }
+
+ assert(mr->terminates);
+
+ return qemu_get_ram_fd(mr->ram_addr & TARGET_PAGE_MASK);
+}
+
void *memory_region_get_ram_ptr(MemoryRegion *mr)
{
if (mr->alias) {
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 11/16] hostmem: add property to map memory with MAP_SHARED
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (9 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 10/16] hostmem: allow preallocation of any memory region Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 12/16] hostmem: add properties for NUMA memory policy Hu Tao
` (5 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
From: Paolo Bonzini <pbonzini@redhat.com>
A new "share" property can be used with the "memory-file" backend to
map memory with MAP_SHARED instead of MAP_PRIVATE.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem-file.c | 26 +++++++++++++++++++++++++-
exec.c | 18 ++++++++++--------
include/exec/memory.h | 2 ++
include/exec/ram_addr.h | 3 ++-
memory.c | 3 ++-
numa.c | 2 +-
6 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 35e32d1..d20f79b 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -28,6 +28,8 @@ typedef struct HostMemoryBackendFile HostMemoryBackendFile;
struct HostMemoryBackendFile {
HostMemoryBackend parent_obj;
+
+ bool share;
char *mem_path;
};
@@ -51,7 +53,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
backend->force_prealloc = mem_prealloc;
memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
object_get_canonical_path(OBJECT(backend)),
- backend->size,
+ backend->size, fb->share,
fb->mem_path, errp);
}
#endif
@@ -87,9 +89,31 @@ static void set_mem_path(Object *o, const char *str, Error **errp)
fb->mem_path = g_strdup(str);
}
+static bool file_memory_backend_get_share(Object *o, Error **errp)
+{
+ HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
+
+ return fb->share;
+}
+
+static void file_memory_backend_set_share(Object *o, bool value, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(o);
+ HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
+
+ if (memory_region_size(&backend->mr)) {
+ error_setg(errp, "cannot change property value");
+ return;
+ }
+ fb->share = value;
+}
+
static void
file_backend_instance_init(Object *o)
{
+ object_property_add_bool(o, "share",
+ file_memory_backend_get_share,
+ file_memory_backend_set_share, NULL);
object_property_add_str(o, "mem-path", get_mem_path,
set_mem_path, NULL);
}
diff --git a/exec.c b/exec.c
index 520d673..8705cc5 100644
--- a/exec.c
+++ b/exec.c
@@ -73,6 +73,9 @@ static MemoryRegion io_mem_unassigned;
/* RAM is pre-allocated and passed into qemu_ram_alloc_from_ptr */
#define RAM_PREALLOC (1 << 0)
+/* RAM is mmap-ed with MAP_SHARED */
+#define RAM_SHARED (1 << 1)
+
#endif
struct CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
@@ -1074,7 +1077,9 @@ static void *file_ram_alloc(RAMBlock *block,
perror("ftruncate");
}
- area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+ area = mmap(0, memory, PROT_READ | PROT_WRITE,
+ (block->flags & RAM_SHARED ? MAP_SHARED : MAP_PRIVATE),
+ fd, 0);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
"unable to map backing store for hugepages");
@@ -1270,7 +1275,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
#ifdef __linux__
ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
- const char *mem_path,
+ bool share, const char *mem_path,
Error **errp)
{
RAMBlock *new_block;
@@ -1295,6 +1300,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
new_block = g_malloc0(sizeof(*new_block));
new_block->mr = mr;
new_block->length = size;
+ new_block->flags = share ? RAM_SHARED : 0;
new_block->host = file_ram_alloc(new_block, size,
mem_path, errp);
if (!new_block->host) {
@@ -1397,12 +1403,8 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
flags = MAP_FIXED;
munmap(vaddr, length);
if (block->fd >= 0) {
-#ifdef MAP_POPULATE
- flags |= mem_prealloc ? MAP_POPULATE | MAP_SHARED :
- MAP_PRIVATE;
-#else
- flags |= MAP_PRIVATE;
-#endif
+ flags |= (block->flags & RAM_SHARED ?
+ MAP_SHARED : MAP_PRIVATE);
area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
flags, block->fd, offset);
} else {
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 36226f7..f01f623 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -321,6 +321,7 @@ void memory_region_init_ram(MemoryRegion *mr,
* @owner: the object that tracks the region's reference count
* @name: the name of the region.
* @size: size of the region.
+ * @share: %true if memory must be mmaped with the MAP_SHARED flag
* @path: the path in which to allocate the RAM.
* @errp: pointer to Error*, to store an error if it happens.
*/
@@ -328,6 +329,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
struct Object *owner,
const char *name,
uint64_t size,
+ bool share,
const char *path,
Error **errp);
#endif
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index d352f60..1d4ac74 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -23,7 +23,8 @@
#include "hw/xen/xen.h"
ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
- const char *mem_path, Error **errp);
+ 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);
diff --git a/memory.c b/memory.c
index bcef72b..203b097 100644
--- a/memory.c
+++ b/memory.c
@@ -1025,6 +1025,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
struct Object *owner,
const char *name,
uint64_t size,
+ bool share,
const char *path,
Error **errp)
{
@@ -1032,7 +1033,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
- mr->ram_addr = qemu_ram_alloc_from_file(size, mr, path, errp);
+ mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, errp);
}
#endif
diff --git a/numa.c b/numa.c
index 6ea267e..8ba5a38 100644
--- a/numa.c
+++ b/numa.c
@@ -231,7 +231,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
if (mem_path) {
#ifdef __linux__
Error *err = NULL;
- memory_region_init_ram_from_file(mr, owner, name, ram_size,
+ memory_region_init_ram_from_file(mr, owner, name, ram_size, false,
mem_path, &err);
/* Legacy behavior: if allocation failed, fall back to
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 12/16] hostmem: add properties for NUMA memory policy
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (10 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 11/16] hostmem: add property to map memory with MAP_SHARED Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 13/16] tests: fix memory leak in test of string input visitor Hu Tao
` (4 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Marcelo Tosatti,
Paolo Bonzini, Igor Mammedov, Yasunori Goto
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
[Raise errors on setting properties if !CONFIG_NUMA. Add BUILD_BUG_ON
checks. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++-
include/sysemu/hostmem.h | 4 ++
qapi-schema.json | 20 +++++++
3 files changed, 159 insertions(+), 1 deletion(-)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index ebef620..ca10c51 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -10,12 +10,21 @@
* See the COPYING file in the top-level directory.
*/
#include "sysemu/hostmem.h"
-#include "sysemu/sysemu.h"
#include "qapi/visitor.h"
+#include "qapi-types.h"
+#include "qapi-visit.h"
#include "qapi/qmp/qerror.h"
#include "qemu/config-file.h"
#include "qom/object_interfaces.h"
+#ifdef CONFIG_NUMA
+#include <numaif.h>
+QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_DEFAULT != MPOL_DEFAULT);
+QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_PREFERRED != MPOL_PREFERRED);
+QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_BIND != MPOL_BIND);
+QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_INTERLEAVE != MPOL_INTERLEAVE);
+#endif
+
static void
host_memory_backend_get_size(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
@@ -53,6 +62,84 @@ out:
error_propagate(errp, local_err);
}
+static void
+host_memory_backend_get_host_nodes(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+ uint16List *host_nodes = NULL;
+ uint16List **node = &host_nodes;
+ unsigned long value;
+
+ value = find_first_bit(backend->host_nodes, MAX_NODES);
+ if (value == MAX_NODES) {
+ return;
+ }
+
+ *node = g_malloc0(sizeof(**node));
+ (*node)->value = value;
+ node = &(*node)->next;
+
+ do {
+ value = find_next_bit(backend->host_nodes, MAX_NODES, value + 1);
+ if (value == MAX_NODES) {
+ break;
+ }
+
+ *node = g_malloc0(sizeof(**node));
+ (*node)->value = value;
+ node = &(*node)->next;
+ } while (true);
+
+ visit_type_uint16List(v, &host_nodes, name, errp);
+}
+
+static void
+host_memory_backend_set_host_nodes(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+#ifdef CONFIG_NUMA
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+ uint16List *l = NULL;
+
+ visit_type_uint16List(v, &l, name, errp);
+
+ while (l) {
+ bitmap_set(backend->host_nodes, l->value, 1);
+ l = l->next;
+ }
+#else
+ error_setg(errp, "NUMA node binding are not supported by this QEMU");
+#endif
+}
+
+static void
+host_memory_backend_get_policy(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+ int policy = backend->policy;
+
+ visit_type_enum(v, &policy, HostMemPolicy_lookup, NULL, name, errp);
+}
+
+static void
+host_memory_backend_set_policy(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+ int policy;
+
+ visit_type_enum(v, &policy, HostMemPolicy_lookup, NULL, name, errp);
+ backend->policy = policy;
+
+#ifndef CONFIG_NUMA
+ if (policy != HOST_MEM_POLICY_DEFAULT) {
+ error_setg(errp, "NUMA policies are not supported by this QEMU");
+ }
+#endif
+}
+
static bool host_memory_backend_get_merge(Object *obj, Error **errp)
{
HostMemoryBackend *backend = MEMORY_BACKEND(obj);
@@ -162,6 +249,12 @@ static void host_memory_backend_init(Object *obj)
object_property_add(obj, "size", "int",
host_memory_backend_get_size,
host_memory_backend_set_size, NULL, NULL, NULL);
+ object_property_add(obj, "host-nodes", "int",
+ host_memory_backend_get_host_nodes,
+ host_memory_backend_set_host_nodes, NULL, NULL, NULL);
+ object_property_add(obj, "policy", "str",
+ host_memory_backend_get_policy,
+ host_memory_backend_set_policy, NULL, NULL, NULL);
}
static void host_memory_backend_finalize(Object *obj)
@@ -204,6 +297,47 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
if (!backend->dump) {
qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP);
}
+#ifdef CONFIG_NUMA
+ unsigned long lastbit = find_last_bit(backend->host_nodes, MAX_NODES);
+ /* lastbit == MAX_NODES means maxnode = 0 */
+ unsigned long maxnode = (lastbit + 1) % (MAX_NODES + 1);
+ /* ensure policy won't be ignored in case memory is preallocated
+ * before mbind(). note: MPOL_MF_STRICT is ignored on hugepages so
+ * this doesn't catch hugepage case. */
+ unsigned flags = MPOL_MF_STRICT;
+
+ /* check for invalid host-nodes and policies and give more verbose
+ * error messages than mbind(). */
+ if (maxnode && backend->policy == MPOL_DEFAULT) {
+ error_setg(errp, "host-nodes must be empty for policy default,"
+ " or you should explicitly specify a policy other"
+ " than default");
+ return;
+ } else if (maxnode == 0 && backend->policy != MPOL_DEFAULT) {
+ error_setg(errp, "host-nodes must be set for policy %s",
+ HostMemPolicy_lookup[backend->policy]);
+ return;
+ }
+
+ /* We can have up to MAX_NODES nodes, but we need to pass maxnode+1
+ * as argument to mbind() due to an old Linux bug (feature?) which
+ * cuts off the last specified node. This means backend->host_nodes
+ * must have MAX_NODES+1 bits available.
+ */
+ assert(sizeof(backend->host_nodes) >=
+ BITS_TO_LONGS(MAX_NODES + 1) * sizeof(unsigned long));
+ assert(maxnode <= MAX_NODES);
+ if (mbind(ptr, sz, backend->policy,
+ maxnode ? backend->host_nodes : NULL, maxnode + 1, flags)) {
+ error_setg_errno(errp, errno,
+ "cannot bind memory to host NUMA nodes");
+ return;
+ }
+#endif
+ /* Preallocate memory after the NUMA policy has been instantiated.
+ * This is necessary to guarantee memory is allocated with
+ * specified NUMA policy in place.
+ */
if (backend->prealloc) {
os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
}
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 4cae673..1ce4394 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -12,10 +12,12 @@
#ifndef QEMU_RAM_H
#define QEMU_RAM_H
+#include "sysemu/sysemu.h" /* for MAX_NODES */
#include "qom/object.h"
#include "qapi/error.h"
#include "exec/memory.h"
#include "qemu/option.h"
+#include "qemu/bitmap.h"
#define TYPE_MEMORY_BACKEND "memory-backend"
#define MEMORY_BACKEND(obj) \
@@ -54,6 +56,8 @@ struct HostMemoryBackend {
uint64_t size;
bool merge, dump;
bool prealloc, force_prealloc;
+ DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
+ HostMemPolicy policy;
MemoryRegion mr;
};
diff --git a/qapi-schema.json b/qapi-schema.json
index d5ab066..0898c00 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4759,3 +4759,23 @@
'*cpus': ['uint16'],
'*mem': 'size',
'*memdev': 'str' }}
+
+##
+# @HostMemPolicy
+#
+# Host memory policy types
+#
+# @default: restore default policy, remove any nondefault policy
+#
+# @preferred: set the preferred host nodes for allocation
+#
+# @bind: a strict policy that restricts memory allocation to the
+# host nodes specified
+#
+# @interleave: memory allocations are interleaved across the set
+# of host nodes specified
+#
+# Since 2.1
+##
+{ 'enum': 'HostMemPolicy',
+ 'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 13/16] tests: fix memory leak in test of string input visitor
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (11 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 12/16] hostmem: add properties for NUMA memory policy Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 14/16] qapi: make string input visitor parse int list Hu Tao
` (3 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
tests/test-string-input-visitor.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 877e737..01a78f0 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -193,6 +193,7 @@ static void test_visitor_in_fuzz(TestInputVisitorData *data,
v = visitor_input_test_init(data, buf);
visit_type_int(v, &ires, NULL, NULL);
+ visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, buf);
visit_type_bool(v, &bres, NULL, NULL);
@@ -200,11 +201,13 @@ static void test_visitor_in_fuzz(TestInputVisitorData *data,
v = visitor_input_test_init(data, buf);
visit_type_number(v, &nres, NULL, NULL);
+ visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, buf);
sres = NULL;
visit_type_str(v, &sres, NULL, NULL);
g_free(sres);
+ visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, buf);
visit_type_EnumOne(v, &eres, NULL, NULL);
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 14/16] qapi: make string input visitor parse int list
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (12 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 13/16] tests: fix memory leak in test of string input visitor Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 15/16] qapi: make string output " Hu Tao
` (2 subsequent siblings)
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
qapi/string-input-visitor.c | 181 ++++++++++++++++++++++++++++++++++++--
tests/test-string-input-visitor.c | 36 ++++++++
2 files changed, 209 insertions(+), 8 deletions(-)
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 5780944..85ac6a1 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -15,31 +15,182 @@
#include "qapi/visitor-impl.h"
#include "qapi/qmp/qerror.h"
#include "qemu/option.h"
+#include "qemu/queue.h"
+#include "qemu/range.h"
+
struct StringInputVisitor
{
Visitor visitor;
+
+ bool head;
+
+ SignedRangeList *ranges;
+ SignedRange *cur_range;
+ int64_t cur;
+
const char *string;
};
+static void parse_str(StringInputVisitor *siv, Error **errp)
+{
+ char *str = (char *) siv->string;
+ long long start, end;
+ SignedRange *r, *next;
+ char *endptr;
+
+ if (siv->ranges) {
+ return;
+ }
+
+ siv->ranges = g_malloc0(sizeof(*siv->ranges));
+ QTAILQ_INIT(siv->ranges);
+ errno = 0;
+ do {
+ start = strtoll(str, &endptr, 0);
+ if (errno == 0 && endptr > str && INT64_MIN <= start &&
+ start <= INT64_MAX) {
+ if (*endptr == '\0') {
+ if (!range_list_add(siv->ranges, start, 1)) {
+ goto error;
+ }
+ str = NULL;
+ } else if (*endptr == '-') {
+ str = endptr + 1;
+ end = strtoll(str, &endptr, 0);
+ if (errno == 0 && endptr > str &&
+ INT64_MIN <= end && end <= INT64_MAX && start <= end &&
+ (start > INT64_MAX - 65536 ||
+ end < start + 65536)) {
+ if (*endptr == '\0') {
+ if (!range_list_add(siv->ranges, start,
+ end - start + 1)) {
+ goto error;
+ }
+ str = NULL;
+ } else if (*endptr == ',') {
+ str = endptr + 1;
+ if (!range_list_add(siv->ranges, start,
+ end - start + 1)) {
+ goto error;
+ }
+ } else {
+ goto error;
+ }
+ } else {
+ goto error;
+ }
+ } else if (*endptr == ',') {
+ str = endptr + 1;
+ if (!range_list_add(siv->ranges, start, 1)) {
+ goto error;
+ }
+ } else {
+ goto error;
+ }
+ } else {
+ goto error;
+ }
+ } while (str);
+
+ return;
+error:
+ if (siv->ranges) {
+ QTAILQ_FOREACH_SAFE(r, siv->ranges, entry, next) {
+ QTAILQ_REMOVE(siv->ranges, r, entry);
+ g_free(r);
+ }
+ g_free(siv->ranges);
+ siv->ranges = NULL;
+ }
+}
+
+static void
+start_list(Visitor *v, const char *name, Error **errp)
+{
+ StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+
+ parse_str(siv, errp);
+
+ if (siv->ranges) {
+ siv->cur_range = QTAILQ_FIRST(siv->ranges);
+ if (siv->cur_range) {
+ siv->cur = siv->cur_range->start;
+ }
+ }
+}
+
+static GenericList *
+next_list(Visitor *v, GenericList **list, Error **errp)
+{
+ StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+ GenericList **link;
+
+ if (!siv->ranges || !siv->cur_range) {
+ return NULL;
+ }
+
+ if (siv->cur < siv->cur_range->start ||
+ siv->cur >= (siv->cur_range->start + siv->cur_range->length)) {
+ siv->cur_range = QTAILQ_NEXT(siv->cur_range, entry);
+ if (siv->cur_range) {
+ siv->cur = siv->cur_range->start;
+ } else {
+ return NULL;
+ }
+ }
+
+ if (siv->head) {
+ link = list;
+ siv->head = false;
+ } else {
+ link = &(*list)->next;
+ }
+
+ *link = g_malloc0(sizeof **link);
+ return *link;
+}
+
+static void
+end_list(Visitor *v, Error **errp)
+{
+ StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+ siv->head = true;
+}
+
static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
Error **errp)
{
StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
- char *endp = (char *) siv->string;
- long long val;
- errno = 0;
- if (siv->string) {
- val = strtoll(siv->string, &endp, 0);
- }
- if (!siv->string || errno || endp == siv->string || *endp) {
+ if (!siv->string) {
error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"integer");
return;
}
- *obj = val;
+ parse_str(siv, errp);
+
+ if (!siv->ranges) {
+ goto error;
+ }
+
+ if (!siv->cur_range) {
+ siv->cur_range = QTAILQ_FIRST(siv->ranges);
+ if (siv->cur_range) {
+ siv->cur = siv->cur_range->start;
+ } else {
+ goto error;
+ }
+ }
+
+ *obj = siv->cur;
+ siv->cur++;
+ return;
+
+error:
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
+ "an int64 value or range");
}
static void parse_type_size(Visitor *v, uint64_t *obj, const char *name,
@@ -140,6 +291,16 @@ Visitor *string_input_get_visitor(StringInputVisitor *v)
void string_input_visitor_cleanup(StringInputVisitor *v)
{
+ SignedRange *r, *next;
+
+ if (v->ranges) {
+ QTAILQ_FOREACH_SAFE(r, v->ranges, entry, next) {
+ QTAILQ_REMOVE(v->ranges, r, entry);
+ g_free(r);
+ }
+ g_free(v->ranges);
+ }
+
g_free(v);
}
@@ -155,8 +316,12 @@ StringInputVisitor *string_input_visitor_new(const char *str)
v->visitor.type_bool = parse_type_bool;
v->visitor.type_str = parse_type_str;
v->visitor.type_number = parse_type_number;
+ v->visitor.start_list = start_list;
+ v->visitor.next_list = next_list;
+ v->visitor.end_list = end_list;
v->visitor.optional = parse_optional;
v->string = str;
+ v->head = true;
return v;
}
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 01a78f0..b08a7db 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -64,6 +64,35 @@ static void test_visitor_in_int(TestInputVisitorData *data,
g_assert_cmpint(res, ==, value);
}
+static void test_visitor_in_intList(TestInputVisitorData *data,
+ const void *unused)
+{
+ int64_t value[] = {-2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
+ int16List *res = NULL, *tmp;
+ Error *errp = NULL;
+ Visitor *v;
+ int i = 0;
+
+ v = visitor_input_test_init(data, "1,2,-2-1,2-4,20,5-9,1-8");
+
+ visit_type_int16List(v, &res, NULL, &errp);
+ g_assert(errp == NULL);
+ tmp = res;
+ while (i < sizeof(value) / sizeof(value[0])) {
+ g_assert(tmp);
+ g_assert_cmpint(tmp->value, ==, value[i++]);
+ tmp = tmp->next;
+ }
+ g_assert(!tmp);
+
+ tmp = res;
+ while (tmp) {
+ res = res->next;
+ g_free(tmp);
+ tmp = res;
+ }
+}
+
static void test_visitor_in_bool(TestInputVisitorData *data,
const void *unused)
{
@@ -170,6 +199,7 @@ static void test_visitor_in_fuzz(TestInputVisitorData *data,
const void *unused)
{
int64_t ires;
+ intList *ilres;
bool bres;
double nres;
char *sres;
@@ -196,6 +226,10 @@ static void test_visitor_in_fuzz(TestInputVisitorData *data,
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, buf);
+ visit_type_intList(v, &ilres, NULL, NULL);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, buf);
visit_type_bool(v, &bres, NULL, NULL);
visitor_input_teardown(data, NULL);
@@ -231,6 +265,8 @@ int main(int argc, char **argv)
input_visitor_test_add("/string-visitor/input/int",
&in_visitor_data, test_visitor_in_int);
+ input_visitor_test_add("/string-visitor/input/intList",
+ &in_visitor_data, test_visitor_in_intList);
input_visitor_test_add("/string-visitor/input/bool",
&in_visitor_data, test_visitor_in_bool);
input_visitor_test_add("/string-visitor/input/number",
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 15/16] qapi: make string output visitor parse int list
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (13 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 14/16] qapi: make string input visitor parse int list Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 16/16] qmp: add query-memdev Hu Tao
2014-06-12 7:41 ` [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Michael S. Tsirkin
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
qapi/string-output-visitor.c | 230 +++++++++++++++++++++++++++++++++++--
tests/test-string-output-visitor.c | 34 ++++++
2 files changed, 254 insertions(+), 10 deletions(-)
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index fb1d2e8..ccebc7a 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -16,32 +16,173 @@
#include "qapi/qmp/qerror.h"
#include "qemu/host-utils.h"
#include <math.h>
+#include "qemu/range.h"
+
+enum ListMode {
+ LM_NONE, /* not traversing a list of repeated options */
+ LM_STARTED, /* start_list() succeeded */
+
+ LM_IN_PROGRESS, /* next_list() has been called.
+ *
+ * Generating the next list link will consume the most
+ * recently parsed QemuOpt instance of the repeated
+ * option.
+ *
+ * Parsing a value into the list link will examine the
+ * next QemuOpt instance of the repeated option, and
+ * possibly enter LM_SIGNED_INTERVAL or
+ * LM_UNSIGNED_INTERVAL.
+ */
+
+ LM_SIGNED_INTERVAL, /* next_list() has been called.
+ *
+ * Generating the next list link will consume the most
+ * recently stored element from the signed interval,
+ * parsed from the most recent QemuOpt instance of the
+ * repeated option. This may consume QemuOpt itself
+ * and return to LM_IN_PROGRESS.
+ *
+ * Parsing a value into the list link will store the
+ * next element of the signed interval.
+ */
+
+ LM_UNSIGNED_INTERVAL,/* Same as above, only for an unsigned interval. */
+
+ LM_END
+};
+
+typedef enum ListMode ListMode;
struct StringOutputVisitor
{
Visitor visitor;
bool human;
- char *string;
+ GString *string;
+ bool head;
+ ListMode list_mode;
+ union {
+ int64_t s;
+ uint64_t u;
+ } range_start, range_end;
+ SignedRangeList *ranges;
};
static void string_output_set(StringOutputVisitor *sov, char *string)
{
- g_free(sov->string);
- sov->string = string;
+ if (sov->string) {
+ g_string_free(sov->string, true);
+ }
+ sov->string = g_string_new(string);
+ g_free(string);
+}
+
+static void string_output_append(StringOutputVisitor *sov, int64_t a)
+{
+ range_list_add(sov->ranges, a, 1);
+}
+
+static void string_output_append_range(StringOutputVisitor *sov,
+ int64_t s, int64_t e)
+{
+ range_list_add(sov->ranges, s, e);
}
static void print_type_int(Visitor *v, int64_t *obj, const char *name,
Error **errp)
{
StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
- char *out;
+ SignedRange *r;
+
+ if (!sov->ranges) {
+ sov->ranges = g_malloc0(sizeof(*sov->ranges));
+ QTAILQ_INIT(sov->ranges);
+ }
+
+ switch (sov->list_mode) {
+ case LM_NONE:
+ string_output_append(sov, *obj);
+ break;
+
+ case LM_STARTED:
+ sov->range_start.s = *obj;
+ sov->range_end.s = *obj;
+ sov->list_mode = LM_IN_PROGRESS;
+ return;
+
+ case LM_IN_PROGRESS:
+ if (sov->range_end.s + 1 == *obj) {
+ sov->range_end.s++;
+ } else {
+ if (sov->range_start.s == sov->range_end.s) {
+ string_output_append(sov, sov->range_end.s);
+ } else {
+ assert(sov->range_start.s < sov->range_end.s);
+ string_output_append_range(sov, sov->range_start.s,
+ sov->range_end.s -
+ sov->range_start.s + 1);
+ }
+
+ sov->range_start.s = *obj;
+ sov->range_end.s = *obj;
+ }
+ return;
+
+ case LM_END:
+ if (sov->range_end.s + 1 == *obj) {
+ sov->range_end.s++;
+ assert(sov->range_start.s < sov->range_end.s);
+ string_output_append_range(sov, sov->range_start.s,
+ sov->range_end.s -
+ sov->range_start.s + 1);
+ } else {
+ if (sov->range_start.s == sov->range_end.s) {
+ string_output_append(sov, sov->range_end.s);
+ } else {
+ assert(sov->range_start.s < sov->range_end.s);
+
+ string_output_append_range(sov, sov->range_start.s,
+ sov->range_end.s -
+ sov->range_start.s + 1);
+ }
+ string_output_append(sov, *obj);
+ }
+ break;
+
+ default:
+ abort();
+ }
+
+ QTAILQ_FOREACH(r, sov->ranges, entry) {
+ if (r->length > 1) {
+ g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
+ r->start,
+ s_range_end(r->start, r->length));
+ } else {
+ g_string_append_printf(sov->string, "%" PRId64,
+ r->start);
+ }
+ if (r->entry.tqe_next) {
+ g_string_append(sov->string, ",");
+ }
+ }
if (sov->human) {
- out = g_strdup_printf("%lld (%#llx)", (long long) *obj, (long long) *obj);
- } else {
- out = g_strdup_printf("%lld", (long long) *obj);
+ g_string_append(sov->string, " (");
+ QTAILQ_FOREACH(r, sov->ranges, entry) {
+ if (r->length > 1) {
+ g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
+ r->start,
+ s_range_end(r->start, r->length));
+ } else {
+ g_string_append_printf(sov->string, "%" PRIx64,
+ r->start);
+ }
+ if (r->entry.tqe_next) {
+ g_string_append(sov->string, ",");
+ }
+ }
+ g_string_append(sov->string, ")");
}
- string_output_set(sov, out);
}
static void print_type_size(Visitor *v, uint64_t *obj, const char *name,
@@ -103,9 +244,61 @@ static void print_type_number(Visitor *v, double *obj, const char *name,
string_output_set(sov, g_strdup_printf("%f", *obj));
}
+static void
+start_list(Visitor *v, const char *name, Error **errp)
+{
+ StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
+
+ /* we can't traverse a list in a list */
+ assert(sov->list_mode == LM_NONE);
+ sov->list_mode = LM_STARTED;
+ sov->head = true;
+}
+
+static GenericList *
+next_list(Visitor *v, GenericList **list, Error **errp)
+{
+ StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
+ GenericList *ret = NULL;
+ if (*list) {
+ if (sov->head) {
+ ret = *list;
+ } else {
+ ret = (*list)->next;
+ }
+
+ if (sov->head) {
+ if (ret && ret->next == NULL) {
+ sov->list_mode = LM_NONE;
+ }
+ sov->head = false;
+ } else {
+ if (ret && ret->next == NULL) {
+ sov->list_mode = LM_END;
+ }
+ }
+ }
+
+ return ret;
+}
+
+static void
+end_list(Visitor *v, Error **errp)
+{
+ StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
+
+ assert(sov->list_mode == LM_STARTED ||
+ sov->list_mode == LM_END ||
+ sov->list_mode == LM_NONE ||
+ sov->list_mode == LM_IN_PROGRESS);
+ sov->list_mode = LM_NONE;
+ sov->head = true;
+
+}
+
char *string_output_get_string(StringOutputVisitor *sov)
{
- char *string = sov->string;
+ char *string = g_string_free(sov->string, false);
sov->string = NULL;
return string;
}
@@ -117,7 +310,20 @@ Visitor *string_output_get_visitor(StringOutputVisitor *sov)
void string_output_visitor_cleanup(StringOutputVisitor *sov)
{
- g_free(sov->string);
+ SignedRange *r, *next;
+
+ if (sov->string) {
+ g_string_free(sov->string, true);
+ }
+
+ if (sov->ranges) {
+ QTAILQ_FOREACH_SAFE(r, sov->ranges, entry, next) {
+ QTAILQ_REMOVE(sov->ranges, r, entry);
+ s_range_free(r);
+ }
+ g_free(sov->ranges);
+ }
+
g_free(sov);
}
@@ -127,6 +333,7 @@ StringOutputVisitor *string_output_visitor_new(bool human)
v = g_malloc0(sizeof(*v));
+ v->string = g_string_new(NULL);
v->human = human;
v->visitor.type_enum = output_type_enum;
v->visitor.type_int = print_type_int;
@@ -134,6 +341,9 @@ StringOutputVisitor *string_output_visitor_new(bool human)
v->visitor.type_bool = print_type_bool;
v->visitor.type_str = print_type_str;
v->visitor.type_number = print_type_number;
+ v->visitor.start_list = start_list;
+ v->visitor.next_list = next_list;
+ v->visitor.end_list = end_list;
return v;
}
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 2af5a21..d2ad580 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -57,6 +57,38 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
g_free(str);
}
+static void test_visitor_out_intList(TestOutputVisitorData *data,
+ const void *unused)
+{
+ int64_t value[] = {-10, -7, -2, -1, 0, 1, 9, 10, 16, 15, 14,
+ 3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};
+ intList *list = NULL, **tmp = &list;
+ int i;
+ Error *errp = NULL;
+ char *str;
+
+ for (i = 0; i < sizeof(value) / sizeof(value[0]); i++) {
+ *tmp = g_malloc0(sizeof(**tmp));
+ (*tmp)->value = value[i];
+ tmp = &(*tmp)->next;
+ }
+
+ visit_type_intList(data->ov, &list, NULL, &errp);
+ g_assert(errp == NULL);
+
+ str = string_output_get_string(data->sov);
+ g_assert(str != NULL);
+ g_assert_cmpstr(str, ==,
+ "-10,-7,-2-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
+ g_free(str);
+ while (list) {
+ intList *tmp2;
+ tmp2 = list->next;
+ g_free(list);
+ list = tmp2;
+ }
+}
+
static void test_visitor_out_bool(TestOutputVisitorData *data,
const void *unused)
{
@@ -182,6 +214,8 @@ int main(int argc, char **argv)
&out_visitor_data, test_visitor_out_enum);
output_visitor_test_add("/string-visitor/output/enum-errors",
&out_visitor_data, test_visitor_out_enum_errors);
+ output_visitor_test_add("/string-visitor/output/intList",
+ &out_visitor_data, test_visitor_out_intList);
g_test_run();
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH v5 16/16] qmp: add query-memdev
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (14 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 15/16] qapi: make string output " Hu Tao
@ 2014-06-10 11:15 ` Hu Tao
2014-06-12 7:41 ` [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Michael S. Tsirkin
16 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-10 11:15 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Yasunori Goto
Add qmp command query-memdev to query for information
of memory devices
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
numa.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 40 +++++++++++++++++++++++++++++++
qmp-commands.hx | 38 ++++++++++++++++++++++++++++++
3 files changed, 150 insertions(+)
diff --git a/numa.c b/numa.c
index 8ba5a38..ce9382d 100644
--- a/numa.c
+++ b/numa.c
@@ -31,9 +31,14 @@
#include "qapi-visit.h"
#include "qapi/opts-visitor.h"
#include "qapi/dealloc-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/string-output-visitor.h"
+#include "qapi/string-input-visitor.h"
#include "qapi/qmp/qerror.h"
#include "hw/boards.h"
#include "sysemu/hostmem.h"
+#include "qmp-commands.h"
QemuOptsList qemu_numa_opts = {
.name = "numa",
@@ -280,3 +285,70 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
addr += size;
}
}
+
+MemdevList *qmp_query_memdev(Error **errp)
+{
+ MemdevList *list = NULL, *m;
+ HostMemoryBackend *backend;
+ Error *err = NULL;
+ int i;
+
+ for (i = 0; i < nb_numa_nodes; i++) {
+ backend = numa_info[i].node_memdev;
+
+ m = g_malloc0(sizeof(*m));
+ m->value = g_malloc0(sizeof(*m->value));
+ m->value->size = object_property_get_int(OBJECT(backend), "size",
+ &err);
+ if (err) {
+ goto error;
+ }
+
+ m->value->merge = object_property_get_bool(OBJECT(backend), "merge",
+ &err);
+ if (err) {
+ goto error;
+ }
+
+ m->value->dump = object_property_get_bool(OBJECT(backend), "dump",
+ &err);
+ if (err) {
+ goto error;
+ }
+
+ m->value->prealloc = object_property_get_bool(OBJECT(backend),
+ "prealloc", &err);
+ if (err) {
+ goto error;
+ }
+
+ m->value->policy = object_property_get_enum(OBJECT(backend),
+ "policy",
+ HostMemPolicy_lookup,
+ &err);
+ if (err) {
+ goto error;
+ }
+
+ object_property_get_uint16List(OBJECT(backend), "host-nodes",
+ &m->value->host_nodes, &err);
+ if (err) {
+ goto error;
+ }
+
+ m->next = list;
+ list = m;
+ }
+
+ return list;
+
+error:
+ while (list) {
+ m = list;
+ list = list->next;
+ g_free(m->value);
+ g_free(m);
+ }
+ qerror_report_err(err);
+ return NULL;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 0898c00..c3eafcf 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4779,3 +4779,43 @@
##
{ 'enum': 'HostMemPolicy',
'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
+
+##
+# @Memdev:
+#
+# Information of memory device
+#
+# @size: memory device size
+#
+# @merge: enables or disables memory merge support
+#
+# @dump: includes memory device's memory in a core dump or not
+#
+# @prealloc: enables or disables memory preallocation
+#
+# @host-nodes: host nodes for its memory policy
+#
+# @policy: memory policy of memory device
+#
+# Since: 2.1
+##
+
+{ 'type': 'Memdev',
+ 'data': {
+ 'size': 'size',
+ 'merge': 'bool',
+ 'dump': 'bool',
+ 'prealloc': 'bool',
+ 'host-nodes': ['uint16'],
+ 'policy': 'HostMemPolicy' }}
+
+##
+# @query-memdev:
+#
+# Returns information for all memory devices.
+#
+# Returns: a list of @Memdev.
+#
+# Since: 2.1
+##
+{ 'command': 'query-memdev', 'returns': ['Memdev'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d8aa4ed..bb34cd8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3572,3 +3572,41 @@ Example:
} } ] }
EQMP
+
+ {
+ .name = "query-memdev",
+ .args_type = "",
+ .mhandler.cmd_new = qmp_marshal_input_query_memdev,
+ },
+
+SQMP
+query-memdev
+------------
+
+Show memory devices information.
+
+
+Example (1):
+
+-> { "execute": "query-memdev" }
+<- { "return": [
+ {
+ "size": 536870912,
+ "merge": false,
+ "dump": true,
+ "prealloc": false,
+ "host-nodes": [0, 1],
+ "policy": "bind"
+ },
+ {
+ "size": 536870912,
+ "merge": false,
+ "dump": true,
+ "prealloc": true,
+ "host-nodes": [2, 3],
+ "policy": "preferred"
+ }
+ ]
+ }
+
+EQMP
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH v5 00/16] NUMA series v5
2014-06-10 11:15 [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Hu Tao
` (15 preceding siblings ...)
2014-06-10 11:15 ` [Qemu-devel] [PATCH v5 16/16] qmp: add query-memdev Hu Tao
@ 2014-06-12 7:41 ` Michael S. Tsirkin
2014-06-12 7:53 ` Hu Tao
16 siblings, 1 reply; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-12 7:41 UTC (permalink / raw)
To: Hu Tao
Cc: Eduardo Habkost, qemu-devel, Paolo Bonzini, Igor Mammedov, Yasunori Goto
On Tue, Jun 10, 2014 at 07:15:13PM +0800, Hu Tao wrote:
> note: based on MST's numa tree.
>
> changes are error messages and docs tweaks to address comments to
> v4.
Applied, numa tree applied to the pci tree as well.
Two points I would like to see addressed before it's going upstream:
- please validate and test the pci branch
there have been lots of changes
- please work to drop the SignedRange: for all
current purposes existing Range is sufficient,
instead of a list and sort re-implementation, it is
better to use g_list_insert_sorted
Thanks everyone.
>
> Hu Tao (7):
> backend:hostmem: replace hostmemory with host_memory
> hostmem: separate allocation from UserCreatable complete method
> hostmem: add properties for NUMA memory policy
> tests: fix memory leak in test of string input visitor
> qapi: make string input visitor parse int list
> qapi: make string output visitor parse int list
> qmp: add query-memdev
>
> Paolo Bonzini (8):
> vl: redo -object parsing
> fixup! qmp: improve error reporting for -object and object-add
> pc: pass MachineState to pc_memory_init
> fixup! numa: add -numa node,memdev= option
> hostmem: add file-based HostMemoryBackend
> hostmem: add merge and dump properties
> hostmem: allow preallocation of any memory region
> hostmem: add property to map memory with MAP_SHARED
>
> Wanlong Gao (1):
> fixup! NUMA: check if the total numa memory size is equal to ram_size
>
> backends/Makefile.objs | 1 +
> backends/hostmem-file.c | 134 ++++++++++++++++
> backends/hostmem-ram.c | 7 +-
> backends/hostmem.c | 304 +++++++++++++++++++++++++++++++++++--
> exec.c | 25 ++-
> hw/i386/pc.c | 24 +--
> hw/i386/pc_piix.c | 8 +-
> hw/i386/pc_q35.c | 4 +-
> include/exec/memory.h | 12 ++
> include/exec/ram_addr.h | 4 +-
> include/hw/i386/pc.h | 7 +-
> include/qemu/osdep.h | 10 ++
> include/sysemu/hostmem.h | 8 +
> memory.c | 14 +-
> numa.c | 78 +++++++++-
> qapi-schema.json | 60 ++++++++
> qapi/string-input-visitor.c | 181 +++++++++++++++++++++-
> qapi/string-output-visitor.c | 230 ++++++++++++++++++++++++++--
> qemu-options.hx | 6 +-
> qmp-commands.hx | 38 +++++
> qmp.c | 3 +-
> tests/test-string-input-visitor.c | 39 +++++
> tests/test-string-output-visitor.c | 34 +++++
> vl.c | 65 ++++----
> 24 files changed, 1191 insertions(+), 105 deletions(-)
> create mode 100644 backends/hostmem-file.c
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH v5 00/16] NUMA series v5
2014-06-12 7:41 ` [Qemu-devel] [PATCH v5 00/16] NUMA series v5 Michael S. Tsirkin
@ 2014-06-12 7:53 ` Hu Tao
2014-06-13 8:03 ` Michael S. Tsirkin
0 siblings, 1 reply; 47+ messages in thread
From: Hu Tao @ 2014-06-12 7:53 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eduardo Habkost, qemu-devel, Paolo Bonzini, Igor Mammedov, Yasunori Goto
On Thu, Jun 12, 2014 at 10:41:18AM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 10, 2014 at 07:15:13PM +0800, Hu Tao wrote:
> > note: based on MST's numa tree.
> >
> > changes are error messages and docs tweaks to address comments to
> > v4.
>
> Applied, numa tree applied to the pci tree as well.
> Two points I would like to see addressed before it's going upstream:
> - please validate and test the pci branch
> there have been lots of changes
> - please work to drop the SignedRange: for all
> current purposes existing Range is sufficient,
> instead of a list and sort re-implementation, it is
> better to use g_list_insert_sorted
I see. Thanks for your work!
>
> Thanks everyone.
>
> >
> > Hu Tao (7):
> > backend:hostmem: replace hostmemory with host_memory
> > hostmem: separate allocation from UserCreatable complete method
> > hostmem: add properties for NUMA memory policy
> > tests: fix memory leak in test of string input visitor
> > qapi: make string input visitor parse int list
> > qapi: make string output visitor parse int list
> > qmp: add query-memdev
> >
> > Paolo Bonzini (8):
> > vl: redo -object parsing
> > fixup! qmp: improve error reporting for -object and object-add
> > pc: pass MachineState to pc_memory_init
> > fixup! numa: add -numa node,memdev= option
> > hostmem: add file-based HostMemoryBackend
> > hostmem: add merge and dump properties
> > hostmem: allow preallocation of any memory region
> > hostmem: add property to map memory with MAP_SHARED
> >
> > Wanlong Gao (1):
> > fixup! NUMA: check if the total numa memory size is equal to ram_size
> >
> > backends/Makefile.objs | 1 +
> > backends/hostmem-file.c | 134 ++++++++++++++++
> > backends/hostmem-ram.c | 7 +-
> > backends/hostmem.c | 304 +++++++++++++++++++++++++++++++++++--
> > exec.c | 25 ++-
> > hw/i386/pc.c | 24 +--
> > hw/i386/pc_piix.c | 8 +-
> > hw/i386/pc_q35.c | 4 +-
> > include/exec/memory.h | 12 ++
> > include/exec/ram_addr.h | 4 +-
> > include/hw/i386/pc.h | 7 +-
> > include/qemu/osdep.h | 10 ++
> > include/sysemu/hostmem.h | 8 +
> > memory.c | 14 +-
> > numa.c | 78 +++++++++-
> > qapi-schema.json | 60 ++++++++
> > qapi/string-input-visitor.c | 181 +++++++++++++++++++++-
> > qapi/string-output-visitor.c | 230 ++++++++++++++++++++++++++--
> > qemu-options.hx | 6 +-
> > qmp-commands.hx | 38 +++++
> > qmp.c | 3 +-
> > tests/test-string-input-visitor.c | 39 +++++
> > tests/test-string-output-visitor.c | 34 +++++
> > vl.c | 65 ++++----
> > 24 files changed, 1191 insertions(+), 105 deletions(-)
> > create mode 100644 backends/hostmem-file.c
> >
> > --
> > 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/16] NUMA series v5
2014-06-12 7:53 ` Hu Tao
@ 2014-06-13 8:03 ` Michael S. Tsirkin
2014-06-13 8:18 ` Paolo Bonzini
0 siblings, 1 reply; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-13 8:03 UTC (permalink / raw)
To: Hu Tao
Cc: Eduardo Habkost, qemu-devel, Paolo Bonzini, Igor Mammedov, Yasunori Goto
On Thu, Jun 12, 2014 at 03:53:00PM +0800, Hu Tao wrote:
> On Thu, Jun 12, 2014 at 10:41:18AM +0300, Michael S. Tsirkin wrote:
> > On Tue, Jun 10, 2014 at 07:15:13PM +0800, Hu Tao wrote:
> > > note: based on MST's numa tree.
> > >
> > > changes are error messages and docs tweaks to address comments to
> > > v4.
> >
> > Applied, numa tree applied to the pci tree as well.
> > Two points I would like to see addressed before it's going upstream:
> > - please validate and test the pci branch
> > there have been lots of changes
> > - please work to drop the SignedRange: for all
> > current purposes existing Range is sufficient,
> > instead of a list and sort re-implementation, it is
> > better to use g_list_insert_sorted
>
> I see. Thanks for your work!
Could you confirm you'll be able to fix these by Monday?
I'd like to send the pull request then.
> >
> > Thanks everyone.
> >
> > >
> > > Hu Tao (7):
> > > backend:hostmem: replace hostmemory with host_memory
> > > hostmem: separate allocation from UserCreatable complete method
> > > hostmem: add properties for NUMA memory policy
> > > tests: fix memory leak in test of string input visitor
> > > qapi: make string input visitor parse int list
> > > qapi: make string output visitor parse int list
> > > qmp: add query-memdev
> > >
> > > Paolo Bonzini (8):
> > > vl: redo -object parsing
> > > fixup! qmp: improve error reporting for -object and object-add
> > > pc: pass MachineState to pc_memory_init
> > > fixup! numa: add -numa node,memdev= option
> > > hostmem: add file-based HostMemoryBackend
> > > hostmem: add merge and dump properties
> > > hostmem: allow preallocation of any memory region
> > > hostmem: add property to map memory with MAP_SHARED
> > >
> > > Wanlong Gao (1):
> > > fixup! NUMA: check if the total numa memory size is equal to ram_size
> > >
> > > backends/Makefile.objs | 1 +
> > > backends/hostmem-file.c | 134 ++++++++++++++++
> > > backends/hostmem-ram.c | 7 +-
> > > backends/hostmem.c | 304 +++++++++++++++++++++++++++++++++++--
> > > exec.c | 25 ++-
> > > hw/i386/pc.c | 24 +--
> > > hw/i386/pc_piix.c | 8 +-
> > > hw/i386/pc_q35.c | 4 +-
> > > include/exec/memory.h | 12 ++
> > > include/exec/ram_addr.h | 4 +-
> > > include/hw/i386/pc.h | 7 +-
> > > include/qemu/osdep.h | 10 ++
> > > include/sysemu/hostmem.h | 8 +
> > > memory.c | 14 +-
> > > numa.c | 78 +++++++++-
> > > qapi-schema.json | 60 ++++++++
> > > qapi/string-input-visitor.c | 181 +++++++++++++++++++++-
> > > qapi/string-output-visitor.c | 230 ++++++++++++++++++++++++++--
> > > qemu-options.hx | 6 +-
> > > qmp-commands.hx | 38 +++++
> > > qmp.c | 3 +-
> > > tests/test-string-input-visitor.c | 39 +++++
> > > tests/test-string-output-visitor.c | 34 +++++
> > > vl.c | 65 ++++----
> > > 24 files changed, 1191 insertions(+), 105 deletions(-)
> > > create mode 100644 backends/hostmem-file.c
> > >
> > > --
> > > 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/16] NUMA series v5
2014-06-13 8:03 ` Michael S. Tsirkin
@ 2014-06-13 8:18 ` Paolo Bonzini
2014-06-13 8:46 ` Michael S. Tsirkin
2014-06-13 8:49 ` Hu Tao
0 siblings, 2 replies; 47+ messages in thread
From: Paolo Bonzini @ 2014-06-13 8:18 UTC (permalink / raw)
To: Michael S. Tsirkin, Hu Tao
Cc: Yasunori Goto, qemu-devel, Eduardo Habkost, Igor Mammedov
Il 13/06/2014 10:03, Michael S. Tsirkin ha scritto:
>>> > > Two points I would like to see addressed before it's going upstream:
>>> > > - please validate and test the pci branch
>>> > > there have been lots of changes
>>> > > - please work to drop the SignedRange: for all
>>> > > current purposes existing Range is sufficient,
>>> > > instead of a list and sort re-implementation, it is
>>> > > better to use g_list_insert_sorted
>> >
>> > I see. Thanks for your work!
> Could you confirm you'll be able to fix these by Monday?
> I'd like to send the pull request then.
I think SignedRange can be dealt with after the merge.
Paolo
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/16] NUMA series v5
2014-06-13 8:18 ` Paolo Bonzini
@ 2014-06-13 8:46 ` Michael S. Tsirkin
2014-06-13 8:49 ` Hu Tao
1 sibling, 0 replies; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-13 8:46 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Eduardo Habkost, Hu Tao, qemu-devel, Yasunori Goto, Igor Mammedov
On Fri, Jun 13, 2014 at 10:18:44AM +0200, Paolo Bonzini wrote:
> Il 13/06/2014 10:03, Michael S. Tsirkin ha scritto:
> >>>> > Two points I would like to see addressed before it's going upstream:
> >>>> > - please validate and test the pci branch
> >>>> > there have been lots of changes
> >>>> > - please work to drop the SignedRange: for all
> >>>> > current purposes existing Range is sufficient,
> >>>> > instead of a list and sort re-implementation, it is
> >>>> > better to use g_list_insert_sorted
> >>>
> >>> I see. Thanks for your work!
> >Could you confirm you'll be able to fix these by Monday?
> >I'd like to send the pull request then.
>
> I think SignedRange can be dealt with after the merge.
>
> Paolo
Could be, but let's not go there, cleaning this up isn't hard.
--
MST
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/16] NUMA series v5
2014-06-13 8:18 ` Paolo Bonzini
2014-06-13 8:46 ` Michael S. Tsirkin
@ 2014-06-13 8:49 ` Hu Tao
2014-06-13 8:54 ` Michael S. Tsirkin
1 sibling, 1 reply; 47+ messages in thread
From: Hu Tao @ 2014-06-13 8:49 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Igor Mammedov,
Yasunori Goto
On Fri, Jun 13, 2014 at 10:18:44AM +0200, Paolo Bonzini wrote:
> Il 13/06/2014 10:03, Michael S. Tsirkin ha scritto:
> >>>> > Two points I would like to see addressed before it's going upstream:
> >>>> > - please validate and test the pci branch
> >>>> > there have been lots of changes
> >>>> > - please work to drop the SignedRange: for all
> >>>> > current purposes existing Range is sufficient,
> >>>> > instead of a list and sort re-implementation, it is
> >>>> > better to use g_list_insert_sorted
> >>>
> >>> I see. Thanks for your work!
> >Could you confirm you'll be able to fix these by Monday?
> >I'd like to send the pull request then.
>
> I think SignedRange can be dealt with after the merge.
>
> Paolo
I agree. Using existing Range makes string output visitor don't support
negative values which it does support before SignedRange.
Besides SignedRange, tests show that there are several problems with the
pci tree(core dumps, unexpecting exit of qemu).
Hu
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH v5 00/16] NUMA series v5
2014-06-13 8:49 ` Hu Tao
@ 2014-06-13 8:54 ` Michael S. Tsirkin
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Hu Tao
0 siblings, 1 reply; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-13 8:54 UTC (permalink / raw)
To: Hu Tao
Cc: Eduardo Habkost, Yasunori Goto, qemu-devel, Igor Mammedov, Paolo Bonzini
On Fri, Jun 13, 2014 at 04:49:18PM +0800, Hu Tao wrote:
> On Fri, Jun 13, 2014 at 10:18:44AM +0200, Paolo Bonzini wrote:
> > Il 13/06/2014 10:03, Michael S. Tsirkin ha scritto:
> > >>>> > Two points I would like to see addressed before it's going upstream:
> > >>>> > - please validate and test the pci branch
> > >>>> > there have been lots of changes
> > >>>> > - please work to drop the SignedRange: for all
> > >>>> > current purposes existing Range is sufficient,
> > >>>> > instead of a list and sort re-implementation, it is
> > >>>> > better to use g_list_insert_sorted
> > >>>
> > >>> I see. Thanks for your work!
> > >Could you confirm you'll be able to fix these by Monday?
> > >I'd like to send the pull request then.
> >
> > I think SignedRange can be dealt with after the merge.
> >
> > Paolo
>
> I agree. Using existing Range makes string output visitor don't support
> negative values which it does support before SignedRange.
Are there any users though? I think it's more a bug than a feature.
Also we do want to get rid of rewriting g_list_insert_sorted though, that's
for sure.
> Besides SignedRange, tests show that there are several problems with the
> pci tree(core dumps, unexpecting exit of qemu).
>
> Hu
OK pls share this info, also pls bisect if possible.
Thanks!
--
MST
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree
2014-06-13 8:54 ` Michael S. Tsirkin
@ 2014-06-14 4:48 ` Hu Tao
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 1/4] get rid of signed range Hu Tao
` (4 more replies)
0 siblings, 5 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-14 4:48 UTC (permalink / raw)
To: qemu-devel
Cc: Yasunori Goto, Paolo Bonzini, Igor Mammedov, Michael S. Tsirkin
Michael,
This is fixes for your pci tree.
patch 1 remove signed range as requested.
There are 3 problems in current pci tree, as follows:
1. pc-dimm specified on command line but only -m size (aka not -m size,maxmem,slots)
./x86_64-softmmu/qemu-system-x86_64 -hda
/home/data/libvirt-images/f18.img -smp 2 -object
memory-backend-ram,size=512M,id=ram-node0,prealloc=y,policy=bind,host-nodes=0
-device pc-dimm,id=d0,memdev=ram-node0 -m 640M -qmp
unix:/tmp/m,server,nowait -monitor stdio -enable-kvm
result:
qemu/hw/mem/pc-dimm.c:110: pc_dimm_get_free_addr: Assertion
`address_space_end > address_space_size' failed.
Aborted (core dumped)
patch 2 fixes this.
2. using qemu monitor command object-add to add a memory-backend-ram
object whose's size is too big
./x86_64-softmmu/qemu-system-x86_64 -hda
/home/data/libvirt-images/f18.img -smp 2 -m 512M -qmp
unix:/tmp/m,server,nowait -monitor stdio -enable-kvm
in monitor:
(qemu)object_add memory-backend-ram,size=40960G,id=mem0
result:
qemu just exits with message: Cannot set up guest memory 'mem0': Cannot allocate memory
patch 3 fixes this.
3. specifying a non-existing directory for memory-backend-file
./x86_64-softmmu/qemu-system-x86_64 -hda
/home/data/libvirt-images/f18.img -smp 2 -m 512M,maxmem=1000G,slots=100
-qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object
memory-backend-file,size=512M,id=mem0,mem-path=/nonexistingdir -device
pc-dimm,id=d0,memdev=mem0
result:
/nonexistingdir: No such file or directory
Bad ram offset fffffffffffff000
Aborted (core dumped)
patch 4 fixes this.
please review. Thanks!
Hu Tao (4):
get rid of signed range
check if we have space left for hotplugged memory
exec: don't exit unconditionally if failed to allocate memory
memory-backend-file: error out if failed to allocate memory
backends/hostmem-file.c | 3 +
backends/hostmem-ram.c | 3 +
exec.c | 6 +-
hw/mem/pc-dimm.c | 7 +-
include/qemu/range.h | 144 ++++++++++++-------------------------
qapi/string-input-visitor.c | 116 +++++++++++++++++-------------
qapi/string-output-visitor.c | 97 +++++++++++++------------
tests/test-string-input-visitor.c | 4 +-
tests/test-string-output-visitor.c | 8 +--
9 files changed, 182 insertions(+), 206 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread* [Qemu-devel] [PATCH RFC 1/4] get rid of signed range
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Hu Tao
@ 2014-06-14 4:48 ` Hu Tao
2014-06-15 9:00 ` Michael S. Tsirkin
2014-06-16 15:06 ` Michael S. Tsirkin
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 2/4] check if we have space left for hotplugged memory Hu Tao
` (3 subsequent siblings)
4 siblings, 2 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-14 4:48 UTC (permalink / raw)
To: qemu-devel
Cc: Yasunori Goto, Paolo Bonzini, Igor Mammedov, Michael S. Tsirkin
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
include/qemu/range.h | 144 ++++++++++++-------------------------
qapi/string-input-visitor.c | 116 +++++++++++++++++-------------
qapi/string-output-visitor.c | 97 +++++++++++++------------
tests/test-string-input-visitor.c | 4 +-
tests/test-string-output-visitor.c | 8 +--
5 files changed, 165 insertions(+), 204 deletions(-)
diff --git a/include/qemu/range.h b/include/qemu/range.h
index 8879f8a..cfa021f 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -61,127 +61,75 @@ static inline int ranges_overlap(uint64_t first1, uint64_t len1,
return !(last2 < first1 || last1 < first2);
}
-typedef struct SignedRangeList SignedRangeList;
-
-typedef struct SignedRange {
- int64_t start;
- int64_t length;
-
- QTAILQ_ENTRY(SignedRange) entry;
-} SignedRange;
-
-QTAILQ_HEAD(SignedRangeList, SignedRange);
-
-static inline int64_t s_range_end(int64_t start, int64_t length)
-{
- return start + length - 1;
-}
-
-/* negative length or overflow */
-static inline bool s_range_overflow(int64_t start, int64_t length)
+/* 0,1 can merge with 1,2 but don't overlap */
+static inline bool ranges_can_merge(Range *range1, Range *range2)
{
- return s_range_end(start, length) < start;
+ return !(range1->end < range2->begin || range2->end < range1->begin);
}
-static inline SignedRange *s_range_new(int64_t start, int64_t length)
+static inline int range_merge(Range *range1, Range *range2)
{
- SignedRange *range = NULL;
-
- if (s_range_overflow(start, length)) {
- return NULL;
+ if (ranges_can_merge(range1, range2)) {
+ if (range1->end < range2->end) {
+ range1->end = range2->end;
+ }
+ if (range1->begin > range2->begin) {
+ range1->begin = range2->begin;
+ }
+ return 0;
}
- range = g_malloc0(sizeof(*range));
- range->start = start;
- range->length = length;
-
- return range;
-}
-
-static inline void s_range_free(SignedRange *range)
-{
- g_free(range);
+ return -1;
}
-static inline bool s_range_overlap(int64_t start1, int64_t length1,
- int64_t start2, int64_t length2)
+static inline GList *g_list_insert_sorted_merged(GList *list,
+ gpointer data,
+ GCompareFunc func)
{
- return !((start1 + length1) < start2 || (start2 + length2) < start1);
-}
+ GList *l, *next = NULL;
+ Range *r, *nextr;
-static inline int s_range_join(SignedRange *range,
- int64_t start, int64_t length)
-{
- if (s_range_overflow(start, length)) {
- return -1;
+ if (!list) {
+ list = g_list_insert_sorted(list, data, func);
+ return list;
}
- if (s_range_overlap(range->start, range->length, start, length)) {
- int64_t end = s_range_end(range->start, range->length);
- if (end < s_range_end(start, length)) {
- end = s_range_end(start, length);
+ nextr = data;
+ l = list;
+ while (l && l != next && nextr) {
+ r = l->data;
+ if (ranges_can_merge(r, nextr)) {
+ range_merge(r, nextr);
+ l = g_list_remove_link(l, next);
+ next = g_list_next(l);
+ if (next) {
+ nextr = next->data;
+ } else {
+ nextr = NULL;
+ }
+ } else {
+ l = g_list_next(l);
}
- if (range->start > start) {
- range->start = start;
- }
- range->length = end - range->start + 1;
- return 0;
}
- return -1;
+ if (!l) {
+ list = g_list_insert_sorted(list, data, func);
+ }
+
+ return list;
}
-static inline int s_range_compare(int64_t start1, int64_t length1,
- int64_t start2, int64_t length2)
+static inline gint range_compare(gconstpointer a, gconstpointer b)
{
- if (start1 == start2 && length1 == length2) {
+ Range *ra = (Range *)a, *rb = (Range *)b;
+ if (ra->begin == rb->begin && ra->end == rb->end) {
return 0;
- } else if (s_range_end(start1, length1) <
- s_range_end(start2, length2)) {
+ } else if (range_get_last(ra->begin, ra->end) <
+ range_get_last(rb->begin, rb->end)) {
return -1;
} else {
return 1;
}
}
-/* Add range to list. Keep them sorted, and merge ranges whenever possible */
-static inline bool range_list_add(SignedRangeList *list,
- int64_t start, int64_t length)
-{
- SignedRange *r, *next, *new_range = NULL, *cur = NULL;
-
- if (s_range_overflow(start, length)) {
- return false;
- }
-
- QTAILQ_FOREACH_SAFE(r, list, entry, next) {
- if (s_range_overlap(r->start, r->length, start, length)) {
- s_range_join(r, start, length);
- break;
- } else if (s_range_compare(start, length, r->start, r->length) < 0) {
- cur = r;
- break;
- }
- }
-
- if (!r) {
- new_range = s_range_new(start, length);
- QTAILQ_INSERT_TAIL(list, new_range, entry);
- } else if (cur) {
- new_range = s_range_new(start, length);
- QTAILQ_INSERT_BEFORE(cur, new_range, entry);
- } else {
- SignedRange *next = QTAILQ_NEXT(r, entry);
- while (next && s_range_overlap(r->start, r->length,
- next->start, next->length)) {
- s_range_join(r, next->start, next->length);
- QTAILQ_REMOVE(list, next, entry);
- s_range_free(next);
- next = QTAILQ_NEXT(r, entry);
- }
- }
-
- return true;
-}
-
#endif
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 85ac6a1..0b2490b 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -25,8 +25,8 @@ struct StringInputVisitor
bool head;
- SignedRangeList *ranges;
- SignedRange *cur_range;
+ GList *ranges;
+ GList *cur_range;
int64_t cur;
const char *string;
@@ -36,24 +36,25 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
{
char *str = (char *) siv->string;
long long start, end;
- SignedRange *r, *next;
+ Range *cur;
char *endptr;
if (siv->ranges) {
return;
}
- siv->ranges = g_malloc0(sizeof(*siv->ranges));
- QTAILQ_INIT(siv->ranges);
errno = 0;
do {
start = strtoll(str, &endptr, 0);
if (errno == 0 && endptr > str && INT64_MIN <= start &&
start <= INT64_MAX) {
if (*endptr == '\0') {
- if (!range_list_add(siv->ranges, start, 1)) {
- goto error;
- }
+ cur = g_malloc0(sizeof(*cur));
+ cur->begin = start;
+ cur->end = start + 1;
+ siv->ranges = g_list_insert_sorted_merged(siv->ranges, cur,
+ range_compare);
+ cur = NULL;
str = NULL;
} else if (*endptr == '-') {
str = endptr + 1;
@@ -63,17 +64,25 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
(start > INT64_MAX - 65536 ||
end < start + 65536)) {
if (*endptr == '\0') {
- if (!range_list_add(siv->ranges, start,
- end - start + 1)) {
- goto error;
- }
+ cur = g_malloc0(sizeof(*cur));
+ cur->begin = start;
+ cur->end = end + 1;
+ siv->ranges =
+ g_list_insert_sorted_merged(siv->ranges,
+ cur,
+ range_compare);
+ cur = NULL;
str = NULL;
} else if (*endptr == ',') {
str = endptr + 1;
- if (!range_list_add(siv->ranges, start,
- end - start + 1)) {
- goto error;
- }
+ cur = g_malloc0(sizeof(*cur));
+ cur->begin = start;
+ cur->end = end + 1;
+ siv->ranges =
+ g_list_insert_sorted_merged(siv->ranges,
+ cur,
+ range_compare);
+ cur = NULL;
} else {
goto error;
}
@@ -82,9 +91,13 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
}
} else if (*endptr == ',') {
str = endptr + 1;
- if (!range_list_add(siv->ranges, start, 1)) {
- goto error;
- }
+ cur = g_malloc0(sizeof(*cur));
+ cur->begin = start;
+ cur->end = start + 1;
+ siv->ranges = g_list_insert_sorted_merged(siv->ranges,
+ cur,
+ range_compare);
+ cur = NULL;
} else {
goto error;
}
@@ -95,14 +108,8 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
return;
error:
- if (siv->ranges) {
- QTAILQ_FOREACH_SAFE(r, siv->ranges, entry, next) {
- QTAILQ_REMOVE(siv->ranges, r, entry);
- g_free(r);
- }
- g_free(siv->ranges);
- siv->ranges = NULL;
- }
+ g_list_free_full(siv->ranges, g_free);
+ assert(siv->ranges == NULL);
}
static void
@@ -112,10 +119,11 @@ start_list(Visitor *v, const char *name, Error **errp)
parse_str(siv, errp);
- if (siv->ranges) {
- siv->cur_range = QTAILQ_FIRST(siv->ranges);
- if (siv->cur_range) {
- siv->cur = siv->cur_range->start;
+ siv->cur_range = g_list_first(siv->ranges);
+ if (siv->cur_range) {
+ Range *r = siv->cur_range->data;
+ if (r) {
+ siv->cur = r->begin;
}
}
}
@@ -125,19 +133,27 @@ next_list(Visitor *v, GenericList **list, Error **errp)
{
StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
GenericList **link;
+ Range *r;
if (!siv->ranges || !siv->cur_range) {
return NULL;
}
- if (siv->cur < siv->cur_range->start ||
- siv->cur >= (siv->cur_range->start + siv->cur_range->length)) {
- siv->cur_range = QTAILQ_NEXT(siv->cur_range, entry);
- if (siv->cur_range) {
- siv->cur = siv->cur_range->start;
- } else {
+ r = siv->cur_range->data;
+ if (!r) {
+ return NULL;
+ }
+
+ if (siv->cur < r->begin || siv->cur >= r->end) {
+ siv->cur_range = g_list_next(siv->cur_range);
+ if (!siv->cur_range) {
return NULL;
}
+ r = siv->cur_range->data;
+ if (!r) {
+ return NULL;
+ }
+ siv->cur = r->begin;
}
if (siv->head) {
@@ -176,12 +192,19 @@ static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
}
if (!siv->cur_range) {
- siv->cur_range = QTAILQ_FIRST(siv->ranges);
- if (siv->cur_range) {
- siv->cur = siv->cur_range->start;
- } else {
+ Range *r;
+
+ siv->cur_range = g_list_first(siv->ranges);
+ if (!siv->cur_range) {
+ goto error;
+ }
+
+ r = siv->cur_range->data;
+ if (!r) {
goto error;
}
+
+ siv->cur = r->begin;
}
*obj = siv->cur;
@@ -291,16 +314,7 @@ Visitor *string_input_get_visitor(StringInputVisitor *v)
void string_input_visitor_cleanup(StringInputVisitor *v)
{
- SignedRange *r, *next;
-
- if (v->ranges) {
- QTAILQ_FOREACH_SAFE(r, v->ranges, entry, next) {
- QTAILQ_REMOVE(v->ranges, r, entry);
- g_free(r);
- }
- g_free(v->ranges);
- }
-
+ g_list_free_full(v->ranges, g_free);
g_free(v);
}
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index ccebc7a..1c0834a 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -64,7 +64,7 @@ struct StringOutputVisitor
int64_t s;
uint64_t u;
} range_start, range_end;
- SignedRangeList *ranges;
+ GList *ranges;
};
static void string_output_set(StringOutputVisitor *sov, char *string)
@@ -78,25 +78,50 @@ static void string_output_set(StringOutputVisitor *sov, char *string)
static void string_output_append(StringOutputVisitor *sov, int64_t a)
{
- range_list_add(sov->ranges, a, 1);
+ Range *r = g_malloc0(sizeof(*r));
+ r->begin = a;
+ r->end = a + 1;
+ sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
}
static void string_output_append_range(StringOutputVisitor *sov,
int64_t s, int64_t e)
{
- range_list_add(sov->ranges, s, e);
+ Range *r = g_malloc0(sizeof(*r));
+ r->begin = s;
+ r->end = e + 1;
+ sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
+}
+
+static void format_string(StringOutputVisitor *sov, Range *r, bool next,
+ bool human)
+{
+ if (r->end - r->begin > 1) {
+ if (human) {
+ g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
+ r->begin, r->end - 1);
+
+ } else {
+ g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
+ r->begin, r->end - 1);
+ }
+ } else {
+ if (human) {
+ g_string_append_printf(sov->string, "%" PRIx64, r->begin);
+ } else {
+ g_string_append_printf(sov->string, "%" PRId64, r->begin);
+ }
+ }
+ if (next) {
+ g_string_append(sov->string, ",");
+ }
}
static void print_type_int(Visitor *v, int64_t *obj, const char *name,
Error **errp)
{
StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
- SignedRange *r;
-
- if (!sov->ranges) {
- sov->ranges = g_malloc0(sizeof(*sov->ranges));
- QTAILQ_INIT(sov->ranges);
- }
+ GList *l;
switch (sov->list_mode) {
case LM_NONE:
@@ -118,8 +143,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
} else {
assert(sov->range_start.s < sov->range_end.s);
string_output_append_range(sov, sov->range_start.s,
- sov->range_end.s -
- sov->range_start.s + 1);
+ sov->range_end.s);
}
sov->range_start.s = *obj;
@@ -132,8 +156,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
sov->range_end.s++;
assert(sov->range_start.s < sov->range_end.s);
string_output_append_range(sov, sov->range_start.s,
- sov->range_end.s -
- sov->range_start.s + 1);
+ sov->range_end.s);
} else {
if (sov->range_start.s == sov->range_end.s) {
string_output_append(sov, sov->range_end.s);
@@ -141,8 +164,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
assert(sov->range_start.s < sov->range_end.s);
string_output_append_range(sov, sov->range_start.s,
- sov->range_end.s -
- sov->range_start.s + 1);
+ sov->range_end.s);
}
string_output_append(sov, *obj);
}
@@ -152,34 +174,20 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
abort();
}
- QTAILQ_FOREACH(r, sov->ranges, entry) {
- if (r->length > 1) {
- g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
- r->start,
- s_range_end(r->start, r->length));
- } else {
- g_string_append_printf(sov->string, "%" PRId64,
- r->start);
- }
- if (r->entry.tqe_next) {
- g_string_append(sov->string, ",");
- }
+ l = sov->ranges;
+ while (l) {
+ Range *r = l->data;
+ format_string(sov, r, l->next != NULL, false);
+ l = l->next;
}
if (sov->human) {
+ l = sov->ranges;
g_string_append(sov->string, " (");
- QTAILQ_FOREACH(r, sov->ranges, entry) {
- if (r->length > 1) {
- g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
- r->start,
- s_range_end(r->start, r->length));
- } else {
- g_string_append_printf(sov->string, "%" PRIx64,
- r->start);
- }
- if (r->entry.tqe_next) {
- g_string_append(sov->string, ",");
- }
+ while (l) {
+ Range *r = l->data;
+ format_string(sov, r, l->next != NULL, false);
+ l = l->next;
}
g_string_append(sov->string, ")");
}
@@ -310,20 +318,11 @@ Visitor *string_output_get_visitor(StringOutputVisitor *sov)
void string_output_visitor_cleanup(StringOutputVisitor *sov)
{
- SignedRange *r, *next;
-
if (sov->string) {
g_string_free(sov->string, true);
}
- if (sov->ranges) {
- QTAILQ_FOREACH_SAFE(r, sov->ranges, entry, next) {
- QTAILQ_REMOVE(sov->ranges, r, entry);
- s_range_free(r);
- }
- g_free(sov->ranges);
- }
-
+ g_list_free_full(sov->ranges, g_free);
g_free(sov);
}
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index b08a7db..b01e2f2 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -67,13 +67,13 @@ static void test_visitor_in_int(TestInputVisitorData *data,
static void test_visitor_in_intList(TestInputVisitorData *data,
const void *unused)
{
- int64_t value[] = {-2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
+ int64_t value[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
int16List *res = NULL, *tmp;
Error *errp = NULL;
Visitor *v;
int i = 0;
- v = visitor_input_test_init(data, "1,2,-2-1,2-4,20,5-9,1-8");
+ v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
visit_type_int16List(v, &res, NULL, &errp);
g_assert(errp == NULL);
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index d2ad580..28e7359 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -44,7 +44,7 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
static void test_visitor_out_int(TestOutputVisitorData *data,
const void *unused)
{
- int64_t value = -42;
+ int64_t value = 42;
Error *err = NULL;
char *str;
@@ -53,14 +53,14 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
str = string_output_get_string(data->sov);
g_assert(str != NULL);
- g_assert_cmpstr(str, ==, "-42");
+ g_assert_cmpstr(str, ==, "42");
g_free(str);
}
static void test_visitor_out_intList(TestOutputVisitorData *data,
const void *unused)
{
- int64_t value[] = {-10, -7, -2, -1, 0, 1, 9, 10, 16, 15, 14,
+ int64_t value[] = {0, 1, 9, 10, 16, 15, 14,
3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};
intList *list = NULL, **tmp = &list;
int i;
@@ -79,7 +79,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
str = string_output_get_string(data->sov);
g_assert(str != NULL);
g_assert_cmpstr(str, ==,
- "-10,-7,-2-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
+ "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
g_free(str);
while (list) {
intList *tmp2;
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/4] get rid of signed range
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 1/4] get rid of signed range Hu Tao
@ 2014-06-15 9:00 ` Michael S. Tsirkin
2014-06-16 9:47 ` Hu Tao
2014-06-16 15:06 ` Michael S. Tsirkin
1 sibling, 1 reply; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-15 9:00 UTC (permalink / raw)
To: Hu Tao; +Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Sat, Jun 14, 2014 at 12:48:56PM +0800, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
This also fixed make check failures that I was seeing on 32 bit systems.
Applied, but I split this patch up and applied as fixup
to the original.
In the future you can request such fixes by making
subject be "fixup! <original subject>"
This is possible as long as tree is not merged.
> ---
> include/qemu/range.h | 144 ++++++++++++-------------------------
> qapi/string-input-visitor.c | 116 +++++++++++++++++-------------
> qapi/string-output-visitor.c | 97 +++++++++++++------------
> tests/test-string-input-visitor.c | 4 +-
> tests/test-string-output-visitor.c | 8 +--
> 5 files changed, 165 insertions(+), 204 deletions(-)
>
> diff --git a/include/qemu/range.h b/include/qemu/range.h
> index 8879f8a..cfa021f 100644
> --- a/include/qemu/range.h
> +++ b/include/qemu/range.h
> @@ -61,127 +61,75 @@ static inline int ranges_overlap(uint64_t first1, uint64_t len1,
> return !(last2 < first1 || last1 < first2);
> }
>
> -typedef struct SignedRangeList SignedRangeList;
> -
> -typedef struct SignedRange {
> - int64_t start;
> - int64_t length;
> -
> - QTAILQ_ENTRY(SignedRange) entry;
> -} SignedRange;
> -
> -QTAILQ_HEAD(SignedRangeList, SignedRange);
> -
> -static inline int64_t s_range_end(int64_t start, int64_t length)
> -{
> - return start + length - 1;
> -}
> -
> -/* negative length or overflow */
> -static inline bool s_range_overflow(int64_t start, int64_t length)
> +/* 0,1 can merge with 1,2 but don't overlap */
> +static inline bool ranges_can_merge(Range *range1, Range *range2)
> {
> - return s_range_end(start, length) < start;
> + return !(range1->end < range2->begin || range2->end < range1->begin);
> }
>
> -static inline SignedRange *s_range_new(int64_t start, int64_t length)
> +static inline int range_merge(Range *range1, Range *range2)
> {
> - SignedRange *range = NULL;
> -
> - if (s_range_overflow(start, length)) {
> - return NULL;
> + if (ranges_can_merge(range1, range2)) {
> + if (range1->end < range2->end) {
> + range1->end = range2->end;
> + }
> + if (range1->begin > range2->begin) {
> + range1->begin = range2->begin;
> + }
> + return 0;
> }
>
> - range = g_malloc0(sizeof(*range));
> - range->start = start;
> - range->length = length;
> -
> - return range;
> -}
> -
> -static inline void s_range_free(SignedRange *range)
> -{
> - g_free(range);
> + return -1;
> }
>
> -static inline bool s_range_overlap(int64_t start1, int64_t length1,
> - int64_t start2, int64_t length2)
> +static inline GList *g_list_insert_sorted_merged(GList *list,
> + gpointer data,
> + GCompareFunc func)
> {
> - return !((start1 + length1) < start2 || (start2 + length2) < start1);
> -}
> + GList *l, *next = NULL;
> + Range *r, *nextr;
>
> -static inline int s_range_join(SignedRange *range,
> - int64_t start, int64_t length)
> -{
> - if (s_range_overflow(start, length)) {
> - return -1;
> + if (!list) {
> + list = g_list_insert_sorted(list, data, func);
> + return list;
> }
>
> - if (s_range_overlap(range->start, range->length, start, length)) {
> - int64_t end = s_range_end(range->start, range->length);
> - if (end < s_range_end(start, length)) {
> - end = s_range_end(start, length);
> + nextr = data;
> + l = list;
> + while (l && l != next && nextr) {
> + r = l->data;
> + if (ranges_can_merge(r, nextr)) {
> + range_merge(r, nextr);
> + l = g_list_remove_link(l, next);
> + next = g_list_next(l);
> + if (next) {
> + nextr = next->data;
> + } else {
> + nextr = NULL;
> + }
> + } else {
> + l = g_list_next(l);
> }
> - if (range->start > start) {
> - range->start = start;
> - }
> - range->length = end - range->start + 1;
> - return 0;
> }
>
> - return -1;
> + if (!l) {
> + list = g_list_insert_sorted(list, data, func);
> + }
> +
> + return list;
> }
>
> -static inline int s_range_compare(int64_t start1, int64_t length1,
> - int64_t start2, int64_t length2)
> +static inline gint range_compare(gconstpointer a, gconstpointer b)
> {
> - if (start1 == start2 && length1 == length2) {
> + Range *ra = (Range *)a, *rb = (Range *)b;
> + if (ra->begin == rb->begin && ra->end == rb->end) {
> return 0;
> - } else if (s_range_end(start1, length1) <
> - s_range_end(start2, length2)) {
> + } else if (range_get_last(ra->begin, ra->end) <
> + range_get_last(rb->begin, rb->end)) {
> return -1;
> } else {
> return 1;
> }
> }
>
> -/* Add range to list. Keep them sorted, and merge ranges whenever possible */
> -static inline bool range_list_add(SignedRangeList *list,
> - int64_t start, int64_t length)
> -{
> - SignedRange *r, *next, *new_range = NULL, *cur = NULL;
> -
> - if (s_range_overflow(start, length)) {
> - return false;
> - }
> -
> - QTAILQ_FOREACH_SAFE(r, list, entry, next) {
> - if (s_range_overlap(r->start, r->length, start, length)) {
> - s_range_join(r, start, length);
> - break;
> - } else if (s_range_compare(start, length, r->start, r->length) < 0) {
> - cur = r;
> - break;
> - }
> - }
> -
> - if (!r) {
> - new_range = s_range_new(start, length);
> - QTAILQ_INSERT_TAIL(list, new_range, entry);
> - } else if (cur) {
> - new_range = s_range_new(start, length);
> - QTAILQ_INSERT_BEFORE(cur, new_range, entry);
> - } else {
> - SignedRange *next = QTAILQ_NEXT(r, entry);
> - while (next && s_range_overlap(r->start, r->length,
> - next->start, next->length)) {
> - s_range_join(r, next->start, next->length);
> - QTAILQ_REMOVE(list, next, entry);
> - s_range_free(next);
> - next = QTAILQ_NEXT(r, entry);
> - }
> - }
> -
> - return true;
> -}
> -
> #endif
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 85ac6a1..0b2490b 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -25,8 +25,8 @@ struct StringInputVisitor
>
> bool head;
>
> - SignedRangeList *ranges;
> - SignedRange *cur_range;
> + GList *ranges;
> + GList *cur_range;
> int64_t cur;
>
> const char *string;
> @@ -36,24 +36,25 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> {
> char *str = (char *) siv->string;
> long long start, end;
> - SignedRange *r, *next;
> + Range *cur;
> char *endptr;
>
> if (siv->ranges) {
> return;
> }
>
> - siv->ranges = g_malloc0(sizeof(*siv->ranges));
> - QTAILQ_INIT(siv->ranges);
> errno = 0;
> do {
> start = strtoll(str, &endptr, 0);
> if (errno == 0 && endptr > str && INT64_MIN <= start &&
> start <= INT64_MAX) {
> if (*endptr == '\0') {
> - if (!range_list_add(siv->ranges, start, 1)) {
> - goto error;
> - }
> + cur = g_malloc0(sizeof(*cur));
> + cur->begin = start;
> + cur->end = start + 1;
> + siv->ranges = g_list_insert_sorted_merged(siv->ranges, cur,
> + range_compare);
> + cur = NULL;
> str = NULL;
> } else if (*endptr == '-') {
> str = endptr + 1;
> @@ -63,17 +64,25 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> (start > INT64_MAX - 65536 ||
> end < start + 65536)) {
> if (*endptr == '\0') {
> - if (!range_list_add(siv->ranges, start,
> - end - start + 1)) {
> - goto error;
> - }
> + cur = g_malloc0(sizeof(*cur));
> + cur->begin = start;
> + cur->end = end + 1;
> + siv->ranges =
> + g_list_insert_sorted_merged(siv->ranges,
> + cur,
> + range_compare);
> + cur = NULL;
> str = NULL;
> } else if (*endptr == ',') {
> str = endptr + 1;
> - if (!range_list_add(siv->ranges, start,
> - end - start + 1)) {
> - goto error;
> - }
> + cur = g_malloc0(sizeof(*cur));
> + cur->begin = start;
> + cur->end = end + 1;
> + siv->ranges =
> + g_list_insert_sorted_merged(siv->ranges,
> + cur,
> + range_compare);
> + cur = NULL;
> } else {
> goto error;
> }
> @@ -82,9 +91,13 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> }
> } else if (*endptr == ',') {
> str = endptr + 1;
> - if (!range_list_add(siv->ranges, start, 1)) {
> - goto error;
> - }
> + cur = g_malloc0(sizeof(*cur));
> + cur->begin = start;
> + cur->end = start + 1;
> + siv->ranges = g_list_insert_sorted_merged(siv->ranges,
> + cur,
> + range_compare);
> + cur = NULL;
> } else {
> goto error;
> }
> @@ -95,14 +108,8 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
>
> return;
> error:
> - if (siv->ranges) {
> - QTAILQ_FOREACH_SAFE(r, siv->ranges, entry, next) {
> - QTAILQ_REMOVE(siv->ranges, r, entry);
> - g_free(r);
> - }
> - g_free(siv->ranges);
> - siv->ranges = NULL;
> - }
> + g_list_free_full(siv->ranges, g_free);
> + assert(siv->ranges == NULL);
> }
>
> static void
> @@ -112,10 +119,11 @@ start_list(Visitor *v, const char *name, Error **errp)
>
> parse_str(siv, errp);
>
> - if (siv->ranges) {
> - siv->cur_range = QTAILQ_FIRST(siv->ranges);
> - if (siv->cur_range) {
> - siv->cur = siv->cur_range->start;
> + siv->cur_range = g_list_first(siv->ranges);
> + if (siv->cur_range) {
> + Range *r = siv->cur_range->data;
> + if (r) {
> + siv->cur = r->begin;
> }
> }
> }
> @@ -125,19 +133,27 @@ next_list(Visitor *v, GenericList **list, Error **errp)
> {
> StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> GenericList **link;
> + Range *r;
>
> if (!siv->ranges || !siv->cur_range) {
> return NULL;
> }
>
> - if (siv->cur < siv->cur_range->start ||
> - siv->cur >= (siv->cur_range->start + siv->cur_range->length)) {
> - siv->cur_range = QTAILQ_NEXT(siv->cur_range, entry);
> - if (siv->cur_range) {
> - siv->cur = siv->cur_range->start;
> - } else {
> + r = siv->cur_range->data;
> + if (!r) {
> + return NULL;
> + }
> +
> + if (siv->cur < r->begin || siv->cur >= r->end) {
> + siv->cur_range = g_list_next(siv->cur_range);
> + if (!siv->cur_range) {
> return NULL;
> }
> + r = siv->cur_range->data;
> + if (!r) {
> + return NULL;
> + }
> + siv->cur = r->begin;
> }
>
> if (siv->head) {
> @@ -176,12 +192,19 @@ static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
> }
>
> if (!siv->cur_range) {
> - siv->cur_range = QTAILQ_FIRST(siv->ranges);
> - if (siv->cur_range) {
> - siv->cur = siv->cur_range->start;
> - } else {
> + Range *r;
> +
> + siv->cur_range = g_list_first(siv->ranges);
> + if (!siv->cur_range) {
> + goto error;
> + }
> +
> + r = siv->cur_range->data;
> + if (!r) {
> goto error;
> }
> +
> + siv->cur = r->begin;
> }
>
> *obj = siv->cur;
> @@ -291,16 +314,7 @@ Visitor *string_input_get_visitor(StringInputVisitor *v)
>
> void string_input_visitor_cleanup(StringInputVisitor *v)
> {
> - SignedRange *r, *next;
> -
> - if (v->ranges) {
> - QTAILQ_FOREACH_SAFE(r, v->ranges, entry, next) {
> - QTAILQ_REMOVE(v->ranges, r, entry);
> - g_free(r);
> - }
> - g_free(v->ranges);
> - }
> -
> + g_list_free_full(v->ranges, g_free);
> g_free(v);
> }
>
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index ccebc7a..1c0834a 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -64,7 +64,7 @@ struct StringOutputVisitor
> int64_t s;
> uint64_t u;
> } range_start, range_end;
> - SignedRangeList *ranges;
> + GList *ranges;
> };
>
> static void string_output_set(StringOutputVisitor *sov, char *string)
> @@ -78,25 +78,50 @@ static void string_output_set(StringOutputVisitor *sov, char *string)
>
> static void string_output_append(StringOutputVisitor *sov, int64_t a)
> {
> - range_list_add(sov->ranges, a, 1);
> + Range *r = g_malloc0(sizeof(*r));
> + r->begin = a;
> + r->end = a + 1;
> + sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
> }
>
> static void string_output_append_range(StringOutputVisitor *sov,
> int64_t s, int64_t e)
> {
> - range_list_add(sov->ranges, s, e);
> + Range *r = g_malloc0(sizeof(*r));
> + r->begin = s;
> + r->end = e + 1;
> + sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
> +}
> +
> +static void format_string(StringOutputVisitor *sov, Range *r, bool next,
> + bool human)
> +{
> + if (r->end - r->begin > 1) {
> + if (human) {
> + g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
> + r->begin, r->end - 1);
> +
> + } else {
> + g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
> + r->begin, r->end - 1);
> + }
> + } else {
> + if (human) {
> + g_string_append_printf(sov->string, "%" PRIx64, r->begin);
> + } else {
> + g_string_append_printf(sov->string, "%" PRId64, r->begin);
> + }
> + }
> + if (next) {
> + g_string_append(sov->string, ",");
> + }
> }
>
> static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> Error **errp)
> {
> StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> - SignedRange *r;
> -
> - if (!sov->ranges) {
> - sov->ranges = g_malloc0(sizeof(*sov->ranges));
> - QTAILQ_INIT(sov->ranges);
> - }
> + GList *l;
>
> switch (sov->list_mode) {
> case LM_NONE:
> @@ -118,8 +143,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> } else {
> assert(sov->range_start.s < sov->range_end.s);
> string_output_append_range(sov, sov->range_start.s,
> - sov->range_end.s -
> - sov->range_start.s + 1);
> + sov->range_end.s);
> }
>
> sov->range_start.s = *obj;
> @@ -132,8 +156,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> sov->range_end.s++;
> assert(sov->range_start.s < sov->range_end.s);
> string_output_append_range(sov, sov->range_start.s,
> - sov->range_end.s -
> - sov->range_start.s + 1);
> + sov->range_end.s);
> } else {
> if (sov->range_start.s == sov->range_end.s) {
> string_output_append(sov, sov->range_end.s);
> @@ -141,8 +164,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> assert(sov->range_start.s < sov->range_end.s);
>
> string_output_append_range(sov, sov->range_start.s,
> - sov->range_end.s -
> - sov->range_start.s + 1);
> + sov->range_end.s);
> }
> string_output_append(sov, *obj);
> }
> @@ -152,34 +174,20 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> abort();
> }
>
> - QTAILQ_FOREACH(r, sov->ranges, entry) {
> - if (r->length > 1) {
> - g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
> - r->start,
> - s_range_end(r->start, r->length));
> - } else {
> - g_string_append_printf(sov->string, "%" PRId64,
> - r->start);
> - }
> - if (r->entry.tqe_next) {
> - g_string_append(sov->string, ",");
> - }
> + l = sov->ranges;
> + while (l) {
> + Range *r = l->data;
> + format_string(sov, r, l->next != NULL, false);
> + l = l->next;
> }
>
> if (sov->human) {
> + l = sov->ranges;
> g_string_append(sov->string, " (");
> - QTAILQ_FOREACH(r, sov->ranges, entry) {
> - if (r->length > 1) {
> - g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
> - r->start,
> - s_range_end(r->start, r->length));
> - } else {
> - g_string_append_printf(sov->string, "%" PRIx64,
> - r->start);
> - }
> - if (r->entry.tqe_next) {
> - g_string_append(sov->string, ",");
> - }
> + while (l) {
> + Range *r = l->data;
> + format_string(sov, r, l->next != NULL, false);
> + l = l->next;
> }
> g_string_append(sov->string, ")");
> }
> @@ -310,20 +318,11 @@ Visitor *string_output_get_visitor(StringOutputVisitor *sov)
>
> void string_output_visitor_cleanup(StringOutputVisitor *sov)
> {
> - SignedRange *r, *next;
> -
> if (sov->string) {
> g_string_free(sov->string, true);
> }
>
> - if (sov->ranges) {
> - QTAILQ_FOREACH_SAFE(r, sov->ranges, entry, next) {
> - QTAILQ_REMOVE(sov->ranges, r, entry);
> - s_range_free(r);
> - }
> - g_free(sov->ranges);
> - }
> -
> + g_list_free_full(sov->ranges, g_free);
> g_free(sov);
> }
>
> diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
> index b08a7db..b01e2f2 100644
> --- a/tests/test-string-input-visitor.c
> +++ b/tests/test-string-input-visitor.c
> @@ -67,13 +67,13 @@ static void test_visitor_in_int(TestInputVisitorData *data,
> static void test_visitor_in_intList(TestInputVisitorData *data,
> const void *unused)
> {
> - int64_t value[] = {-2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
> + int64_t value[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
> int16List *res = NULL, *tmp;
> Error *errp = NULL;
> Visitor *v;
> int i = 0;
>
> - v = visitor_input_test_init(data, "1,2,-2-1,2-4,20,5-9,1-8");
> + v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
>
> visit_type_int16List(v, &res, NULL, &errp);
> g_assert(errp == NULL);
> diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
> index d2ad580..28e7359 100644
> --- a/tests/test-string-output-visitor.c
> +++ b/tests/test-string-output-visitor.c
> @@ -44,7 +44,7 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
> static void test_visitor_out_int(TestOutputVisitorData *data,
> const void *unused)
> {
> - int64_t value = -42;
> + int64_t value = 42;
> Error *err = NULL;
> char *str;
>
> @@ -53,14 +53,14 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
>
> str = string_output_get_string(data->sov);
> g_assert(str != NULL);
> - g_assert_cmpstr(str, ==, "-42");
> + g_assert_cmpstr(str, ==, "42");
> g_free(str);
> }
>
> static void test_visitor_out_intList(TestOutputVisitorData *data,
> const void *unused)
> {
> - int64_t value[] = {-10, -7, -2, -1, 0, 1, 9, 10, 16, 15, 14,
> + int64_t value[] = {0, 1, 9, 10, 16, 15, 14,
> 3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};
> intList *list = NULL, **tmp = &list;
> int i;
> @@ -79,7 +79,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
> str = string_output_get_string(data->sov);
> g_assert(str != NULL);
> g_assert_cmpstr(str, ==,
> - "-10,-7,-2-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
> + "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
> g_free(str);
> while (list) {
> intList *tmp2;
> --
> 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 1/4] get rid of signed range
2014-06-15 9:00 ` Michael S. Tsirkin
@ 2014-06-16 9:47 ` Hu Tao
0 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-16 9:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Sun, Jun 15, 2014 at 12:00:55PM +0300, Michael S. Tsirkin wrote:
> On Sat, Jun 14, 2014 at 12:48:56PM +0800, Hu Tao wrote:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>
> This also fixed make check failures that I was seeing on 32 bit systems.
> Applied, but I split this patch up and applied as fixup
> to the original.
> In the future you can request such fixes by making
> subject be "fixup! <original subject>"
> This is possible as long as tree is not merged.
Thanks!
>
> > ---
> > include/qemu/range.h | 144 ++++++++++++-------------------------
> > qapi/string-input-visitor.c | 116 +++++++++++++++++-------------
> > qapi/string-output-visitor.c | 97 +++++++++++++------------
> > tests/test-string-input-visitor.c | 4 +-
> > tests/test-string-output-visitor.c | 8 +--
> > 5 files changed, 165 insertions(+), 204 deletions(-)
> >
> > diff --git a/include/qemu/range.h b/include/qemu/range.h
> > index 8879f8a..cfa021f 100644
> > --- a/include/qemu/range.h
> > +++ b/include/qemu/range.h
> > @@ -61,127 +61,75 @@ static inline int ranges_overlap(uint64_t first1, uint64_t len1,
> > return !(last2 < first1 || last1 < first2);
> > }
> >
> > -typedef struct SignedRangeList SignedRangeList;
> > -
> > -typedef struct SignedRange {
> > - int64_t start;
> > - int64_t length;
> > -
> > - QTAILQ_ENTRY(SignedRange) entry;
> > -} SignedRange;
> > -
> > -QTAILQ_HEAD(SignedRangeList, SignedRange);
> > -
> > -static inline int64_t s_range_end(int64_t start, int64_t length)
> > -{
> > - return start + length - 1;
> > -}
> > -
> > -/* negative length or overflow */
> > -static inline bool s_range_overflow(int64_t start, int64_t length)
> > +/* 0,1 can merge with 1,2 but don't overlap */
> > +static inline bool ranges_can_merge(Range *range1, Range *range2)
> > {
> > - return s_range_end(start, length) < start;
> > + return !(range1->end < range2->begin || range2->end < range1->begin);
> > }
> >
> > -static inline SignedRange *s_range_new(int64_t start, int64_t length)
> > +static inline int range_merge(Range *range1, Range *range2)
> > {
> > - SignedRange *range = NULL;
> > -
> > - if (s_range_overflow(start, length)) {
> > - return NULL;
> > + if (ranges_can_merge(range1, range2)) {
> > + if (range1->end < range2->end) {
> > + range1->end = range2->end;
> > + }
> > + if (range1->begin > range2->begin) {
> > + range1->begin = range2->begin;
> > + }
> > + return 0;
> > }
> >
> > - range = g_malloc0(sizeof(*range));
> > - range->start = start;
> > - range->length = length;
> > -
> > - return range;
> > -}
> > -
> > -static inline void s_range_free(SignedRange *range)
> > -{
> > - g_free(range);
> > + return -1;
> > }
> >
> > -static inline bool s_range_overlap(int64_t start1, int64_t length1,
> > - int64_t start2, int64_t length2)
> > +static inline GList *g_list_insert_sorted_merged(GList *list,
> > + gpointer data,
> > + GCompareFunc func)
> > {
> > - return !((start1 + length1) < start2 || (start2 + length2) < start1);
> > -}
> > + GList *l, *next = NULL;
> > + Range *r, *nextr;
> >
> > -static inline int s_range_join(SignedRange *range,
> > - int64_t start, int64_t length)
> > -{
> > - if (s_range_overflow(start, length)) {
> > - return -1;
> > + if (!list) {
> > + list = g_list_insert_sorted(list, data, func);
> > + return list;
> > }
> >
> > - if (s_range_overlap(range->start, range->length, start, length)) {
> > - int64_t end = s_range_end(range->start, range->length);
> > - if (end < s_range_end(start, length)) {
> > - end = s_range_end(start, length);
> > + nextr = data;
> > + l = list;
> > + while (l && l != next && nextr) {
> > + r = l->data;
> > + if (ranges_can_merge(r, nextr)) {
> > + range_merge(r, nextr);
> > + l = g_list_remove_link(l, next);
> > + next = g_list_next(l);
> > + if (next) {
> > + nextr = next->data;
> > + } else {
> > + nextr = NULL;
> > + }
> > + } else {
> > + l = g_list_next(l);
> > }
> > - if (range->start > start) {
> > - range->start = start;
> > - }
> > - range->length = end - range->start + 1;
> > - return 0;
> > }
> >
> > - return -1;
> > + if (!l) {
> > + list = g_list_insert_sorted(list, data, func);
> > + }
> > +
> > + return list;
> > }
> >
> > -static inline int s_range_compare(int64_t start1, int64_t length1,
> > - int64_t start2, int64_t length2)
> > +static inline gint range_compare(gconstpointer a, gconstpointer b)
> > {
> > - if (start1 == start2 && length1 == length2) {
> > + Range *ra = (Range *)a, *rb = (Range *)b;
> > + if (ra->begin == rb->begin && ra->end == rb->end) {
> > return 0;
> > - } else if (s_range_end(start1, length1) <
> > - s_range_end(start2, length2)) {
> > + } else if (range_get_last(ra->begin, ra->end) <
> > + range_get_last(rb->begin, rb->end)) {
> > return -1;
> > } else {
> > return 1;
> > }
> > }
> >
> > -/* Add range to list. Keep them sorted, and merge ranges whenever possible */
> > -static inline bool range_list_add(SignedRangeList *list,
> > - int64_t start, int64_t length)
> > -{
> > - SignedRange *r, *next, *new_range = NULL, *cur = NULL;
> > -
> > - if (s_range_overflow(start, length)) {
> > - return false;
> > - }
> > -
> > - QTAILQ_FOREACH_SAFE(r, list, entry, next) {
> > - if (s_range_overlap(r->start, r->length, start, length)) {
> > - s_range_join(r, start, length);
> > - break;
> > - } else if (s_range_compare(start, length, r->start, r->length) < 0) {
> > - cur = r;
> > - break;
> > - }
> > - }
> > -
> > - if (!r) {
> > - new_range = s_range_new(start, length);
> > - QTAILQ_INSERT_TAIL(list, new_range, entry);
> > - } else if (cur) {
> > - new_range = s_range_new(start, length);
> > - QTAILQ_INSERT_BEFORE(cur, new_range, entry);
> > - } else {
> > - SignedRange *next = QTAILQ_NEXT(r, entry);
> > - while (next && s_range_overlap(r->start, r->length,
> > - next->start, next->length)) {
> > - s_range_join(r, next->start, next->length);
> > - QTAILQ_REMOVE(list, next, entry);
> > - s_range_free(next);
> > - next = QTAILQ_NEXT(r, entry);
> > - }
> > - }
> > -
> > - return true;
> > -}
> > -
> > #endif
> > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> > index 85ac6a1..0b2490b 100644
> > --- a/qapi/string-input-visitor.c
> > +++ b/qapi/string-input-visitor.c
> > @@ -25,8 +25,8 @@ struct StringInputVisitor
> >
> > bool head;
> >
> > - SignedRangeList *ranges;
> > - SignedRange *cur_range;
> > + GList *ranges;
> > + GList *cur_range;
> > int64_t cur;
> >
> > const char *string;
> > @@ -36,24 +36,25 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> > {
> > char *str = (char *) siv->string;
> > long long start, end;
> > - SignedRange *r, *next;
> > + Range *cur;
> > char *endptr;
> >
> > if (siv->ranges) {
> > return;
> > }
> >
> > - siv->ranges = g_malloc0(sizeof(*siv->ranges));
> > - QTAILQ_INIT(siv->ranges);
> > errno = 0;
> > do {
> > start = strtoll(str, &endptr, 0);
> > if (errno == 0 && endptr > str && INT64_MIN <= start &&
> > start <= INT64_MAX) {
> > if (*endptr == '\0') {
> > - if (!range_list_add(siv->ranges, start, 1)) {
> > - goto error;
> > - }
> > + cur = g_malloc0(sizeof(*cur));
> > + cur->begin = start;
> > + cur->end = start + 1;
> > + siv->ranges = g_list_insert_sorted_merged(siv->ranges, cur,
> > + range_compare);
> > + cur = NULL;
> > str = NULL;
> > } else if (*endptr == '-') {
> > str = endptr + 1;
> > @@ -63,17 +64,25 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> > (start > INT64_MAX - 65536 ||
> > end < start + 65536)) {
> > if (*endptr == '\0') {
> > - if (!range_list_add(siv->ranges, start,
> > - end - start + 1)) {
> > - goto error;
> > - }
> > + cur = g_malloc0(sizeof(*cur));
> > + cur->begin = start;
> > + cur->end = end + 1;
> > + siv->ranges =
> > + g_list_insert_sorted_merged(siv->ranges,
> > + cur,
> > + range_compare);
> > + cur = NULL;
> > str = NULL;
> > } else if (*endptr == ',') {
> > str = endptr + 1;
> > - if (!range_list_add(siv->ranges, start,
> > - end - start + 1)) {
> > - goto error;
> > - }
> > + cur = g_malloc0(sizeof(*cur));
> > + cur->begin = start;
> > + cur->end = end + 1;
> > + siv->ranges =
> > + g_list_insert_sorted_merged(siv->ranges,
> > + cur,
> > + range_compare);
> > + cur = NULL;
> > } else {
> > goto error;
> > }
> > @@ -82,9 +91,13 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> > }
> > } else if (*endptr == ',') {
> > str = endptr + 1;
> > - if (!range_list_add(siv->ranges, start, 1)) {
> > - goto error;
> > - }
> > + cur = g_malloc0(sizeof(*cur));
> > + cur->begin = start;
> > + cur->end = start + 1;
> > + siv->ranges = g_list_insert_sorted_merged(siv->ranges,
> > + cur,
> > + range_compare);
> > + cur = NULL;
> > } else {
> > goto error;
> > }
> > @@ -95,14 +108,8 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> >
> > return;
> > error:
> > - if (siv->ranges) {
> > - QTAILQ_FOREACH_SAFE(r, siv->ranges, entry, next) {
> > - QTAILQ_REMOVE(siv->ranges, r, entry);
> > - g_free(r);
> > - }
> > - g_free(siv->ranges);
> > - siv->ranges = NULL;
> > - }
> > + g_list_free_full(siv->ranges, g_free);
> > + assert(siv->ranges == NULL);
> > }
> >
> > static void
> > @@ -112,10 +119,11 @@ start_list(Visitor *v, const char *name, Error **errp)
> >
> > parse_str(siv, errp);
> >
> > - if (siv->ranges) {
> > - siv->cur_range = QTAILQ_FIRST(siv->ranges);
> > - if (siv->cur_range) {
> > - siv->cur = siv->cur_range->start;
> > + siv->cur_range = g_list_first(siv->ranges);
> > + if (siv->cur_range) {
> > + Range *r = siv->cur_range->data;
> > + if (r) {
> > + siv->cur = r->begin;
> > }
> > }
> > }
> > @@ -125,19 +133,27 @@ next_list(Visitor *v, GenericList **list, Error **errp)
> > {
> > StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> > GenericList **link;
> > + Range *r;
> >
> > if (!siv->ranges || !siv->cur_range) {
> > return NULL;
> > }
> >
> > - if (siv->cur < siv->cur_range->start ||
> > - siv->cur >= (siv->cur_range->start + siv->cur_range->length)) {
> > - siv->cur_range = QTAILQ_NEXT(siv->cur_range, entry);
> > - if (siv->cur_range) {
> > - siv->cur = siv->cur_range->start;
> > - } else {
> > + r = siv->cur_range->data;
> > + if (!r) {
> > + return NULL;
> > + }
> > +
> > + if (siv->cur < r->begin || siv->cur >= r->end) {
> > + siv->cur_range = g_list_next(siv->cur_range);
> > + if (!siv->cur_range) {
> > return NULL;
> > }
> > + r = siv->cur_range->data;
> > + if (!r) {
> > + return NULL;
> > + }
> > + siv->cur = r->begin;
> > }
> >
> > if (siv->head) {
> > @@ -176,12 +192,19 @@ static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
> > }
> >
> > if (!siv->cur_range) {
> > - siv->cur_range = QTAILQ_FIRST(siv->ranges);
> > - if (siv->cur_range) {
> > - siv->cur = siv->cur_range->start;
> > - } else {
> > + Range *r;
> > +
> > + siv->cur_range = g_list_first(siv->ranges);
> > + if (!siv->cur_range) {
> > + goto error;
> > + }
> > +
> > + r = siv->cur_range->data;
> > + if (!r) {
> > goto error;
> > }
> > +
> > + siv->cur = r->begin;
> > }
> >
> > *obj = siv->cur;
> > @@ -291,16 +314,7 @@ Visitor *string_input_get_visitor(StringInputVisitor *v)
> >
> > void string_input_visitor_cleanup(StringInputVisitor *v)
> > {
> > - SignedRange *r, *next;
> > -
> > - if (v->ranges) {
> > - QTAILQ_FOREACH_SAFE(r, v->ranges, entry, next) {
> > - QTAILQ_REMOVE(v->ranges, r, entry);
> > - g_free(r);
> > - }
> > - g_free(v->ranges);
> > - }
> > -
> > + g_list_free_full(v->ranges, g_free);
> > g_free(v);
> > }
> >
> > diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> > index ccebc7a..1c0834a 100644
> > --- a/qapi/string-output-visitor.c
> > +++ b/qapi/string-output-visitor.c
> > @@ -64,7 +64,7 @@ struct StringOutputVisitor
> > int64_t s;
> > uint64_t u;
> > } range_start, range_end;
> > - SignedRangeList *ranges;
> > + GList *ranges;
> > };
> >
> > static void string_output_set(StringOutputVisitor *sov, char *string)
> > @@ -78,25 +78,50 @@ static void string_output_set(StringOutputVisitor *sov, char *string)
> >
> > static void string_output_append(StringOutputVisitor *sov, int64_t a)
> > {
> > - range_list_add(sov->ranges, a, 1);
> > + Range *r = g_malloc0(sizeof(*r));
> > + r->begin = a;
> > + r->end = a + 1;
> > + sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
> > }
> >
> > static void string_output_append_range(StringOutputVisitor *sov,
> > int64_t s, int64_t e)
> > {
> > - range_list_add(sov->ranges, s, e);
> > + Range *r = g_malloc0(sizeof(*r));
> > + r->begin = s;
> > + r->end = e + 1;
> > + sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
> > +}
> > +
> > +static void format_string(StringOutputVisitor *sov, Range *r, bool next,
> > + bool human)
> > +{
> > + if (r->end - r->begin > 1) {
> > + if (human) {
> > + g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
> > + r->begin, r->end - 1);
> > +
> > + } else {
> > + g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
> > + r->begin, r->end - 1);
> > + }
> > + } else {
> > + if (human) {
> > + g_string_append_printf(sov->string, "%" PRIx64, r->begin);
> > + } else {
> > + g_string_append_printf(sov->string, "%" PRId64, r->begin);
> > + }
> > + }
> > + if (next) {
> > + g_string_append(sov->string, ",");
> > + }
> > }
> >
> > static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> > Error **errp)
> > {
> > StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> > - SignedRange *r;
> > -
> > - if (!sov->ranges) {
> > - sov->ranges = g_malloc0(sizeof(*sov->ranges));
> > - QTAILQ_INIT(sov->ranges);
> > - }
> > + GList *l;
> >
> > switch (sov->list_mode) {
> > case LM_NONE:
> > @@ -118,8 +143,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> > } else {
> > assert(sov->range_start.s < sov->range_end.s);
> > string_output_append_range(sov, sov->range_start.s,
> > - sov->range_end.s -
> > - sov->range_start.s + 1);
> > + sov->range_end.s);
> > }
> >
> > sov->range_start.s = *obj;
> > @@ -132,8 +156,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> > sov->range_end.s++;
> > assert(sov->range_start.s < sov->range_end.s);
> > string_output_append_range(sov, sov->range_start.s,
> > - sov->range_end.s -
> > - sov->range_start.s + 1);
> > + sov->range_end.s);
> > } else {
> > if (sov->range_start.s == sov->range_end.s) {
> > string_output_append(sov, sov->range_end.s);
> > @@ -141,8 +164,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> > assert(sov->range_start.s < sov->range_end.s);
> >
> > string_output_append_range(sov, sov->range_start.s,
> > - sov->range_end.s -
> > - sov->range_start.s + 1);
> > + sov->range_end.s);
> > }
> > string_output_append(sov, *obj);
> > }
> > @@ -152,34 +174,20 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> > abort();
> > }
> >
> > - QTAILQ_FOREACH(r, sov->ranges, entry) {
> > - if (r->length > 1) {
> > - g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
> > - r->start,
> > - s_range_end(r->start, r->length));
> > - } else {
> > - g_string_append_printf(sov->string, "%" PRId64,
> > - r->start);
> > - }
> > - if (r->entry.tqe_next) {
> > - g_string_append(sov->string, ",");
> > - }
> > + l = sov->ranges;
> > + while (l) {
> > + Range *r = l->data;
> > + format_string(sov, r, l->next != NULL, false);
> > + l = l->next;
> > }
> >
> > if (sov->human) {
> > + l = sov->ranges;
> > g_string_append(sov->string, " (");
> > - QTAILQ_FOREACH(r, sov->ranges, entry) {
> > - if (r->length > 1) {
> > - g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
> > - r->start,
> > - s_range_end(r->start, r->length));
> > - } else {
> > - g_string_append_printf(sov->string, "%" PRIx64,
> > - r->start);
> > - }
> > - if (r->entry.tqe_next) {
> > - g_string_append(sov->string, ",");
> > - }
> > + while (l) {
> > + Range *r = l->data;
> > + format_string(sov, r, l->next != NULL, false);
> > + l = l->next;
> > }
> > g_string_append(sov->string, ")");
> > }
> > @@ -310,20 +318,11 @@ Visitor *string_output_get_visitor(StringOutputVisitor *sov)
> >
> > void string_output_visitor_cleanup(StringOutputVisitor *sov)
> > {
> > - SignedRange *r, *next;
> > -
> > if (sov->string) {
> > g_string_free(sov->string, true);
> > }
> >
> > - if (sov->ranges) {
> > - QTAILQ_FOREACH_SAFE(r, sov->ranges, entry, next) {
> > - QTAILQ_REMOVE(sov->ranges, r, entry);
> > - s_range_free(r);
> > - }
> > - g_free(sov->ranges);
> > - }
> > -
> > + g_list_free_full(sov->ranges, g_free);
> > g_free(sov);
> > }
> >
> > diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
> > index b08a7db..b01e2f2 100644
> > --- a/tests/test-string-input-visitor.c
> > +++ b/tests/test-string-input-visitor.c
> > @@ -67,13 +67,13 @@ static void test_visitor_in_int(TestInputVisitorData *data,
> > static void test_visitor_in_intList(TestInputVisitorData *data,
> > const void *unused)
> > {
> > - int64_t value[] = {-2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
> > + int64_t value[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
> > int16List *res = NULL, *tmp;
> > Error *errp = NULL;
> > Visitor *v;
> > int i = 0;
> >
> > - v = visitor_input_test_init(data, "1,2,-2-1,2-4,20,5-9,1-8");
> > + v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
> >
> > visit_type_int16List(v, &res, NULL, &errp);
> > g_assert(errp == NULL);
> > diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
> > index d2ad580..28e7359 100644
> > --- a/tests/test-string-output-visitor.c
> > +++ b/tests/test-string-output-visitor.c
> > @@ -44,7 +44,7 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
> > static void test_visitor_out_int(TestOutputVisitorData *data,
> > const void *unused)
> > {
> > - int64_t value = -42;
> > + int64_t value = 42;
> > Error *err = NULL;
> > char *str;
> >
> > @@ -53,14 +53,14 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
> >
> > str = string_output_get_string(data->sov);
> > g_assert(str != NULL);
> > - g_assert_cmpstr(str, ==, "-42");
> > + g_assert_cmpstr(str, ==, "42");
> > g_free(str);
> > }
> >
> > static void test_visitor_out_intList(TestOutputVisitorData *data,
> > const void *unused)
> > {
> > - int64_t value[] = {-10, -7, -2, -1, 0, 1, 9, 10, 16, 15, 14,
> > + int64_t value[] = {0, 1, 9, 10, 16, 15, 14,
> > 3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};
> > intList *list = NULL, **tmp = &list;
> > int i;
> > @@ -79,7 +79,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
> > str = string_output_get_string(data->sov);
> > g_assert(str != NULL);
> > g_assert_cmpstr(str, ==,
> > - "-10,-7,-2-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
> > + "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
> > g_free(str);
> > while (list) {
> > intList *tmp2;
> > --
> > 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/4] get rid of signed range
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 1/4] get rid of signed range Hu Tao
2014-06-15 9:00 ` Michael S. Tsirkin
@ 2014-06-16 15:06 ` Michael S. Tsirkin
1 sibling, 0 replies; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-16 15:06 UTC (permalink / raw)
To: Hu Tao; +Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Sat, Jun 14, 2014 at 12:48:56PM +0800, Hu Tao wrote:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Breaks build with glib < 2.28.
Replaced free_full with foreach + free.
> ---
> include/qemu/range.h | 144 ++++++++++++-------------------------
> qapi/string-input-visitor.c | 116 +++++++++++++++++-------------
> qapi/string-output-visitor.c | 97 +++++++++++++------------
> tests/test-string-input-visitor.c | 4 +-
> tests/test-string-output-visitor.c | 8 +--
> 5 files changed, 165 insertions(+), 204 deletions(-)
>
> diff --git a/include/qemu/range.h b/include/qemu/range.h
> index 8879f8a..cfa021f 100644
> --- a/include/qemu/range.h
> +++ b/include/qemu/range.h
> @@ -61,127 +61,75 @@ static inline int ranges_overlap(uint64_t first1, uint64_t len1,
> return !(last2 < first1 || last1 < first2);
> }
>
> -typedef struct SignedRangeList SignedRangeList;
> -
> -typedef struct SignedRange {
> - int64_t start;
> - int64_t length;
> -
> - QTAILQ_ENTRY(SignedRange) entry;
> -} SignedRange;
> -
> -QTAILQ_HEAD(SignedRangeList, SignedRange);
> -
> -static inline int64_t s_range_end(int64_t start, int64_t length)
> -{
> - return start + length - 1;
> -}
> -
> -/* negative length or overflow */
> -static inline bool s_range_overflow(int64_t start, int64_t length)
> +/* 0,1 can merge with 1,2 but don't overlap */
> +static inline bool ranges_can_merge(Range *range1, Range *range2)
> {
> - return s_range_end(start, length) < start;
> + return !(range1->end < range2->begin || range2->end < range1->begin);
> }
>
> -static inline SignedRange *s_range_new(int64_t start, int64_t length)
> +static inline int range_merge(Range *range1, Range *range2)
> {
> - SignedRange *range = NULL;
> -
> - if (s_range_overflow(start, length)) {
> - return NULL;
> + if (ranges_can_merge(range1, range2)) {
> + if (range1->end < range2->end) {
> + range1->end = range2->end;
> + }
> + if (range1->begin > range2->begin) {
> + range1->begin = range2->begin;
> + }
> + return 0;
> }
>
> - range = g_malloc0(sizeof(*range));
> - range->start = start;
> - range->length = length;
> -
> - return range;
> -}
> -
> -static inline void s_range_free(SignedRange *range)
> -{
> - g_free(range);
> + return -1;
> }
>
> -static inline bool s_range_overlap(int64_t start1, int64_t length1,
> - int64_t start2, int64_t length2)
> +static inline GList *g_list_insert_sorted_merged(GList *list,
> + gpointer data,
> + GCompareFunc func)
> {
> - return !((start1 + length1) < start2 || (start2 + length2) < start1);
> -}
> + GList *l, *next = NULL;
> + Range *r, *nextr;
>
> -static inline int s_range_join(SignedRange *range,
> - int64_t start, int64_t length)
> -{
> - if (s_range_overflow(start, length)) {
> - return -1;
> + if (!list) {
> + list = g_list_insert_sorted(list, data, func);
> + return list;
> }
>
> - if (s_range_overlap(range->start, range->length, start, length)) {
> - int64_t end = s_range_end(range->start, range->length);
> - if (end < s_range_end(start, length)) {
> - end = s_range_end(start, length);
> + nextr = data;
> + l = list;
> + while (l && l != next && nextr) {
> + r = l->data;
> + if (ranges_can_merge(r, nextr)) {
> + range_merge(r, nextr);
> + l = g_list_remove_link(l, next);
> + next = g_list_next(l);
> + if (next) {
> + nextr = next->data;
> + } else {
> + nextr = NULL;
> + }
> + } else {
> + l = g_list_next(l);
> }
> - if (range->start > start) {
> - range->start = start;
> - }
> - range->length = end - range->start + 1;
> - return 0;
> }
>
> - return -1;
> + if (!l) {
> + list = g_list_insert_sorted(list, data, func);
> + }
> +
> + return list;
> }
>
> -static inline int s_range_compare(int64_t start1, int64_t length1,
> - int64_t start2, int64_t length2)
> +static inline gint range_compare(gconstpointer a, gconstpointer b)
> {
> - if (start1 == start2 && length1 == length2) {
> + Range *ra = (Range *)a, *rb = (Range *)b;
> + if (ra->begin == rb->begin && ra->end == rb->end) {
> return 0;
> - } else if (s_range_end(start1, length1) <
> - s_range_end(start2, length2)) {
> + } else if (range_get_last(ra->begin, ra->end) <
> + range_get_last(rb->begin, rb->end)) {
> return -1;
> } else {
> return 1;
> }
> }
>
> -/* Add range to list. Keep them sorted, and merge ranges whenever possible */
> -static inline bool range_list_add(SignedRangeList *list,
> - int64_t start, int64_t length)
> -{
> - SignedRange *r, *next, *new_range = NULL, *cur = NULL;
> -
> - if (s_range_overflow(start, length)) {
> - return false;
> - }
> -
> - QTAILQ_FOREACH_SAFE(r, list, entry, next) {
> - if (s_range_overlap(r->start, r->length, start, length)) {
> - s_range_join(r, start, length);
> - break;
> - } else if (s_range_compare(start, length, r->start, r->length) < 0) {
> - cur = r;
> - break;
> - }
> - }
> -
> - if (!r) {
> - new_range = s_range_new(start, length);
> - QTAILQ_INSERT_TAIL(list, new_range, entry);
> - } else if (cur) {
> - new_range = s_range_new(start, length);
> - QTAILQ_INSERT_BEFORE(cur, new_range, entry);
> - } else {
> - SignedRange *next = QTAILQ_NEXT(r, entry);
> - while (next && s_range_overlap(r->start, r->length,
> - next->start, next->length)) {
> - s_range_join(r, next->start, next->length);
> - QTAILQ_REMOVE(list, next, entry);
> - s_range_free(next);
> - next = QTAILQ_NEXT(r, entry);
> - }
> - }
> -
> - return true;
> -}
> -
> #endif
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 85ac6a1..0b2490b 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -25,8 +25,8 @@ struct StringInputVisitor
>
> bool head;
>
> - SignedRangeList *ranges;
> - SignedRange *cur_range;
> + GList *ranges;
> + GList *cur_range;
> int64_t cur;
>
> const char *string;
> @@ -36,24 +36,25 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> {
> char *str = (char *) siv->string;
> long long start, end;
> - SignedRange *r, *next;
> + Range *cur;
> char *endptr;
>
> if (siv->ranges) {
> return;
> }
>
> - siv->ranges = g_malloc0(sizeof(*siv->ranges));
> - QTAILQ_INIT(siv->ranges);
> errno = 0;
> do {
> start = strtoll(str, &endptr, 0);
> if (errno == 0 && endptr > str && INT64_MIN <= start &&
> start <= INT64_MAX) {
> if (*endptr == '\0') {
> - if (!range_list_add(siv->ranges, start, 1)) {
> - goto error;
> - }
> + cur = g_malloc0(sizeof(*cur));
> + cur->begin = start;
> + cur->end = start + 1;
> + siv->ranges = g_list_insert_sorted_merged(siv->ranges, cur,
> + range_compare);
> + cur = NULL;
> str = NULL;
> } else if (*endptr == '-') {
> str = endptr + 1;
> @@ -63,17 +64,25 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> (start > INT64_MAX - 65536 ||
> end < start + 65536)) {
> if (*endptr == '\0') {
> - if (!range_list_add(siv->ranges, start,
> - end - start + 1)) {
> - goto error;
> - }
> + cur = g_malloc0(sizeof(*cur));
> + cur->begin = start;
> + cur->end = end + 1;
> + siv->ranges =
> + g_list_insert_sorted_merged(siv->ranges,
> + cur,
> + range_compare);
> + cur = NULL;
> str = NULL;
> } else if (*endptr == ',') {
> str = endptr + 1;
> - if (!range_list_add(siv->ranges, start,
> - end - start + 1)) {
> - goto error;
> - }
> + cur = g_malloc0(sizeof(*cur));
> + cur->begin = start;
> + cur->end = end + 1;
> + siv->ranges =
> + g_list_insert_sorted_merged(siv->ranges,
> + cur,
> + range_compare);
> + cur = NULL;
> } else {
> goto error;
> }
> @@ -82,9 +91,13 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> }
> } else if (*endptr == ',') {
> str = endptr + 1;
> - if (!range_list_add(siv->ranges, start, 1)) {
> - goto error;
> - }
> + cur = g_malloc0(sizeof(*cur));
> + cur->begin = start;
> + cur->end = start + 1;
> + siv->ranges = g_list_insert_sorted_merged(siv->ranges,
> + cur,
> + range_compare);
> + cur = NULL;
> } else {
> goto error;
> }
> @@ -95,14 +108,8 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
>
> return;
> error:
> - if (siv->ranges) {
> - QTAILQ_FOREACH_SAFE(r, siv->ranges, entry, next) {
> - QTAILQ_REMOVE(siv->ranges, r, entry);
> - g_free(r);
> - }
> - g_free(siv->ranges);
> - siv->ranges = NULL;
> - }
> + g_list_free_full(siv->ranges, g_free);
> + assert(siv->ranges == NULL);
> }
>
> static void
> @@ -112,10 +119,11 @@ start_list(Visitor *v, const char *name, Error **errp)
>
> parse_str(siv, errp);
>
> - if (siv->ranges) {
> - siv->cur_range = QTAILQ_FIRST(siv->ranges);
> - if (siv->cur_range) {
> - siv->cur = siv->cur_range->start;
> + siv->cur_range = g_list_first(siv->ranges);
> + if (siv->cur_range) {
> + Range *r = siv->cur_range->data;
> + if (r) {
> + siv->cur = r->begin;
> }
> }
> }
> @@ -125,19 +133,27 @@ next_list(Visitor *v, GenericList **list, Error **errp)
> {
> StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> GenericList **link;
> + Range *r;
>
> if (!siv->ranges || !siv->cur_range) {
> return NULL;
> }
>
> - if (siv->cur < siv->cur_range->start ||
> - siv->cur >= (siv->cur_range->start + siv->cur_range->length)) {
> - siv->cur_range = QTAILQ_NEXT(siv->cur_range, entry);
> - if (siv->cur_range) {
> - siv->cur = siv->cur_range->start;
> - } else {
> + r = siv->cur_range->data;
> + if (!r) {
> + return NULL;
> + }
> +
> + if (siv->cur < r->begin || siv->cur >= r->end) {
> + siv->cur_range = g_list_next(siv->cur_range);
> + if (!siv->cur_range) {
> return NULL;
> }
> + r = siv->cur_range->data;
> + if (!r) {
> + return NULL;
> + }
> + siv->cur = r->begin;
> }
>
> if (siv->head) {
> @@ -176,12 +192,19 @@ static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
> }
>
> if (!siv->cur_range) {
> - siv->cur_range = QTAILQ_FIRST(siv->ranges);
> - if (siv->cur_range) {
> - siv->cur = siv->cur_range->start;
> - } else {
> + Range *r;
> +
> + siv->cur_range = g_list_first(siv->ranges);
> + if (!siv->cur_range) {
> + goto error;
> + }
> +
> + r = siv->cur_range->data;
> + if (!r) {
> goto error;
> }
> +
> + siv->cur = r->begin;
> }
>
> *obj = siv->cur;
> @@ -291,16 +314,7 @@ Visitor *string_input_get_visitor(StringInputVisitor *v)
>
> void string_input_visitor_cleanup(StringInputVisitor *v)
> {
> - SignedRange *r, *next;
> -
> - if (v->ranges) {
> - QTAILQ_FOREACH_SAFE(r, v->ranges, entry, next) {
> - QTAILQ_REMOVE(v->ranges, r, entry);
> - g_free(r);
> - }
> - g_free(v->ranges);
> - }
> -
> + g_list_free_full(v->ranges, g_free);
> g_free(v);
> }
>
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index ccebc7a..1c0834a 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -64,7 +64,7 @@ struct StringOutputVisitor
> int64_t s;
> uint64_t u;
> } range_start, range_end;
> - SignedRangeList *ranges;
> + GList *ranges;
> };
>
> static void string_output_set(StringOutputVisitor *sov, char *string)
> @@ -78,25 +78,50 @@ static void string_output_set(StringOutputVisitor *sov, char *string)
>
> static void string_output_append(StringOutputVisitor *sov, int64_t a)
> {
> - range_list_add(sov->ranges, a, 1);
> + Range *r = g_malloc0(sizeof(*r));
> + r->begin = a;
> + r->end = a + 1;
> + sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
> }
>
> static void string_output_append_range(StringOutputVisitor *sov,
> int64_t s, int64_t e)
> {
> - range_list_add(sov->ranges, s, e);
> + Range *r = g_malloc0(sizeof(*r));
> + r->begin = s;
> + r->end = e + 1;
> + sov->ranges = g_list_insert_sorted_merged(sov->ranges, r, range_compare);
> +}
> +
> +static void format_string(StringOutputVisitor *sov, Range *r, bool next,
> + bool human)
> +{
> + if (r->end - r->begin > 1) {
> + if (human) {
> + g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
> + r->begin, r->end - 1);
> +
> + } else {
> + g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
> + r->begin, r->end - 1);
> + }
> + } else {
> + if (human) {
> + g_string_append_printf(sov->string, "%" PRIx64, r->begin);
> + } else {
> + g_string_append_printf(sov->string, "%" PRId64, r->begin);
> + }
> + }
> + if (next) {
> + g_string_append(sov->string, ",");
> + }
> }
>
> static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> Error **errp)
> {
> StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> - SignedRange *r;
> -
> - if (!sov->ranges) {
> - sov->ranges = g_malloc0(sizeof(*sov->ranges));
> - QTAILQ_INIT(sov->ranges);
> - }
> + GList *l;
>
> switch (sov->list_mode) {
> case LM_NONE:
> @@ -118,8 +143,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> } else {
> assert(sov->range_start.s < sov->range_end.s);
> string_output_append_range(sov, sov->range_start.s,
> - sov->range_end.s -
> - sov->range_start.s + 1);
> + sov->range_end.s);
> }
>
> sov->range_start.s = *obj;
> @@ -132,8 +156,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> sov->range_end.s++;
> assert(sov->range_start.s < sov->range_end.s);
> string_output_append_range(sov, sov->range_start.s,
> - sov->range_end.s -
> - sov->range_start.s + 1);
> + sov->range_end.s);
> } else {
> if (sov->range_start.s == sov->range_end.s) {
> string_output_append(sov, sov->range_end.s);
> @@ -141,8 +164,7 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> assert(sov->range_start.s < sov->range_end.s);
>
> string_output_append_range(sov, sov->range_start.s,
> - sov->range_end.s -
> - sov->range_start.s + 1);
> + sov->range_end.s);
> }
> string_output_append(sov, *obj);
> }
> @@ -152,34 +174,20 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
> abort();
> }
>
> - QTAILQ_FOREACH(r, sov->ranges, entry) {
> - if (r->length > 1) {
> - g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
> - r->start,
> - s_range_end(r->start, r->length));
> - } else {
> - g_string_append_printf(sov->string, "%" PRId64,
> - r->start);
> - }
> - if (r->entry.tqe_next) {
> - g_string_append(sov->string, ",");
> - }
> + l = sov->ranges;
> + while (l) {
> + Range *r = l->data;
> + format_string(sov, r, l->next != NULL, false);
> + l = l->next;
> }
>
> if (sov->human) {
> + l = sov->ranges;
> g_string_append(sov->string, " (");
> - QTAILQ_FOREACH(r, sov->ranges, entry) {
> - if (r->length > 1) {
> - g_string_append_printf(sov->string, "%" PRIx64 "-%" PRIx64,
> - r->start,
> - s_range_end(r->start, r->length));
> - } else {
> - g_string_append_printf(sov->string, "%" PRIx64,
> - r->start);
> - }
> - if (r->entry.tqe_next) {
> - g_string_append(sov->string, ",");
> - }
> + while (l) {
> + Range *r = l->data;
> + format_string(sov, r, l->next != NULL, false);
> + l = l->next;
> }
> g_string_append(sov->string, ")");
> }
> @@ -310,20 +318,11 @@ Visitor *string_output_get_visitor(StringOutputVisitor *sov)
>
> void string_output_visitor_cleanup(StringOutputVisitor *sov)
> {
> - SignedRange *r, *next;
> -
> if (sov->string) {
> g_string_free(sov->string, true);
> }
>
> - if (sov->ranges) {
> - QTAILQ_FOREACH_SAFE(r, sov->ranges, entry, next) {
> - QTAILQ_REMOVE(sov->ranges, r, entry);
> - s_range_free(r);
> - }
> - g_free(sov->ranges);
> - }
> -
> + g_list_free_full(sov->ranges, g_free);
> g_free(sov);
> }
>
> diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
> index b08a7db..b01e2f2 100644
> --- a/tests/test-string-input-visitor.c
> +++ b/tests/test-string-input-visitor.c
> @@ -67,13 +67,13 @@ static void test_visitor_in_int(TestInputVisitorData *data,
> static void test_visitor_in_intList(TestInputVisitorData *data,
> const void *unused)
> {
> - int64_t value[] = {-2, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
> + int64_t value[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
> int16List *res = NULL, *tmp;
> Error *errp = NULL;
> Visitor *v;
> int i = 0;
>
> - v = visitor_input_test_init(data, "1,2,-2-1,2-4,20,5-9,1-8");
> + v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
>
> visit_type_int16List(v, &res, NULL, &errp);
> g_assert(errp == NULL);
> diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
> index d2ad580..28e7359 100644
> --- a/tests/test-string-output-visitor.c
> +++ b/tests/test-string-output-visitor.c
> @@ -44,7 +44,7 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
> static void test_visitor_out_int(TestOutputVisitorData *data,
> const void *unused)
> {
> - int64_t value = -42;
> + int64_t value = 42;
> Error *err = NULL;
> char *str;
>
> @@ -53,14 +53,14 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
>
> str = string_output_get_string(data->sov);
> g_assert(str != NULL);
> - g_assert_cmpstr(str, ==, "-42");
> + g_assert_cmpstr(str, ==, "42");
> g_free(str);
> }
>
> static void test_visitor_out_intList(TestOutputVisitorData *data,
> const void *unused)
> {
> - int64_t value[] = {-10, -7, -2, -1, 0, 1, 9, 10, 16, 15, 14,
> + int64_t value[] = {0, 1, 9, 10, 16, 15, 14,
> 3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};
> intList *list = NULL, **tmp = &list;
> int i;
> @@ -79,7 +79,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
> str = string_output_get_string(data->sov);
> g_assert(str != NULL);
> g_assert_cmpstr(str, ==,
> - "-10,-7,-2-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
> + "0-1,3-6,9-16,21-22,9223372036854775806-9223372036854775807");
> g_free(str);
> while (list) {
> intList *tmp2;
> --
> 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH RFC 2/4] check if we have space left for hotplugged memory
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Hu Tao
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 1/4] get rid of signed range Hu Tao
@ 2014-06-14 4:48 ` Hu Tao
2014-06-15 8:53 ` Michael S. Tsirkin
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory Hu Tao
` (2 subsequent siblings)
4 siblings, 1 reply; 47+ messages in thread
From: Hu Tao @ 2014-06-14 4:48 UTC (permalink / raw)
To: qemu-devel
Cc: Yasunori Goto, Paolo Bonzini, Igor Mammedov, Michael S. Tsirkin
If pc-dimm is specified on qemu command line, but only with
-m size (aka not -m size,maxmem,slots) then qemu will core dump.
This patch fixes the problem.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
hw/mem/pc-dimm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 8c26568..6e8bf43 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -107,7 +107,12 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
uint64_t new_addr, ret = 0;
uint64_t address_space_end = address_space_start + address_space_size;
- assert(address_space_end > address_space_size);
+ if (address_space_size == 0) {
+ error_setg(errp, "can't add memory beyond 0x%" PRIx64,
+ address_space_end);
+ goto out;
+ }
+
object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
if (hint) {
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 2/4] check if we have space left for hotplugged memory
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 2/4] check if we have space left for hotplugged memory Hu Tao
@ 2014-06-15 8:53 ` Michael S. Tsirkin
2014-06-16 9:47 ` Hu Tao
0 siblings, 1 reply; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-15 8:53 UTC (permalink / raw)
To: Hu Tao; +Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Sat, Jun 14, 2014 at 12:48:57PM +0800, Hu Tao wrote:
> If pc-dimm is specified on qemu command line, but only with
> -m size (aka not -m size,maxmem,slots) then qemu will core dump.
>
> This patch fixes the problem.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> hw/mem/pc-dimm.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 8c26568..6e8bf43 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -107,7 +107,12 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
> uint64_t new_addr, ret = 0;
> uint64_t address_space_end = address_space_start + address_space_size;
>
> - assert(address_space_end > address_space_size);
> + if (address_space_size == 0) {
> + error_setg(errp, "can't add memory beyond 0x%" PRIx64,
> + address_space_end);
That's quite an unfriendly error message, isn't it?
Why not explain what the problem is to the user?
> + goto out;
> + }
> +
I would move the assert to this point. It protects against
integer overflow.
> object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
>
> if (hint) {
> --
> 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 2/4] check if we have space left for hotplugged memory
2014-06-15 8:53 ` Michael S. Tsirkin
@ 2014-06-16 9:47 ` Hu Tao
0 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-16 9:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Sun, Jun 15, 2014 at 11:53:52AM +0300, Michael S. Tsirkin wrote:
> On Sat, Jun 14, 2014 at 12:48:57PM +0800, Hu Tao wrote:
> > If pc-dimm is specified on qemu command line, but only with
> > -m size (aka not -m size,maxmem,slots) then qemu will core dump.
> >
> > This patch fixes the problem.
> >
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> > hw/mem/pc-dimm.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> > index 8c26568..6e8bf43 100644
> > --- a/hw/mem/pc-dimm.c
> > +++ b/hw/mem/pc-dimm.c
> > @@ -107,7 +107,12 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
> > uint64_t new_addr, ret = 0;
> > uint64_t address_space_end = address_space_start + address_space_size;
> >
> > - assert(address_space_end > address_space_size);
> > + if (address_space_size == 0) {
> > + error_setg(errp, "can't add memory beyond 0x%" PRIx64,
> > + address_space_end);
>
> That's quite an unfriendly error message, isn't it?
> Why not explain what the problem is to the user?
Thanks! patch updated.
>
> > + goto out;
> > + }
> > +
>
> I would move the assert to this point. It protects against
> integer overflow.
>
> > object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
> >
> > if (hint) {
>
>
> > --
> > 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Hu Tao
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 1/4] get rid of signed range Hu Tao
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 2/4] check if we have space left for hotplugged memory Hu Tao
@ 2014-06-14 4:48 ` Hu Tao
2014-06-14 17:07 ` Paolo Bonzini
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 4/4] memory-backend-file: error out " Hu Tao
2014-06-15 10:00 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Michael S. Tsirkin
4 siblings, 1 reply; 47+ messages in thread
From: Hu Tao @ 2014-06-14 4:48 UTC (permalink / raw)
To: qemu-devel
Cc: Yasunori Goto, Paolo Bonzini, Igor Mammedov, Michael S. Tsirkin
return -1 instead.
Now user can add objects memory-backend-ram on-the-fly, fail it if
cannot allocate memory rather than quit qemu.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem-ram.c | 3 +++
exec.c | 6 +++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index d9a8290..afb305d 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -28,6 +28,9 @@ 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);
+ if (backend->mr.ram_addr == -1) {
+ error_setg(errp, "can't allocate memory");
+ }
g_free(path);
}
diff --git a/exec.c b/exec.c
index 8705cc5..74560e5 100644
--- a/exec.c
+++ b/exec.c
@@ -1228,7 +1228,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
if (!new_block->host) {
fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
new_block->mr->name, strerror(errno));
- exit(1);
+ return -1;
}
memory_try_enable_merging(new_block->host, new_block->length);
}
@@ -1356,6 +1356,10 @@ void qemu_ram_free(ram_addr_t addr)
{
RAMBlock *block;
+ if (addr == -1) {
+ return;
+ }
+
/* This assumes the iothread lock is taken here too. */
qemu_mutex_lock_ramlist();
QTAILQ_FOREACH(block, &ram_list.blocks, next) {
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory Hu Tao
@ 2014-06-14 17:07 ` Paolo Bonzini
2014-06-15 9:58 ` Michael S. Tsirkin
0 siblings, 1 reply; 47+ messages in thread
From: Paolo Bonzini @ 2014-06-14 17:07 UTC (permalink / raw)
To: Hu Tao, qemu-devel; +Cc: Yasunori Goto, Michael S. Tsirkin, Igor Mammedov
Il 14/06/2014 06:48, Hu Tao ha scritto:
> return -1 instead.
>
> Now user can add objects memory-backend-ram on-the-fly, fail it if
> cannot allocate memory rather than quit qemu.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
This needs an audit of all callers or, alternatively, we need to add
memory_region_init_ram_nofail. Better leave it for after the merge.
Paolo
> ---
> backends/hostmem-ram.c | 3 +++
> exec.c | 6 +++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index d9a8290..afb305d 100644
> --- a/backends/hostmem-ram.c
> +++ b/backends/hostmem-ram.c
> @@ -28,6 +28,9 @@ 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);
> + if (backend->mr.ram_addr == -1) {
> + error_setg(errp, "can't allocate memory");
> + }
> g_free(path);
> }
>
> diff --git a/exec.c b/exec.c
> index 8705cc5..74560e5 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1228,7 +1228,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
> if (!new_block->host) {
> fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
> new_block->mr->name, strerror(errno));
> - exit(1);
> + return -1;
> }
> memory_try_enable_merging(new_block->host, new_block->length);
> }
> @@ -1356,6 +1356,10 @@ void qemu_ram_free(ram_addr_t addr)
> {
> RAMBlock *block;
>
> + if (addr == -1) {
> + return;
> + }
> +
> /* This assumes the iothread lock is taken here too. */
> qemu_mutex_lock_ramlist();
> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory
2014-06-14 17:07 ` Paolo Bonzini
@ 2014-06-15 9:58 ` Michael S. Tsirkin
2014-06-16 9:54 ` Hu Tao
0 siblings, 1 reply; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-15 9:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Hu Tao, Igor Mammedov, qemu-devel, Yasunori Goto
On Sat, Jun 14, 2014 at 07:07:39PM +0200, Paolo Bonzini wrote:
> Il 14/06/2014 06:48, Hu Tao ha scritto:
> >return -1 instead.
> >
> >Now user can add objects memory-backend-ram on-the-fly, fail it if
> >cannot allocate memory rather than quit qemu.
> >
> >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>
> This needs an audit of all callers or, alternatively, we need to add
> memory_region_init_ram_nofail. Better leave it for after the merge.
>
> Paolo
Specifically memory_region_init_ram_from_file does not seem to
handle failures.
qemu_ram_free chunk also looks weird. Can we not avoid calling
free on invalid addresses?
> >---
> > backends/hostmem-ram.c | 3 +++
> > exec.c | 6 +++++-
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> >diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> >index d9a8290..afb305d 100644
> >--- a/backends/hostmem-ram.c
> >+++ b/backends/hostmem-ram.c
> >@@ -28,6 +28,9 @@ 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);
> >+ if (backend->mr.ram_addr == -1) {
> >+ error_setg(errp, "can't allocate memory");
> >+ }
> > g_free(path);
> > }
> >
> >diff --git a/exec.c b/exec.c
> >index 8705cc5..74560e5 100644
> >--- a/exec.c
> >+++ b/exec.c
> >@@ -1228,7 +1228,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
> > if (!new_block->host) {
> > fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
> > new_block->mr->name, strerror(errno));
> >- exit(1);
> >+ return -1;
> > }
> > memory_try_enable_merging(new_block->host, new_block->length);
> > }
> >@@ -1356,6 +1356,10 @@ void qemu_ram_free(ram_addr_t addr)
> > {
> > RAMBlock *block;
> >
> >+ if (addr == -1) {
> >+ return;
> >+ }
> >+
> > /* This assumes the iothread lock is taken here too. */
> > qemu_mutex_lock_ramlist();
> > QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> >
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory
2014-06-15 9:58 ` Michael S. Tsirkin
@ 2014-06-16 9:54 ` Hu Tao
2014-06-16 10:07 ` Paolo Bonzini
0 siblings, 1 reply; 47+ messages in thread
From: Hu Tao @ 2014-06-16 9:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Sun, Jun 15, 2014 at 12:58:47PM +0300, Michael S. Tsirkin wrote:
> On Sat, Jun 14, 2014 at 07:07:39PM +0200, Paolo Bonzini wrote:
> > Il 14/06/2014 06:48, Hu Tao ha scritto:
> > >return -1 instead.
> > >
> > >Now user can add objects memory-backend-ram on-the-fly, fail it if
> > >cannot allocate memory rather than quit qemu.
> > >
> > >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> >
> > This needs an audit of all callers or, alternatively, we need to add
> > memory_region_init_ram_nofail. Better leave it for after the merge.
Paolo, IIUC you suggested we fix it after merge.
> >
> > Paolo
>
> Specifically memory_region_init_ram_from_file does not seem to
> handle failures.
memory_region_init_ram_from_file has an errp. Also see my updated patch.
>
> qemu_ram_free chunk also looks weird. Can we not avoid calling
> free on invalid addresses?
but we still need to check the address somewhere if not in
qemu_ram_free.
>
> > >---
> > > backends/hostmem-ram.c | 3 +++
> > > exec.c | 6 +++++-
> > > 2 files changed, 8 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> > >index d9a8290..afb305d 100644
> > >--- a/backends/hostmem-ram.c
> > >+++ b/backends/hostmem-ram.c
> > >@@ -28,6 +28,9 @@ 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);
> > >+ if (backend->mr.ram_addr == -1) {
> > >+ error_setg(errp, "can't allocate memory");
> > >+ }
> > > g_free(path);
> > > }
> > >
> > >diff --git a/exec.c b/exec.c
> > >index 8705cc5..74560e5 100644
> > >--- a/exec.c
> > >+++ b/exec.c
> > >@@ -1228,7 +1228,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
> > > if (!new_block->host) {
> > > fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
> > > new_block->mr->name, strerror(errno));
> > >- exit(1);
> > >+ return -1;
> > > }
> > > memory_try_enable_merging(new_block->host, new_block->length);
> > > }
> > >@@ -1356,6 +1356,10 @@ void qemu_ram_free(ram_addr_t addr)
> > > {
> > > RAMBlock *block;
> > >
> > >+ if (addr == -1) {
> > >+ return;
> > >+ }
> > >+
> > > /* This assumes the iothread lock is taken here too. */
> > > qemu_mutex_lock_ramlist();
> > > QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> > >
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory
2014-06-16 9:54 ` Hu Tao
@ 2014-06-16 10:07 ` Paolo Bonzini
0 siblings, 0 replies; 47+ messages in thread
From: Paolo Bonzini @ 2014-06-16 10:07 UTC (permalink / raw)
To: Hu Tao, Michael S. Tsirkin; +Cc: Yasunori Goto, qemu-devel, Igor Mammedov
Il 16/06/2014 11:54, Hu Tao ha scritto:
>>> > > This needs an audit of all callers or, alternatively, we need to add
>>> > > memory_region_init_ram_nofail. Better leave it for after the merge.
> Paolo, IIUC you suggested we fix it after merge.
Yes.
>> > Specifically memory_region_init_ram_from_file does not seem to
>> > handle failures.
> memory_region_init_ram_from_file has an errp. Also see my updated patch.
Yes, memory_region_init_ram_from_file is okay. memory_region_init_ram
is the one that doesn't handle failures.
> > qemu_ram_free chunk also looks weird. Can we not avoid calling
> > free on invalid addresses?
>
> but we still need to check the address somewhere if not in
> qemu_ram_free.
If we add an errp to memory_region_init_ram, the qemu_ram_free hunk
probably will not be necessary anymore.
Paolo
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCH RFC 4/4] memory-backend-file: error out if failed to allocate memory
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Hu Tao
` (2 preceding siblings ...)
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory Hu Tao
@ 2014-06-14 4:48 ` Hu Tao
2014-06-14 17:09 ` Paolo Bonzini
2014-06-15 10:00 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Michael S. Tsirkin
4 siblings, 1 reply; 47+ messages in thread
From: Hu Tao @ 2014-06-14 4:48 UTC (permalink / raw)
To: qemu-devel
Cc: Yasunori Goto, Paolo Bonzini, Igor Mammedov, Michael S. Tsirkin
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)
This patch fixes this problem.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
backends/hostmem-file.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 5179994..70172d1 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -55,6 +55,9 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
object_get_canonical_path(OBJECT(backend)),
backend->size, fb->share,
fb->mem_path, errp);
+ if (backend->mr.ram_addr == -1) {
+ error_setg(errp, "failed to allocate memory");
+ }
}
#endif
}
--
1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 4/4] memory-backend-file: error out if failed to allocate memory
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 4/4] memory-backend-file: error out " Hu Tao
@ 2014-06-14 17:09 ` Paolo Bonzini
2014-06-16 6:30 ` Hu Tao
0 siblings, 1 reply; 47+ messages in thread
From: Paolo Bonzini @ 2014-06-14 17:09 UTC (permalink / raw)
To: Hu Tao, qemu-devel; +Cc: Yasunori Goto, Michael S. Tsirkin, Igor Mammedov
Il 14/06/2014 06:48, Hu Tao ha scritto:
> 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)
>
> This patch fixes this problem.
>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
> backends/hostmem-file.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 5179994..70172d1 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -55,6 +55,9 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> object_get_canonical_path(OBJECT(backend)),
> backend->size, fb->share,
> fb->mem_path, errp);
> + if (backend->mr.ram_addr == -1) {
> + error_setg(errp, "failed to allocate memory");
> + }
qemu_ram_alloc_from_file is where this error_setg should be added instead.
Paolo
> }
> #endif
> }
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 4/4] memory-backend-file: error out if failed to allocate memory
2014-06-14 17:09 ` Paolo Bonzini
@ 2014-06-16 6:30 ` Hu Tao
0 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-16 6:30 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Yasunori Goto, Michael S. Tsirkin, qemu-devel, Igor Mammedov
On Sat, Jun 14, 2014 at 07:09:37PM +0200, Paolo Bonzini wrote:
> Il 14/06/2014 06:48, Hu Tao ha scritto:
> >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)
> >
> >This patch fixes this problem.
> >
> >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> >---
> > backends/hostmem-file.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> >index 5179994..70172d1 100644
> >--- a/backends/hostmem-file.c
> >+++ b/backends/hostmem-file.c
> >@@ -55,6 +55,9 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > object_get_canonical_path(OBJECT(backend)),
> > backend->size, fb->share,
> > fb->mem_path, errp);
> >+ if (backend->mr.ram_addr == -1) {
> >+ error_setg(errp, "failed to allocate memory");
> >+ }
>
> qemu_ram_alloc_from_file is where this error_setg should be added instead.
Thanks, patch updated.
>
> Paolo
>
> > }
> > #endif
> > }
> >
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Hu Tao
` (3 preceding siblings ...)
2014-06-14 4:48 ` [Qemu-devel] [PATCH RFC 4/4] memory-backend-file: error out " Hu Tao
@ 2014-06-15 10:00 ` Michael S. Tsirkin
2014-06-16 6:29 ` Hu Tao
4 siblings, 1 reply; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-15 10:00 UTC (permalink / raw)
To: Hu Tao; +Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Sat, Jun 14, 2014 at 12:48:55PM +0800, Hu Tao wrote:
> Michael,
>
> This is fixes for your pci tree.
>
> patch 1 remove signed range as requested.
This also fixes make check failures so I applied this.
Others don't look like regressions to me -
this is error handling in new functionality, correct?
Thus I'll wait for comments on these to be resolved,
and hopefully for some acks.
> There are 3 problems in current pci tree, as follows:
>
> 1. pc-dimm specified on command line but only -m size (aka not -m size,maxmem,slots)
>
> ./x86_64-softmmu/qemu-system-x86_64 -hda
> /home/data/libvirt-images/f18.img -smp 2 -object
> memory-backend-ram,size=512M,id=ram-node0,prealloc=y,policy=bind,host-nodes=0
> -device pc-dimm,id=d0,memdev=ram-node0 -m 640M -qmp
> unix:/tmp/m,server,nowait -monitor stdio -enable-kvm
>
> result:
>
> qemu/hw/mem/pc-dimm.c:110: pc_dimm_get_free_addr: Assertion
> `address_space_end > address_space_size' failed.
> Aborted (core dumped)
>
> patch 2 fixes this.
>
> 2. using qemu monitor command object-add to add a memory-backend-ram
> object whose's size is too big
>
> ./x86_64-softmmu/qemu-system-x86_64 -hda
> /home/data/libvirt-images/f18.img -smp 2 -m 512M -qmp
> unix:/tmp/m,server,nowait -monitor stdio -enable-kvm
>
> in monitor:
> (qemu)object_add memory-backend-ram,size=40960G,id=mem0
>
> result:
>
> qemu just exits with message: Cannot set up guest memory 'mem0': Cannot allocate memory
>
> patch 3 fixes this.
>
> 3. specifying a non-existing directory for memory-backend-file
>
> ./x86_64-softmmu/qemu-system-x86_64 -hda
> /home/data/libvirt-images/f18.img -smp 2 -m 512M,maxmem=1000G,slots=100
> -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object
> memory-backend-file,size=512M,id=mem0,mem-path=/nonexistingdir -device
> pc-dimm,id=d0,memdev=mem0
>
> result:
>
> /nonexistingdir: No such file or directory
> Bad ram offset fffffffffffff000
> Aborted (core dumped)
>
> patch 4 fixes this.
>
>
> please review. Thanks!
>
>
> Hu Tao (4):
> get rid of signed range
> check if we have space left for hotplugged memory
> exec: don't exit unconditionally if failed to allocate memory
> memory-backend-file: error out if failed to allocate memory
>
> backends/hostmem-file.c | 3 +
> backends/hostmem-ram.c | 3 +
> exec.c | 6 +-
> hw/mem/pc-dimm.c | 7 +-
> include/qemu/range.h | 144 ++++++++++++-------------------------
> qapi/string-input-visitor.c | 116 +++++++++++++++++-------------
> qapi/string-output-visitor.c | 97 +++++++++++++------------
> tests/test-string-input-visitor.c | 4 +-
> tests/test-string-output-visitor.c | 8 +--
> 9 files changed, 182 insertions(+), 206 deletions(-)
>
> --
> 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree
2014-06-15 10:00 ` [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree Michael S. Tsirkin
@ 2014-06-16 6:29 ` Hu Tao
2014-06-16 7:04 ` Michael S. Tsirkin
0 siblings, 1 reply; 47+ messages in thread
From: Hu Tao @ 2014-06-16 6:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Sun, Jun 15, 2014 at 01:00:56PM +0300, Michael S. Tsirkin wrote:
> On Sat, Jun 14, 2014 at 12:48:55PM +0800, Hu Tao wrote:
> > Michael,
> >
> > This is fixes for your pci tree.
> >
> > patch 1 remove signed range as requested.
>
> This also fixes make check failures so I applied this.
>
> Others don't look like regressions to me -
> this is error handling in new functionality, correct?
Yes.
BTW, thre are two more problems:
1. if numa node number doesn't start from 0 then qemu will core dump.
cmd line:
./x86_64-softmmu/qemu-system-x86_64 -hda
/home/data/libvirt-images/f18.img -m 128M,maxmem=2G,slots=3 -qmp
unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object
memory-backend-ram,id=m1,size=128M -numa node,nodeid=1,cpus=1,memdev=m1
This problem can be fixed by:
diff --git a/numa.c b/numa.c
index ce9382d..b00c5cf 100644
--- a/numa.c
+++ b/numa.c
@@ -270,10 +270,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Obj
}
memory_region_init(mr, owner, name, ram_size);
- for (i = 0; i < nb_numa_nodes; i++) {
+ for (i = 0; i < MAX_NODES; i++) {
Error *local_err = NULL;
uint64_t size = numa_info[i].node_mem;
HostMemoryBackend *backend = numa_info[i].node_memdev;
+ if (!backend) {
+ continue;
+ }
MemoryRegion *seg = host_memory_backend_get_memory(backend, &local_err);
if (local_err) {
qerror_report_err(local_err);
2. your current pci tree doesn't compile because patch 'qmp: add query-memdev' is dropped
while commit 5b517e74ed7825(hmp: add info memdev) depends on it.
but patch 'qmp: add query-memdev' itself has a problem: if memory-backend-ram is on the command
line but no numa, info memdev returns nothing:
./x86_64-softmmu/qemu-system-x86_64 -hda /home/data/libvirt-images/f18.img -m 128M,maxmem=2G,slots=3 -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object memory-backend-ram,id=m1,size=128M
QEMU 2.0.50 monitor - type 'help' for more information
(qemu) info memdev
(nothing returned)
even worse, if with -numa mem=size then qemu will core dump:
./x86_64-softmmu/qemu-system-x86_64 -hda /home/data/libvirt-images/f18.img -m 128M,maxmem=2G,slots=3 -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object memory-backend-ram,id=m1,size=128M -numa node,nodeid=0,cpus=0,mem=128M
(qemu) info memdev
Segmentation fault (core dumped)
this is because query_memdev searchs for memdev information in numa_info[].node_memdev,
which don't have value if there is no numa or numa isn't used with memory-backend-ram.
the solution can be gather memdevs in a list. Or query at /objects?
> Thus I'll wait for comments on these to be resolved,
> and hopefully for some acks.
>
> > There are 3 problems in current pci tree, as follows:
> >
> > 1. pc-dimm specified on command line but only -m size (aka not -m size,maxmem,slots)
> >
> > ./x86_64-softmmu/qemu-system-x86_64 -hda
> > /home/data/libvirt-images/f18.img -smp 2 -object
> > memory-backend-ram,size=512M,id=ram-node0,prealloc=y,policy=bind,host-nodes=0
> > -device pc-dimm,id=d0,memdev=ram-node0 -m 640M -qmp
> > unix:/tmp/m,server,nowait -monitor stdio -enable-kvm
> >
> > result:
> >
> > qemu/hw/mem/pc-dimm.c:110: pc_dimm_get_free_addr: Assertion
> > `address_space_end > address_space_size' failed.
> > Aborted (core dumped)
> >
> > patch 2 fixes this.
> >
> > 2. using qemu monitor command object-add to add a memory-backend-ram
> > object whose's size is too big
> >
> > ./x86_64-softmmu/qemu-system-x86_64 -hda
> > /home/data/libvirt-images/f18.img -smp 2 -m 512M -qmp
> > unix:/tmp/m,server,nowait -monitor stdio -enable-kvm
> >
> > in monitor:
> > (qemu)object_add memory-backend-ram,size=40960G,id=mem0
> >
> > result:
> >
> > qemu just exits with message: Cannot set up guest memory 'mem0': Cannot allocate memory
> >
> > patch 3 fixes this.
> >
> > 3. specifying a non-existing directory for memory-backend-file
> >
> > ./x86_64-softmmu/qemu-system-x86_64 -hda
> > /home/data/libvirt-images/f18.img -smp 2 -m 512M,maxmem=1000G,slots=100
> > -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object
> > memory-backend-file,size=512M,id=mem0,mem-path=/nonexistingdir -device
> > pc-dimm,id=d0,memdev=mem0
> >
> > result:
> >
> > /nonexistingdir: No such file or directory
> > Bad ram offset fffffffffffff000
> > Aborted (core dumped)
> >
> > patch 4 fixes this.
> >
> >
> > please review. Thanks!
> >
> >
> > Hu Tao (4):
> > get rid of signed range
> > check if we have space left for hotplugged memory
> > exec: don't exit unconditionally if failed to allocate memory
> > memory-backend-file: error out if failed to allocate memory
> >
> > backends/hostmem-file.c | 3 +
> > backends/hostmem-ram.c | 3 +
> > exec.c | 6 +-
> > hw/mem/pc-dimm.c | 7 +-
> > include/qemu/range.h | 144 ++++++++++++-------------------------
> > qapi/string-input-visitor.c | 116 +++++++++++++++++-------------
> > qapi/string-output-visitor.c | 97 +++++++++++++------------
> > tests/test-string-input-visitor.c | 4 +-
> > tests/test-string-output-visitor.c | 8 +--
> > 9 files changed, 182 insertions(+), 206 deletions(-)
> >
> > --
> > 1.9.3
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree
2014-06-16 6:29 ` Hu Tao
@ 2014-06-16 7:04 ` Michael S. Tsirkin
2014-06-16 8:28 ` Hu Tao
0 siblings, 1 reply; 47+ messages in thread
From: Michael S. Tsirkin @ 2014-06-16 7:04 UTC (permalink / raw)
To: Hu Tao; +Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Mon, Jun 16, 2014 at 02:29:08PM +0800, Hu Tao wrote:
> On Sun, Jun 15, 2014 at 01:00:56PM +0300, Michael S. Tsirkin wrote:
> > On Sat, Jun 14, 2014 at 12:48:55PM +0800, Hu Tao wrote:
> > > Michael,
> > >
> > > This is fixes for your pci tree.
> > >
> > > patch 1 remove signed range as requested.
> >
> > This also fixes make check failures so I applied this.
> >
> > Others don't look like regressions to me -
> > this is error handling in new functionality, correct?
>
> Yes.
>
>
> BTW, thre are two more problems:
>
> 1. if numa node number doesn't start from 0 then qemu will core dump.
>
> cmd line:
>
> ./x86_64-softmmu/qemu-system-x86_64 -hda
> /home/data/libvirt-images/f18.img -m 128M,maxmem=2G,slots=3 -qmp
> unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object
> memory-backend-ram,id=m1,size=128M -numa node,nodeid=1,cpus=1,memdev=m1
>
> This problem can be fixed by:
>
> diff --git a/numa.c b/numa.c
> index ce9382d..b00c5cf 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -270,10 +270,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Obj
> }
>
> memory_region_init(mr, owner, name, ram_size);
> - for (i = 0; i < nb_numa_nodes; i++) {
> + for (i = 0; i < MAX_NODES; i++) {
> Error *local_err = NULL;
> uint64_t size = numa_info[i].node_mem;
> HostMemoryBackend *backend = numa_info[i].node_memdev;
> + if (!backend) {
> + continue;
> + }
> MemoryRegion *seg = host_memory_backend_get_memory(backend, &local_err);
> if (local_err) {
> qerror_report_err(local_err);
>
> 2. your current pci tree doesn't compile because patch 'qmp: add query-memdev' is dropped
> while commit 5b517e74ed7825(hmp: add info memdev) depends on it.
Hmm it builds for me.
Does it only build if some library is present?
> but patch 'qmp: add query-memdev' itself has a problem: if memory-backend-ram is on the command
> line but no numa, info memdev returns nothing:
>
> ./x86_64-softmmu/qemu-system-x86_64 -hda /home/data/libvirt-images/f18.img -m 128M,maxmem=2G,slots=3 -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object memory-backend-ram,id=m1,size=128M
> QEMU 2.0.50 monitor - type 'help' for more information
> (qemu) info memdev
> (nothing returned)
>
> even worse, if with -numa mem=size then qemu will core dump:
>
> ./x86_64-softmmu/qemu-system-x86_64 -hda /home/data/libvirt-images/f18.img -m 128M,maxmem=2G,slots=3 -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object memory-backend-ram,id=m1,size=128M -numa node,nodeid=0,cpus=0,mem=128M
> (qemu) info memdev
> Segmentation fault (core dumped)
>
> this is because query_memdev searchs for memdev information in numa_info[].node_memdev,
> which don't have value if there is no numa or numa isn't used with memory-backend-ram.
>
> the solution can be gather memdevs in a list. Or query at /objects?
query QOM seems nicer.
> > Thus I'll wait for comments on these to be resolved,
> > and hopefully for some acks.
> >
> > > There are 3 problems in current pci tree, as follows:
> > >
> > > 1. pc-dimm specified on command line but only -m size (aka not -m size,maxmem,slots)
> > >
> > > ./x86_64-softmmu/qemu-system-x86_64 -hda
> > > /home/data/libvirt-images/f18.img -smp 2 -object
> > > memory-backend-ram,size=512M,id=ram-node0,prealloc=y,policy=bind,host-nodes=0
> > > -device pc-dimm,id=d0,memdev=ram-node0 -m 640M -qmp
> > > unix:/tmp/m,server,nowait -monitor stdio -enable-kvm
> > >
> > > result:
> > >
> > > qemu/hw/mem/pc-dimm.c:110: pc_dimm_get_free_addr: Assertion
> > > `address_space_end > address_space_size' failed.
> > > Aborted (core dumped)
> > >
> > > patch 2 fixes this.
> > >
> > > 2. using qemu monitor command object-add to add a memory-backend-ram
> > > object whose's size is too big
> > >
> > > ./x86_64-softmmu/qemu-system-x86_64 -hda
> > > /home/data/libvirt-images/f18.img -smp 2 -m 512M -qmp
> > > unix:/tmp/m,server,nowait -monitor stdio -enable-kvm
> > >
> > > in monitor:
> > > (qemu)object_add memory-backend-ram,size=40960G,id=mem0
> > >
> > > result:
> > >
> > > qemu just exits with message: Cannot set up guest memory 'mem0': Cannot allocate memory
> > >
> > > patch 3 fixes this.
> > >
> > > 3. specifying a non-existing directory for memory-backend-file
> > >
> > > ./x86_64-softmmu/qemu-system-x86_64 -hda
> > > /home/data/libvirt-images/f18.img -smp 2 -m 512M,maxmem=1000G,slots=100
> > > -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object
> > > memory-backend-file,size=512M,id=mem0,mem-path=/nonexistingdir -device
> > > pc-dimm,id=d0,memdev=mem0
> > >
> > > result:
> > >
> > > /nonexistingdir: No such file or directory
> > > Bad ram offset fffffffffffff000
> > > Aborted (core dumped)
> > >
> > > patch 4 fixes this.
> > >
> > >
> > > please review. Thanks!
> > >
> > >
> > > Hu Tao (4):
> > > get rid of signed range
> > > check if we have space left for hotplugged memory
> > > exec: don't exit unconditionally if failed to allocate memory
> > > memory-backend-file: error out if failed to allocate memory
> > >
> > > backends/hostmem-file.c | 3 +
> > > backends/hostmem-ram.c | 3 +
> > > exec.c | 6 +-
> > > hw/mem/pc-dimm.c | 7 +-
> > > include/qemu/range.h | 144 ++++++++++++-------------------------
> > > qapi/string-input-visitor.c | 116 +++++++++++++++++-------------
> > > qapi/string-output-visitor.c | 97 +++++++++++++------------
> > > tests/test-string-input-visitor.c | 4 +-
> > > tests/test-string-output-visitor.c | 8 +--
> > > 9 files changed, 182 insertions(+), 206 deletions(-)
> > >
> > > --
> > > 1.9.3
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [Qemu-devel] [PATCH RFC 0/4] fixes for pci tree
2014-06-16 7:04 ` Michael S. Tsirkin
@ 2014-06-16 8:28 ` Hu Tao
0 siblings, 0 replies; 47+ messages in thread
From: Hu Tao @ 2014-06-16 8:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Igor Mammedov, qemu-devel, Yasunori Goto
On Mon, Jun 16, 2014 at 10:04:21AM +0300, Michael S. Tsirkin wrote:
> On Mon, Jun 16, 2014 at 02:29:08PM +0800, Hu Tao wrote:
> > On Sun, Jun 15, 2014 at 01:00:56PM +0300, Michael S. Tsirkin wrote:
> > > On Sat, Jun 14, 2014 at 12:48:55PM +0800, Hu Tao wrote:
> > > > Michael,
> > > >
> > > > This is fixes for your pci tree.
> > > >
> > > > patch 1 remove signed range as requested.
> > >
> > > This also fixes make check failures so I applied this.
> > >
> > > Others don't look like regressions to me -
> > > this is error handling in new functionality, correct?
> >
> > Yes.
> >
> >
> > BTW, thre are two more problems:
> >
> > 1. if numa node number doesn't start from 0 then qemu will core dump.
> >
> > cmd line:
> >
> > ./x86_64-softmmu/qemu-system-x86_64 -hda
> > /home/data/libvirt-images/f18.img -m 128M,maxmem=2G,slots=3 -qmp
> > unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object
> > memory-backend-ram,id=m1,size=128M -numa node,nodeid=1,cpus=1,memdev=m1
> >
> > This problem can be fixed by:
> >
> > diff --git a/numa.c b/numa.c
> > index ce9382d..b00c5cf 100644
> > --- a/numa.c
> > +++ b/numa.c
> > @@ -270,10 +270,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Obj
> > }
> >
> > memory_region_init(mr, owner, name, ram_size);
> > - for (i = 0; i < nb_numa_nodes; i++) {
> > + for (i = 0; i < MAX_NODES; i++) {
> > Error *local_err = NULL;
> > uint64_t size = numa_info[i].node_mem;
> > HostMemoryBackend *backend = numa_info[i].node_memdev;
> > + if (!backend) {
> > + continue;
> > + }
> > MemoryRegion *seg = host_memory_backend_get_memory(backend, &local_err);
> > if (local_err) {
> > qerror_report_err(local_err);
> >
> > 2. your current pci tree doesn't compile because patch 'qmp: add query-memdev' is dropped
> > while commit 5b517e74ed7825(hmp: add info memdev) depends on it.
>
> Hmm it builds for me.
> Does it only build if some library is present?
No, it always builds.
Failed to build both on Fedora 17 and 20. My commandline to build:
../configure --target-list=x86_64-softmmu && make -j4
These are the error messages:
/mnt/data/kernel/qemu/hmp.c: In function ‘hmp_info_memdev’:
/mnt/data/kernel/qemu/hmp.c:1685:5: error: unknown type name ‘MemdevList’
/mnt/data/kernel/qemu/hmp.c:1685:5: error: implicit declaration of function ‘qmp_query_memdev’ [-Werror=implicit-function-declaration]
/mnt/data/kernel/qemu/hmp.c:1685:5: error: nested extern declaration of ‘qmp_query_memdev’ [-Werror=nested-externs]
/mnt/data/kernel/qemu/hmp.c:1685:31: error: initialization makes pointer from integer without a cast [-Werror]
/mnt/data/kernel/qemu/hmp.c:1686:5: error: unknown type name ‘MemdevList’
/mnt/data/kernel/qemu/hmp.c:1694:33: error: request for member ‘value’ in something not a structure or union
/mnt/data/kernel/qemu/hmp.c:1696:56: error: request for member ‘value’ in something not a structure or union
/mnt/data/kernel/qemu/hmp.c:1698:25: error: request for member ‘value’ in something not a structure or union
/mnt/data/kernel/qemu/hmp.c:1700:25: error: request for member ‘value’ in something not a structure or union
/mnt/data/kernel/qemu/hmp.c:1702:25: error: request for member ‘value’ in something not a structure or union
/mnt/data/kernel/qemu/hmp.c:1704:46: error: request for member ‘value’ in something not a structure or union
/mnt/data/kernel/qemu/hmp.c:1709:14: error: request for member ‘next’ in something not a structure or union
cc1: all warnings being treated as errors
make: *** [hmp.o] Error 1
^ permalink raw reply [flat|nested] 47+ messages in thread