aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-04-30qapi: Generate simpler marshalling code when no argumentsMarkus Armbruster
When command FOO has no arguments, its generated qmp_marshal_FOO() is a bit confusing. Make it simpler: visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; } - - if (!err) { - visit_check_struct(v, &err); - } + visit_check_struct(v, &err); visit_end_struct(v, NULL); if (err) { goto out; } Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200424084338.26803-16-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30qapi: Disallow qmp_marshal_FOO(NULL, ...)Markus Armbruster
For QMP commands without arguments, gen_marshal() laboriously generates a qmp_marshal_FOO() that copes with null @args. Turns there's just one caller that passes null instead of an empty QDict. Adjust that caller, and simplify gen_marshal(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200424084338.26803-15-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30qom: Simplify object_property_get_enum()Markus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-14-armbru@redhat.com>
2020-04-30qapi: Only input visitors can actually failMarkus Armbruster
The previous few commits have made this more obvious, and removed the one exception. Time to clarify the documentation, and drop dead error checking. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-13-armbru@redhat.com>
2020-04-30qapi: Assert non-input visitors see only valid alternate tagsMarkus Armbruster
An alternate type's visit_type_FOO() fails when it runs into an invalid ->type. This is appropriate with an input visitor: visit_start_alternate() sets ->type according to the input, and bad input can lead to bad ->type. It should never happen with an output, clone or dealloc visitor: if it did, the alternate being output, cloned or deallocated would be messed up beyond repair. Assert that. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-12-armbru@redhat.com>
2020-04-30qapi: Clean up visitor's recovery from input with invalid typeMarkus Armbruster
An alternate type's visit_type_FOO() fails when it runs into an invalid ->type. If it's an input visit, we then need to free the the object we got from visit_start_alternate(). We do that with qapi_free_FOO(), which uses the dealloc visitor. Trouble is that object is in a bad state: its ->type is invalid. So the dealloc visitor will run into the same error again, and the error recovery skips deallocating the alternate's (invalid) alternative. Works, because qapi_free_FOO() ignores the error. Avoid it instead: free the messed up object with by g_free(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200424084338.26803-11-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30qapi: Assert non-input visitors see only valid narrow integersMarkus Armbruster
visit_type_intN() and visit_type_uintN() fail when the value is out of bounds. This is appropriate with an input visitor: the value comes from input, and input may be bad. It should never happen with the other visitors: the value comes from the caller, and callers must keep it within bounds. Assert that. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-10-armbru@redhat.com>
2020-04-30qapi: Assert output visitors see only valid enum valuesMarkus Armbruster
output_type_enum() fails when *obj is not a valid value of the enum type. Should not happen. Drop the check, along with its unit tests. This unmasks qapi_enum_lookup()'s assertion. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-9-armbru@redhat.com> [Commit message tweaked]
2020-04-30qapi: Fix Visitor contract for start_alternate()Markus Armbruster
The contract demands v->start_alternate() for input and dealloc visitors, but visit_start_alternate() actually requires it for input and clone visitors. Fix the contract, and delete superfluous qapi_dealloc_start_alternate(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-8-armbru@redhat.com>
2020-04-30qapi: Assert incomplete object occurs only in dealloc visitorMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-7-armbru@redhat.com>
2020-04-30qapi: Polish prose in visitor.hMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-6-armbru@redhat.com>
2020-04-30qapi: Document @errp usage more thoroughly in visitor.hMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-5-armbru@redhat.com>
2020-04-30qapi: Fix typo in visit_start_list()'s contractMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-4-armbru@redhat.com>
2020-04-30qapi: Fix the virtual walk example in visitor.h's big commentMarkus Armbruster
Call visit_check_list(). Missed in commit a4a1c70dc7 "qapi: Make input visitors detect unvisited list tails". Drop an irrelevant error_propagate() while there. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-3-armbru@redhat.com>
2020-04-30qapi: Belatedly update visitor.h's big comment for QAPI modulesMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200424084338.26803-2-armbru@redhat.com>
2020-04-30qemu-option: Clean up after the previous commitMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200415083048.14339-6-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()Markus Armbruster
qdict_iter() has just three uses and no test coverage. Replace by qdict_first(), qdict_next() for more concise code and less type punning. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200415083048.14339-5-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() insteadMarkus Armbruster
qlist_iter() has just three uses outside tests/. Replace by QLIST_FOREACH_ENTRY() for more concise code and less type punning. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200415083048.14339-4-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30qobject: Factor out helper json_pretty_newline()Markus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200415083048.14339-3-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Coding style in moved code tidied up]
2020-04-30qobject: Clean up QLIST_FOREACH_ENTRY()Markus Armbruster
QLIST_FOREACH_ENTRY() traverses a tail queue manually. Use QTAILQ_FIRST() and QTAILQ_NEXT() instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200415083048.14339-2-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-29Merge remote-tracking branch 'remotes/armbru/tags/pull-misc-2020-04-29' into ↵Peter Maydell
staging Miscellaneous patches for 2020-04-29 # gpg: Signature made Wed 29 Apr 2020 07:42:52 BST # gpg: using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653 # gpg: issuer "armbru@redhat.com" # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full] # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full] # Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653 * remotes/armbru/tags/pull-misc-2020-04-29: (32 commits) qemu-option: pass NULL rather than 0 to the id of qemu_opts_set() libqos: Give get_machine_allocator() internal linkage fuzz: Simplify how we compute available machines and types Makefile: Drop unused, broken target recurse-fuzz smbus: Fix spd_data_generate() for number of banks > 2 bamboo, sam460ex: Tidy up error message for unsupported RAM size smbus: Fix spd_data_generate() error API violation sam460ex: Suppress useless warning on -m 32 and -m 64 qga: Fix qmp_guest_suspend_{disk, ram}() error handling qga: Fix qmp_guest_get_memory_blocks() error handling tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffff migration/colo: Fix qmp_xen_colo_do_checkpoint() error handling io: Fix qio_channel_socket_close() error handling xen/pt: Fix flawed conversion to realize() virtio-net: Fix duplex=... and speed=... error handling bochs-display: Fix vgamem=SIZE error handling fdc: Fix fallback=auto error handling arm/virt: Fix virt_machine_device_plug_cb() error API violation cpus: Proper range-checking for -icount shift=N cpus: Fix configure_icount() error API violation ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-04-29Open 5.1 development treePeter Maydell
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-04-29qemu-option: pass NULL rather than 0 to the id of qemu_opts_set()Masahiro Yamada
The second argument 'id' is a pointer. Pass NULL rather than 0. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Message-Id: <20200427005704.2475782-1-masahiroy@kernel.org> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-04-29libqos: Give get_machine_allocator() internal linkageMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200424071142.3525-4-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-04-29fuzz: Simplify how we compute available machines and typesMarkus Armbruster
apply_to_qlist(), apply_to_node() work with QObjects. This is designed for use by tests/qtest/qos-test.c, which gets the data in that form via QMP. Goes back to commit fc281c8020 "tests: qgraph API for the qtest driver framework". Commit 275ab39d86 "fuzz: add support for qos-assisted fuzz targets" added another user: qtest/fuzz/qos_fuzz.c. To get the data as QObjects, it uses qmp_marshal_query_machines() and qmp_marshal_qom_list_types(). All this code is rather cumbersome. Switch to working with generated QAPI types instead: * Replace apply_to_qlist() & friends by machines_apply_to_node() and types_apply_to_node(). * Have qos_fuzz.c use qmp_query_machines() and qmp_qom_list_types() instead. * Have qos_test.c convert from QObject to the QAPI types. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200424071142.3525-3-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
2020-04-29Makefile: Drop unused, broken target recurse-fuzzMarkus Armbruster
Target recurse-fuzz depends on pc-bios/optionrom/fuzz, which can't be made. It's not used anywhere. Added in commit c621dc3e01c, looks like cargo cult. Delete. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200424071142.3525-2-armbru@redhat.com> Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
2020-04-29smbus: Fix spd_data_generate() for number of banks > 2Markus Armbruster
spd_data_generate() splits @ram_size bytes into @nbanks RAM banks of 1 << sz_log2 MiB each, like this: size = ram_size >> 20; /* work in terms of megabytes */ [...] nbanks = 1; while (sz_log2 > max_log2 && nbanks < 8) { sz_log2--; nbanks++; } Each iteration halves the size of a bank, and increments the number of banks. Wrong: it should double the number of banks. The bug goes back all the way to commit b296b664ab "smbus: Add a helper to generate SPD EEPROM data". It can't bite because spd_data_generate()'s current users pass only @ram_size that result in *zero* iterations: machine RAM size #banks type bank size fulong2e 256 MiB 1 DDR 256 MiB sam460ex 2048 MiB 1 DDR2 2048 MiB 1024 MiB 1 DDR2 1024 MiB 512 MiB 1 DDR2 512 MiB 256 MiB 1 DDR2 256 MiB 128 MiB 1 SDR 128 MiB 64 MiB 1 SDR 64 MiB 32 MiB 1 SDR 32 MiB Apply the obvious, minimal fix. I admit I'm tempted to rip out the unused (and obviously untested) feature instead, because YAGNI. Note that this is not the final result, as spd_data_generate() next increases #banks from 1 to 2 if possible. This is done "to avoid a bug in MIPS Malta firmware". We don't even use this function with machine type malta. *Shrug* Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422134815.1584-5-armbru@redhat.com>
2020-04-29bamboo, sam460ex: Tidy up error message for unsupported RAM sizeMarkus Armbruster
Improve $ ppc-softmmu/qemu-system-ppc -M sam460ex -m 4096 qemu-system-ppc: Max 1 banks of 2048 ,1024 ,512 ,256 ,128 ,64 ,32 MB DIMM/bank supported qemu-system-ppc: Possible valid RAM size: 2048 to qemu-system-ppc: at most 1 bank of 2048, 1024, 512, 256, 128, 64, 32 MiB each supported Possible valid RAM size: 1024 MiB Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422134815.1584-4-armbru@redhat.com> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-04-29smbus: Fix spd_data_generate() error API violationMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. spd_data_generate() can pass @errp to error_setg() more than once when it adjusts both memory size and type. Harmless, because no caller passes anything that needs adjusting. Until the previous commit, sam460ex passed types that needed adjusting, but not sizes. spd_data_generate()'s contract is rather awkward: If everything's fine, return non-null and don't set an error. Else, if memory size or type need adjusting, return non-null and set an error describing the adjustment. Else, return null and set an error reporting why no data can be generated. Its callers treat the error as a warning even when null is returned. They don't create the "smbus-eeprom" device then. Suspicious. Since the previous commit, only "everything's fine" can actually happen. Drop the unused code and simplify the callers. This gets rid of the error API violation. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422134815.1584-3-armbru@redhat.com>
2020-04-29sam460ex: Suppress useless warning on -m 32 and -m 64Markus Armbruster
Requesting 32 or 64 MiB of RAM with the sam460ex machine type produces a useless warning: qemu-system-ppc: warning: Memory size is too small for SDRAM type, adjusting type This is because sam460ex_init() asks spd_data_generate() for DDR2, which is impossible, so spd_data_generate() corrects it to DDR. The warning goes back to commit 08fd99179a "sam460ex: Clean up SPD EEPROM creation". Make sam460ex_init() pass the correct SDRAM type to get rid of the warning. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422134815.1584-2-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-04-29qga: Fix qmp_guest_suspend_{disk, ram}() error handlingMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second qmp_guest_suspend_disk() and qmp_guest_suspend_ram() pass @local_err first to check_suspend_mode(), then to acquire_privilege(), then to execute_async(). Continuing after errors here can only end in tears. For instance, we risk tripping error_setv()'s assertion. Fixes: aa59637ea1c6a4c83430933f9c44c43e6c3f1b69 Fixes: f54603b6aa765514b2519e74114a2f417759d727 Cc: Michael Roth <mdroth@linux.vnet.ibm.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422130719.28225-15-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-04-29qga: Fix qmp_guest_get_memory_blocks() error handlingMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. qmp_guest_get_memory_blocks() passes &local_err to transfer_memory_block() in a loop. If this fails in more than one iteration, it can trip error_setv()'s assertion. Fix it to break the loop. Cc: Michael Roth <mdroth@linux.vnet.ibm.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422130719.28225-14-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-29tests/test-logging: Fix test for -dfilter 0..0xffffffffffffffffMarkus Armbruster
Fixes: 58e19e6e7914354242a67442d0006f9e31684d1a Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422130719.28225-13-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-04-29migration/colo: Fix qmp_xen_colo_do_checkpoint() error handlingMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. qmp_xen_colo_do_checkpoint() passes @errp first to replication_do_checkpoint_all(), and then to colo_notify_filters_event(). If both fail, this will trip the assertion in error_setv(). Similar code in secondary_vm_do_failover() calls colo_notify_filters_event() only after replication_do_checkpoint_all() succeeded. Do the same here. Fixes: 0e8818f023616677416840d6ddc880db8de3c967 Cc: Zhang Chen <chen.zhang@intel.com> Cc: zhanghailiang <zhang.zhanghailiang@huawei.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Message-Id: <20200422130719.28225-12-armbru@redhat.com>
2020-04-29io: Fix qio_channel_socket_close() error handlingMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. qio_channel_socket_close() passes @errp first to socket_listen_cleanup(), and then, if closesocket() fails, to error_setg_errno(). If socket_listen_cleanup() failed, this will trip the assertion in error_setv(). Fix by ignoring a second error. Fixes: 73564c407caedf992a1c688b5fea776a8b56ba2a Cc: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20200422130719.28225-11-armbru@redhat.com>
2020-04-29xen/pt: Fix flawed conversion to realize()Markus Armbruster
The conversion of xen_pt_initfn() to xen_pt_realize() blindly replaced XEN_PT_ERR() by error_setg(). Several error conditions that did not fail xen_pt_initfn() now fail xen_pt_realize(). Unsurprisingly, the cleanup on these errors looks highly suspicious. Revert the inappropriate replacements. Fixes: 5a11d0f7549e24a10e178a9dc8ff5e698031d9a6 Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Anthony Perard <anthony.perard@citrix.com> Cc: Paul Durrant <paul@xen.org> Cc: xen-devel@lists.xenproject.org Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Paul Durrant <paul@xen.org> Message-Id: <20200422130719.28225-10-armbru@redhat.com>
2020-04-29virtio-net: Fix duplex=... and speed=... error handlingMarkus Armbruster
virtio_net_device_realize() rejects invalid duplex and speed values. The error handling is broken: $ ../qemu/bld-sani/x86_64-softmmu/qemu-system-x86_64 -S -display none -monitor stdio QEMU 4.2.93 monitor - type 'help' for more information (qemu) device_add virtio-net,duplex=x Error: 'duplex' must be 'half' or 'full' (qemu) c ================================================================= ==15654==ERROR: AddressSanitizer: heap-use-after-free on address 0x62e000014590 at pc 0x560b75c8dc13 bp 0x7fffdf1a6950 sp 0x7fffdf1a6940 READ of size 8 at 0x62e000014590 thread T0 #0 0x560b75c8dc12 in object_dynamic_cast_assert /work/armbru/qemu/qom/object.c:826 #1 0x560b74c38ac0 in virtio_vmstate_change /work/armbru/qemu/hw/virtio/virtio.c:3210 #2 0x560b74d9765e in vm_state_notify /work/armbru/qemu/softmmu/vl.c:1271 #3 0x560b7494ba72 in vm_prepare_start /work/armbru/qemu/cpus.c:2156 #4 0x560b7494bacd in vm_start /work/armbru/qemu/cpus.c:2162 #5 0x560b75a7d890 in qmp_cont /work/armbru/qemu/monitor/qmp-cmds.c:160 #6 0x560b75a8d70a in hmp_cont /work/armbru/qemu/monitor/hmp-cmds.c:1043 #7 0x560b75a799f2 in handle_hmp_command /work/armbru/qemu/monitor/hmp.c:1082 [...] 0x62e000014590 is located 33168 bytes inside of 42288-byte region [0x62e00000c400,0x62e000016930) freed by thread T1 here: #0 0x7feadd39491f in __interceptor_free (/lib64/libasan.so.5+0x10d91f) #1 0x7feadcebcd7c in g_free (/lib64/libglib-2.0.so.0+0x55d7c) #2 0x560b75c8fd40 in object_unref /work/armbru/qemu/qom/object.c:1128 #3 0x560b7498a625 in memory_region_unref /work/armbru/qemu/memory.c:1762 #4 0x560b74999fa4 in do_address_space_destroy /work/armbru/qemu/memory.c:2788 #5 0x560b762362fc in call_rcu_thread /work/armbru/qemu/util/rcu.c:283 #6 0x560b761c8884 in qemu_thread_start /work/armbru/qemu/util/qemu-thread-posix.c:519 #7 0x7fead9be34bf in start_thread (/lib64/libpthread.so.0+0x84bf) previously allocated by thread T0 here: #0 0x7feadd394d18 in __interceptor_malloc (/lib64/libasan.so.5+0x10dd18) #1 0x7feadcebcc88 in g_malloc (/lib64/libglib-2.0.so.0+0x55c88) #2 0x560b75c8cf8a in object_new /work/armbru/qemu/qom/object.c:699 #3 0x560b75010ad9 in qdev_device_add /work/armbru/qemu/qdev-monitor.c:654 #4 0x560b750120c2 in qmp_device_add /work/armbru/qemu/qdev-monitor.c:805 #5 0x560b75012c1b in hmp_device_add /work/armbru/qemu/qdev-monitor.c:905 [...] ==15654==ABORTING Cause: virtio_net_device_realize() neglects to bail out after setting the error. Fix that. Fixes: 9473939ed7addcaaeb8fde5c093918fb7fa0919c Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200422130719.28225-9-armbru@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com>
2020-04-29bochs-display: Fix vgamem=SIZE error handlingMarkus Armbruster
bochs_display_realize() rejects out-of-range vgamem. The error handling is broken: $ qemu-system-x86_64 -S -display none -monitor stdio QEMU 4.2.93 monitor - type 'help' for more information (qemu) device_add bochs-display,vgamem=1 Error: bochs-display: video memory too small (qemu) device_add bochs-display,vgamem=1 RAMBlock "0000:00:04.0/bochs-display-vram" already registered, abort! Aborted (core dumped) Cause: bochs_display_realize() neglects to bail out after setting the error. Fix that. Fixes: 765c94290863eef1fc4a67819d452cc13b7854a1 Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200422130719.28225-8-armbru@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
2020-04-29fdc: Fix fallback=auto error handlingMarkus Armbruster
fdctrl_realize_common() rejects fallback=auto. Used by devices "isa-fdc", "sysbus-fdc", "SUNW,fdtwo". The error handling is broken: $ qemu-system-x86_64 -nodefaults -device isa-fdc,fallback=auto,driveA=fd0 -drive if=none,id=fd0 ** ERROR:/work/armbru/qemu/hw/block/fdc.c:434:pick_drive_type: assertion failed: (drv->drive != FLOPPY_DRIVE_TYPE_AUTO) Aborted (core dumped) Cause: fdctrl_realize_common() neglects to bail out after setting the error. Fix that. Fixes: a73275dd6fc3bfda33165bebc28e0c33c20cb0a0 Cc: John Snow <jsnow@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200422130719.28225-7-armbru@redhat.com>
2020-04-29arm/virt: Fix virt_machine_device_plug_cb() error API violationMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. virt_machine_device_plug_cb() passes @errp to cryptodev_builtin_sym_close_session() in a loop. Harmless, because cryptodev_builtin_sym_close_session() can't actually fail. Fix by dropping its Error ** parameter. Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200422130719.28225-6-armbru@redhat.com>
2020-04-29cpus: Proper range-checking for -icount shift=NMarkus Armbruster
timers_state.icount_time_shift must be in [0,63] to avoid undefined behavior when shifting by it, e.g. in cpu_icount_to_ns(). icount_adjust() clamps it to [0,MAX_ICOUNT_SHIFT], with MAX_ICOUNT_SHIFT = 10. configure_icount() doesn't. Fix that. Fixes: a8bfac37085c3372366d722f131a7e18d664ee4d Cc: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422130719.28225-5-armbru@redhat.com>
2020-04-29cpus: Fix configure_icount() error API violationMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. configure_icount() is wrong that way. Harmless, because its @errp is always &error_abort or &error_fatal. Just as wrong (and just as harmless): when it fails, it can still update global state. Fix all that. Cc: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422130719.28225-4-armbru@redhat.com>
2020-04-29block/file-posix: Fix check_cache_dropped() error handlingMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. check_cache_dropped() calls error_setg() in a loop. It fails to break the loop in one instance. If a subsequent iteration error_setg()s again, it trips error_setv()'s assertion. Fix it to break the loop. Fixes: 31be8a2a97ecba7d31a82932286489cac318e9e9 Cc: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200422130719.28225-3-armbru@redhat.com>
2020-04-29cryptodev: Fix cryptodev_builtin_cleanup() error API violationMarkus Armbruster
The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. cryptodev_builtin_cleanup() passes @errp to cryptodev_builtin_sym_close_session() in a loop. Harmless, because cryptodev_builtin_sym_close_session() can't actually fail. Fix it anyway. Cc: Gonglei <arei.gonglei@huawei.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200422130719.28225-2-armbru@redhat.com>
2020-04-29qemu-img: Reject broken -o ""Markus Armbruster
qemu-img create, convert, amend, and measure use accumulate_options() to merge multiple -o options. This is broken for -o "": $ qemu-img create -f qcow2 -o backing_file=a -o "" -o backing_fmt=raw,size=1M new.qcow2 qemu-img: warning: Could not verify backing image. This may become an error in future versions. Could not open 'a,backing_fmt=raw': No such file or directory Formatting 'new.qcow2', fmt=qcow2 size=1048576 backing_file=a,,backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ qemu-img info new.qcow2 image: new.qcow2 file format: qcow2 virtual size: 1 MiB (1048576 bytes) disk size: 196 KiB cluster_size: 65536 --> backing file: a,backing_fmt=raw Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false Merging these three -o the obvious way is wrong, because it results in an unwanted ',' escape: backing_file=a,,backing_fmt=raw,size=1M ~~ We could silently drop -o "", but Kevin asked me to reject it instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200415074927.19897-10-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-29qemu-img: Move is_valid_option_list() to qemu-img.c and rewriteMarkus Armbruster
is_valid_option_list()'s purpose is ensuring qemu-img.c's can safely join multiple parameter strings separated by ',' like this: g_strdup_printf("%s,%s", params1, params2); How it does that is anything but obvious. A close reading of the code reveals that it fails exactly when its argument starts with ',' or ends with an odd number of ','. Makes sense, actually, because when the argument starts with ',', a separating ',' preceding it would get escaped, and when it ends with an odd number of ',', a separating ',' following it would get escaped. Move it to qemu-img.c and rewrite it the obvious way. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200415074927.19897-9-armbru@redhat.com>
2020-04-29qemu-img: Factor out accumulate_options() helperMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200415074927.19897-8-armbru@redhat.com>
2020-04-29qemu-option: Avoid has_help_option() in qemu_opts_parse_noisily()Markus Armbruster
When opts_parse() sets @invalidp to true, qemu_opts_parse_noisily() uses has_help_option() to decide whether to print help. This parses the input string a second time. Easy to avoid: replace @invalidp by @help_wanted. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200415074927.19897-7-armbru@redhat.com>
2020-04-29test-qemu-opts: Simplify test_has_help_option() after bug fixMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200415074927.19897-6-armbru@redhat.com>
2020-04-29qemu-option: Fix has_help_option()'s sloppy parsingMarkus Armbruster
has_help_option() uses its own parser. It's inconsistent with qemu_opts_parse(), as demonstrated by test-qemu-opts case /qemu-opts/has_help_option. Fix by reusing the common parser. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200415074927.19897-5-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>