Copying Igor and Eduardo for a hostmem.c bug. Search for "core dumped".
David Hildenbrand <da...@redhat.com> writes: >>> >>> Tests have to be fixed up: >>> - Two BUGs were hardcoded that are fixed now >>> - The string-input-visitor now actually returns a parsed list and not >>> an ordered set. >> >> I'd expect this to necessitate an update of callers that expect a set, but... >> >>> Signed-off-by: David Hildenbrand <da...@redhat.com> >>> --- >>> include/qapi/string-input-visitor.h | 4 +- >>> qapi/string-input-visitor.c | 410 ++++++++++++++++------------ >>> tests/test-string-input-visitor.c | 18 +- >>> 3 files changed, 239 insertions(+), 193 deletions(-) >> >> ... there's none. >> >> Let me know if you need help finding them. I think we tracked them down >> during the discussion that led to this series. >> > > Indeed, I missed to document that. So here is the outcome: > > 1. backends/hostmem.c:host_memory_backend_set_host_nodes() > > -> calls visit_type_uint16List(via bitmap) > -> the code can deal with duplicates/unsorted lists (bitmap_set) Yes. > Side node: I am not sure if there should be some range checks, but maybe > the bitmap is large enough .... hm ... Fishy. MAX_NODES is 128. Tinker, tinker, ... $ upstream-qemu -nodefaults -object memory-backend-file,id=mem0,mem-path=x,size=4096,host-nodes=12345 Segmentation fault (core dumped) Igor, Eduardo, this is yours. There's another use of visit_type_uint16List() is this file, but it's in property getter host_memory_backend_get_host_nodes(), and property getters aren't used with the string input visitor. > 2. qapi-visit.c::visit_type_Memdev_members() > > -> calls visit_type_uint16List() > -> I think this never used for input, only for output / freeing Yes, it's used by query-memdev with the QObject output visitor to build the value of @host-nodes. > 3. qapi-visit.c::visit_type_NumaNodeOptions_members() > > -> calls visit_type_uint16List() > -> I think this never used for input, only for output / freeing It's used for input, but with the opts visitor, see parse_numa(). > 4. qapi-visit.c::visit_type_RockerOfDpaGroup_members > > -> calls visit_type_uint32List() > -> I think this never used for input, only for output / freeing Yes, it's used by query-rocker-of-dpa-groups with the QObject output visitor to build the value of @group-ids. > 5. qapi-visit.c::visit_type_RxFilterInfo_members() > > -> calls visit_type_intList() > -> I think this never used for input, only for output / freeing Yes, it's used by query-rx-filter with the QObject output visitor to build the value of @vlan-table. > 6. numa.c::query_memdev() > > -> calls object_property_get_uint16List() > --> String parsed via visit_type_uint16List() into list QOM, hard to understand. The value of struct HostMemoryBackend member @host-nodes (a bitmap) is first converted to a list (sorted, no duplicates) with host_memory_backend_get_host_nodes() via object_property_get(), then converted to a string with the string output visitor. The resulting string is then converted back to a list with the string input visitor. Despite the shenanigans going on in the string output visitor, I'd expect the resulting list to also be sorted and without duplicates. > -> qmp_query_memdev() uses this list > --> Not relevant if unique or sorted Depends on the contract of QMP command query-memdev. Here's the relevant part. # @host-nodes: host nodes for its memory policy Useless. "Sorted, no duplicates" might have become de facto ABI. Not sure. However, I believe your patch won't affect it, as per the argument I just made. > -> hmp_info_memdev() uses this list > --> List converted again to a string using string output visitor > > -> I don't think unique/sorted is relevant here. HMP is not a stable interface. > Am I missing anything / is any of my statements wrong? Searching the QAPI schema for lists of integers coughs up block latency histogram stuff, but that's unrelated, as far as I can tell. Looks like we're good. I didn't expect that :) [...]