fixes for pci tree, v2
v2 cover fixes for pci tree, v2
Michael,
This is fixes for your pci tree v2. please review. thanks!
changes to v1:
1. adds the missing query-memdev and change into querying qom for memdevs(patch 1)
2. tweak error message (patch 2)
3. improve error handling for memory-backend-file (patch 3)
4. fixup the problem that nodes not start from 0 (patch 4)
Hu Tao (4):
qmp: add query-memdev
check if we have space left for hotplugged memory
memory-backend-file: improve error handling fixup! numa: add -numa node,memdev= option
exec.c | 26 ++++++++++++----- hw/mem/pc-dimm.c | 7 +++++ numa.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- qapi-schema.json | 40 +++++++++++++++++++++++++ qmp-commands.hx | 38 ++++++++++++++++++++++++ 5 files changed, 191 insertions(+), 9 deletions(-)
Signature · 1 line
1.9.3
PATCH v2 1/4 qmp: add query-memdev
Add qmp command query-memdev to query for information of memory devices
--- numa.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 40 +++++++++++++++++++++++++++ qmp-commands.hx | 38 +++++++++++++++++++++++++ 3 files changed, 162 insertions(+)
numa.c +84 -0
@@ -34,6 +34,7 @@#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 +281,86 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, addr += size; } } + +static int query_memdev (Object *obj,void *opaque) +{ +MemdevList **list = opaque; +Error *err =NULL ; + +if (object_dynamic_cast (obj,TYPE_MEMORY_BACKEND )) { +MemdevList *m =g_malloc0 (sizeof (*m)); + + m->value =g_malloc0 (sizeof (*m->value)); + + m->value->size =object_property_get_int (obj,"size" , + &err); +if (err) { + goto error; + } + + m->value->merge =object_property_get_bool (obj,"merge" , + &err); +if (err) { + goto error; + } + + m->value->dump =object_property_get_bool (obj,"dump" , + &err); +if (err) { + goto error; + } + + m->value->prealloc =object_property_get_bool (obj, +"prealloc" , &err); +if (err) { + goto error; + } + + m->value->policy =object_property_get_enum (obj, +"policy" , + HostMemPolicy_lookup, + &err); +if (err) { + goto error; + } + +object_property_get_uint16List (obj,"host-nodes" , + &m->value->host_nodes, &err); +if (err) { + goto error; + } + + m->next = *list; + *list = m; + } + +return 0 ; +error: +return -1 ; +} + +MemdevList *qmp_query_memdev (Error **errp) +{ +Object *obj; +MemdevList *list =NULL , *m; + + obj =object_resolve_path ("/objects" ,NULL ); +if (obj ==NULL ) { +return NULL ; + } + +if (object_child_foreach (obj, query_memdev, &list) !=0 ) { + goto error; + } + +return list; + +error: +while (list) { + m = list; + list = list->next; +g_free (m->value); +g_free (m); + } +return NULL ; +}
qapi-schema.json +40 -0
@@ -4796,3 +4796,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'] }
qmp-commands.hx +38 -0
@@ -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
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 1/4] qmp: add query-memdev
Quoted 23 lines · Hu Tao
Add qmp command query-memdev to query for information of memory devices
Signed-off-by Hu Tao <[email protected]>--- numa.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 40 +++++++++++++++++++++++++++ qmp-commands.hx | 38 +++++++++++++++++++++++++ 3 files changed, 162 insertions(+)numa.c +1
-0@@ -34,6 +34,7 @@#include "qapi/qmp/qerror.h" #include "hw/boards.h" #include "sysemu/hostmem.h" +#include "qmp-commands.h" QemuOptsList qemu_numa_opts = { .name ="numa" ,
numa.c void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, · line 288
Quoted 8 lines
numa.c +5
-0@@ -280,3 +281,86 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, addr += size; } } + +static int query_memdev (Object *obj,void *opaque) +{ +MemdevList **list = opaque; +Error *err =NULL ;
This variable is usually named local_err.
numa.c void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, · line 299
Quoted 11 lines
numa.c +11
-0+ +if (object_dynamic_cast (obj,TYPE_MEMORY_BACKEND )) { +MemdevList *m =g_malloc0 (sizeof (*m)); + + m->value =g_malloc0 (sizeof (*m->value)); + + m->value->size =object_property_get_int (obj,"size" , + &err); +if (err) { + goto error; + }
Is this error ever likely, or would it represent a programming error, in which case passing &error_abort would avoid the need to check for failure afterwords. Similarly for all the other properties.
numa.c void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, · line 303
Quoted 4 lines
numa.c +4
-0+query-memdev +------------ + +Show memory devices information.
Makes sense, but took me a couple reads. Might be better as:
Show information on memory devices.
numa.c void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, · line 306
Quoted 3 lines
numa.c +3
-0+ + +Example (1):
What is the "(1)" for? I'd just drop it (probably copy-and-paste from another command that actually had more than one example)
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH v2 2/4 check if we have space left for hotplugged memory
In reply to Hu TaoIf 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.
--- hw/mem/pc-dimm.c | 7 +++++++ 1 file changed, 7 insertions(+)
hw/mem/pc-dimm.c +7 -0
@@ -107,6 +107,13 @@ 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; +if (address_space_size ==0 ) { +error_setg (errp,"no space left for hotplugged memory. did you forget " +"maxmem and slots on " +"-m(aka -m size,maxmem=maxmem,slots=slots)?" ); + goto out; + } +assert (address_space_end > address_space_size);object_child_foreach (qdev_get_machine (), pc_dimm_built_list, &list);
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 2/4] check if we have space left for hotplugged memory
Quoted 15 lines · On Mon, 16 Jun 2014 18:05:42 +0800 Hu Tao <[email protected]> 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 <[email protected]>--- hw/mem/pc-dimm.c | 7 +++++++ 1 file changed, 7 insertions(+)hw/mem/pc-dimm.c +0
-0
hw/mem/pc-dimm.c uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, · line 113
Quoted 7 lines
hw/mem/pc-dimm.c +4
-0@@ -107,6 +107,13 @@ 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; +if (address_space_size ==0 ) { +error_setg (errp,"no space left for hotplugged memory. did you forget " +"maxmem and slots on " +"-m(aka -m size,maxmem=maxmem,slots=slots)?" );
CLI format here could get out off sync from actual one. Maybe just "no space left in hotplug memory range"
Quoted 6 lines
hw/mem/pc-dimm.c +3
-0+ goto out; + } +assert (address_space_end > address_space_size);object_child_foreach (qdev_get_machine (), pc_dimm_built_list, &list);
Re: [Qemu-devel] [PATCH v2 2/4] check if we have space left for hotplugged memory
In reply to Hu Taohw/mem/pc-dimm.c uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
Quoted 15 lines · On Mon, 16 Jun 2014 18:05:42 +0800 Hu Tao <[email protected]> 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 <[email protected]>--- hw/mem/pc-dimm.c | 7 +++++++ 1 file changed, 7 insertions(+)hw/mem/pc-dimm.c +0
-0@@ -107,6 +107,13 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
another suggestion is to put check in pc_dimm_plug() hotplug handler.
Quoted 13 lines
hw/mem/pc-dimm.c +7
-0uint64_t new_addr, ret =0 ;uint64_t address_space_end = address_space_start + address_space_size; +if (address_space_size ==0 ) { +error_setg (errp,"no space left for hotplugged memory. did you forget " +"maxmem and slots on " +"-m(aka -m size,maxmem=maxmem,slots=slots)?" ); + goto out; + } +assert (address_space_end > address_space_size);object_child_foreach (qdev_get_machine (), pc_dimm_built_list, &list);
PATCH v2 3/4 memory-backend-file: improve error handling
In reply to Hu TaoThis patch fixes two problems of memory-backend-file:
1. If user adds a memory-backend-file object using object_add command, specifying a non-existing directory for property mem-path, qemu will core dump with message:
/nonexistingdir: No such file or directory
Bad ram offset fffffffffffff000
Aborted (core dumped)
2. If user adds a memory-backend-file object using object_add command, specifying a size that is less than huge page size, qemu will core dump with message:
Bad ram offset fffffffffffff000
Aborted (core dumped)
--- exec.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
exec.c +18 -8
@@ -997,7 +997,7 @@ void qemu_mutex_unlock_ramlist(void)#define HUGETLBFS_MAGIC 0x958458f6 -static long gethugepagesize (const char *path) +static long gethugepagesize (const char *path,Error **errp) {struct statfs fs;int ret;
@@ -1007,7 +1007,7 @@ static long gethugepagesize(const char *path) }while (ret !=0 && errno ==EINTR );if (ret !=0 ) { -perror (path); +error_setg_errno (errp, errno,"failed to stat file %s" , path);return 0 ; }
@@ -1025,17 +1025,19 @@ static void *file_ram_alloc(RAMBlock *block,char *filename;char *sanitized_name;char *c; -void *area; +void *area =NULL ;int fd;unsigned long hpagesize; - hpagesize =gethugepagesize (path); + hpagesize =gethugepagesize (path, errp);if (!hpagesize) { goto error; }if (memory < hpagesize) { -return NULL ; +error_setg (errp,"memory size " RAM_ADDR_FMT " should be larger " +"than huge page size %" PRIx64, memory, hpagesize); + goto error; }if (kvm_enabled () && !kvm_has_sync_mmu ()) {
@@ -1095,8 +1097,8 @@ static void *file_ram_alloc(RAMBlock *block,return area; error: -if (mem_prealloc) { -exit (1 ); +if (area && area !=MAP_FAILED ) { +munmap (area, memory); }return NULL ; }
@@ -1279,6 +1281,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, Error **errp) {RAMBlock *new_block; +ram_addr_t addr;if (xen_enabled ()) {error_setg (errp,"-mem-path not supported with Xen\n" );
@@ -1308,7 +1311,14 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, return -1; } -return ram_block_add (new_block); + addr =ram_block_add (new_block); +if (addr ==-1 ) { +g_free (new_block); +error_setg (errp,"failed to allocate memory\n" ); +return -1 ; + } + +return addr; }#endif
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 3/4] memory-backend-file: improve error handling
Quoted 26 lines · On Mon, 16 Jun 2014 18:05:43 +0800 Hu Tao <[email protected]> wrote:
This patch fixes two problems of memory-backend-file:
1. If user adds a memory-backend-file object using
object_addcommand, specifying a non-existing directory for property mem-path, qemu will core dump with message:/nonexistingdir: No such file or directory
Bad ram offsetfffffffffffff000
Aborted (core dumped)2. If user adds a memory-backend-file object using
object_addcommand, specifying a size that is less than huge page size, qemu will core dump with message:Bad ram offset
fffffffffffff000
Aborted (core dumped)Signed-off-by Hu Tao <[email protected]>--- exec.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)exec.c +0
-0
[...]
exec.c ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, · line 1315
Quoted 7 lines
exec.c +2
-1@@ -1308,7 +1311,14 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,return -1 ; } -return ram_block_add (new_block); + addr =ram_block_add (new_block); +if (addr ==-1 ) {
how this hunk is relevant to commit message?
and more important in what case ram_block_add() returns -1?
Quoted 9 lines
exec.c +6
-0+g_free (new_block); +error_setg (errp,"failed to allocate memory\n" ); + return -1; + } + +return addr; }#endif
Re: [Qemu-devel] [PATCH v2 3/4] memory-backend-file: improve error handling
Quoted 34 lines · Igor Mammedov
Quoted 26 lines · On Mon, 16 Jun 2014 18:05:43 +0800 Hu Tao <[email protected]> wrote:
This patch fixes two problems of memory-backend-file:
1. If user adds a memory-backend-file object using
object_addcommand, specifying a non-existing directory for property mem-path, qemu will core dump with message:/nonexistingdir: No such file or directory
Bad ram offsetfffffffffffff000
Aborted (core dumped)2. If user adds a memory-backend-file object using
object_addcommand, specifying a size that is less than huge page size, qemu will core dump with message:Bad ram offset
fffffffffffff000
Aborted (core dumped)Signed-off-by Hu Tao <[email protected]>--- exec.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)exec.c +0
-0[...]
Quoted 4 lines
exec.c +0
-0@@ -1308,7 +1311,14 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,return -1 ; }
Quoted 6 lines
exec.c ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, · line 1315
Quoted 3 lines
exec.c +2
-1-return ram_block_add (new_block); + addr =ram_block_add (new_block); +if (addr ==-1 ) {how this hunk is relevant to commit message?
and more important in what case
ram_block_add()returns -1?
See patch 03 int v1(titled [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory) It is not included in this version but we'll fix it after the merge.
Quoted 10 lines
Quoted 10 lines
exec.c +6
-0+g_free (new_block); +error_setg (errp,"failed to allocate memory\n" ); + return -1; + } + +return addr; }#endif
Re: [Qemu-devel] [PATCH v2 3/4] memory-backend-file: improve error handling
Quoted 40 lines · Hu Tao
Quoted 39 lines · Igor Mammedov
Quoted 26 lines · On Mon, 16 Jun 2014 18:05:43 +0800 Hu Tao <[email protected]> wrote:
This patch fixes two problems of memory-backend-file:
1. If user adds a memory-backend-file object using
object_addcommand, specifying a non-existing directory for property mem-path, qemu will core dump with message:/nonexistingdir: No such file or directory
Bad ram offsetfffffffffffff000
Aborted (core dumped)2. If user adds a memory-backend-file object using
object_addcommand, specifying a size that is less than huge page size, qemu will core dump with message:Bad ram offset
fffffffffffff000
Aborted (core dumped)Signed-off-by Hu Tao <[email protected]>--- exec.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)exec.c +0
-0[...]
exec.c ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, · line 1315
Quoted 7 lines
exec.c +2
-1@@ -1308,7 +1311,14 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,return -1 ; } -return ram_block_add (new_block); + addr =ram_block_add (new_block); +if (addr ==-1 ) {how this hunk is relevant to commit message?
Quoted 4 lines
Quoted 1 line
and more important in what case
ram_block_add()returns -1?See patch 03 int
v1(titled [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory)It is not included in this version but we'll fix it after the merge.
So maybe defer this hunk to after the merge too.
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
exec.c +6
-0+g_free (new_block); +error_setg (errp,"failed to allocate memory\n" ); + return -1; + } + +return addr; }#endif
PATCH v2 4/4 fixup! numa: add -numa node, memdev= option
In reply to Hu Taonote: this is a fixup that can be squashed into commit 0886aa7db0e4.
if numa node number doesn't start from 0 then qemu will core dump.
cmd line to reproduce:
./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
--- numa.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
numa.c +4 -1
@@ -266,10 +266,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, }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);
Signature · 1 line
1.9.3
Quoted 3 lines · Hu Tao
Michael,
This is fixes for your pci tree v2. please review. thanks!
Patches 1,4 applied. Thanks!
Quoted 22 lines
changes to v1:
1. adds the missing query-memdev and change into querying qom for memdevs(patch 1)
2. tweak error message (patch 2)
3. improve error handling for memory-backend-file (patch 3)
4. fixup the problem that nodes not start from 0 (patch 4)Hu Tao (4):
qmp: add query-memdev
check if we have space left for hotplugged memory
memory-backend-file: improve error handling fixup! numa: add -numa node,memdev= optionexec.c | 26 ++++++++++++----- hw/mem/pc-dimm.c | 7 +++++ numa.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- qapi-schema.json | 40 +++++++++++++++++++++++++ qmp-commands.hx | 38 ++++++++++++++++++++++++ 5 files changed, 191 insertions(+), 9 deletions(-)Signature · 1 line
1.9.3
Quoted 6 lines · Michael S. Tsirkin
Quoted 3 lines · Hu Tao
Michael,
This is fixes for your pci tree v2. please review. thanks!
Patches 1,4 applied. Thanks!
What about patches 2,3? If you need I can resend patch 2 based on Igor's
comments. About patch 3, I assume you leave it as it is not a
regression.
Quoted 23 lines
Quoted 23 lines
changes to v1:
1. adds the missing query-memdev and change into querying qom for memdevs(patch 1)
2. tweak error message (patch 2)
3. improve error handling for memory-backend-file (patch 3)
4. fixup the problem that nodes not start from 0 (patch 4)Hu Tao (4):
qmp: add query-memdev
check if we have space left for hotplugged memory
memory-backend-file: improve error handling fixup! numa: add -numa node,memdev= optionexec.c | 26 ++++++++++++----- hw/mem/pc-dimm.c | 7 +++++ numa.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- qapi-schema.json | 40 +++++++++++++++++++++++++ qmp-commands.hx | 38 ++++++++++++++++++++++++ 5 files changed, 191 insertions(+), 9 deletions(-)Signature · 1 line
1.9.3
Quoted 11 lines · Hu Tao
Quoted 6 lines · Michael S. Tsirkin
Quoted 3 lines · Hu Tao
Michael,
This is fixes for your pci tree v2. please review. thanks!
Patches 1,4 applied. Thanks!
What about patches 2,3? If you need I can resend patch 2 based on Igor's
comments. About patch 3, I assume you leave it as it is not a
regression.
Patch 2 isn't a regression either, right?
Please go ahead and repost patches 2,3 addressing comments.
They are unrelated so don't have to be in a series, even.
Quoted 23 lines
Quoted 23 lines
Quoted 23 lines
changes to v1:
1. adds the missing query-memdev and change into querying qom for memdevs(patch 1)
2. tweak error message (patch 2)
3. improve error handling for memory-backend-file (patch 3)
4. fixup the problem that nodes not start from 0 (patch 4)Hu Tao (4):
qmp: add query-memdev
check if we have space left for hotplugged memory
memory-backend-file: improve error handling fixup! numa: add -numa node,memdev= optionexec.c | 26 ++++++++++++----- hw/mem/pc-dimm.c | 7 +++++ numa.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- qapi-schema.json | 40 +++++++++++++++++++++++++ qmp-commands.hx | 38 ++++++++++++++++++++++++ 5 files changed, 191 insertions(+), 9 deletions(-)Signature · 1 line
1.9.3