aboutsummaryrefslogtreecommitdiff
path: root/qom/object.c
AgeCommit message (Collapse)AuthorFilesLines
2023-11-10qom: Add object_property_set_default_list()Kevin Wolf1-0/+6
This function provides a default for properties that are accessed using the list visitor interface. The default is always an empty list. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-ID: <20231109174240.72376-10-kwolf@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-10-03qom: Propagate alignment through type systemRichard Henderson1-0/+14
Propagate alignment just like size. This is required in order to get the correct alignment on most cpu subclasses where the size and alignment is only specified for the base cpu type. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
2022-11-06module: add Error arguments to module_load and module_load_qomClaudio Fontana1-4/+14
improve error handling during module load, by changing: bool module_load(const char *prefix, const char *lib_name); void module_load_qom(const char *type); to: int module_load(const char *prefix, const char *name, Error **errp); int module_load_qom(const char *type, Error **errp); where the return value is: -1 on module load error, and errp is set with the error 0 on module or one of its dependencies are not installed 1 on module load success 2 on module load success (module already loaded or built-in) module_load_qom_one has been introduced in: commit 28457744c345 ("module: qom module support"), which built on top of module_load_one, but discarded the bool return value. Restore it. Adapt all callers to emit errors, or ignore them, or fail hard, as appropriate in each context. Replace the previous emission of errors via fprintf in _some_ error conditions with Error and error_report, so as to emit to the appropriate target. A memory leak is also fixed as part of the module_load changes. audio: when attempting to load an audio module, report module load errors. Note that still for some callers, a single issue may generate multiple error reports, and this could be improved further. Regarding the audio code itself, audio_add() seems to ignore errors, and this should probably be improved. block: when attempting to load a block module, report module load errors. For the code paths that already use the Error API, take advantage of those to report module load errors into the Error parameter. For the other code paths, we currently emit the error, but this could be improved further by adding Error parameters to all possible code paths. console: when attempting to load a display module, report module load errors. qdev: when creating a new qdev Device object (DeviceState), report load errors. If a module cannot be loaded to create that device, now abort execution (if no CONFIG_MODULE) or exit (if CONFIG_MODULE). qom/object.c: when initializing a QOM object, or looking up class_by_name, report module load errors. qtest: when processing the "module_load" qtest command, report errors in the load of the module. Signed-off-by: Claudio Fontana <cfontana@suse.de> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20220929093035.4231-4-cfontana@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2022-11-06module: rename module_load_one to module_loadClaudio Fontana1-2/+2
Signed-off-by: Claudio Fontana <cfontana@suse.de> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20220929093035.4231-3-cfontana@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2022-10-27qom: Improve error messages when property has no getter or setterMarkus Armbruster1-2/+4
When you try to set a property that has no setter, the error message blames "insufficient permission": $ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio QEMU 7.1.50 monitor - type 'help' for more information (qemu) qom-set /machine type q35 Error: Insufficient permission to perform this operation This implies it could work with "sufficient permission". It can't. Change the error message to: Error: Property 'pc-i440fx-7.2-machine.type' is not writable Do the same for getting a property that has no getter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221012153801.2604340-2-armbru@redhat.com> Reviewed-by: David Hildenbrand <david@redhat.com>
2022-03-08Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into stagingPeter Maydell1-1/+5
virtio,pc,pci: features, cleanups, fixes vhost-user enabled on non-linux systems beginning of nvme sriov support bigger tx queue for vdpa virtio iommu bypass FADT flag to detect legacy keyboards Fixes, cleanups all over the place Signed-off-by: Michael S. Tsirkin <mst@redhat.com> # gpg: Signature made Mon 07 Mar 2022 22:43:31 GMT # gpg: using RSA key 5D09FD0871C8F85B94CA8A0D281F0DB8D28D5469 # gpg: issuer "mst@redhat.com" # gpg: Good signature from "Michael S. Tsirkin <mst@kernel.org>" [full] # gpg: aka "Michael S. Tsirkin <mst@redhat.com>" [full] # Primary key fingerprint: 0270 606B 6F3C DF3D 0B17 0970 C350 3912 AFBE 8E67 # Subkey fingerprint: 5D09 FD08 71C8 F85B 94CA 8A0D 281F 0DB8 D28D 5469 * remotes/mst/tags/for_upstream: (47 commits) hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present tests/acpi: i386: update FACP table differences hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table tests/acpi: i386: allow FACP acpi table changes docs: vhost-user: add subsection for non-Linux platforms configure, meson: allow enabling vhost-user on all POSIX systems vhost: use wfd on functions setting vring call fd event_notifier: add event_notifier_get_wfd() pci: drop COMPAT_PROP_PCP for 2.0 machine types hw/smbios: Add table 4 parameter, "processor-id" x86: cleanup unused compat_apic_id_mode vhost-vsock: detach the virqueue element in case of error pc: add option to disable PS/2 mouse/keyboard acpi: pcihp: pcie: set power on cap on parent slot pci: expose TYPE_XIO3130_DOWNSTREAM name pci: show id info when pci BDF conflict hw/misc/pvpanic: Use standard headers instead headers: Add pvpanic.h pci-bridge/xio3130_downstream: Fix error handling pci-bridge/xio3130_upstream: Fix error handling ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org> # Conflicts: # docs/specs/index.rst
2022-03-07osdep: Move memalign-related functions to their own headerPeter Maydell1-0/+1
Move the various memalign-related functions out of osdep.h and into their own header, which we include only where they are used. While we're doing this, add some brief documentation comments. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20220226180723.1706285-10-peter.maydell@linaro.org
2022-03-04qom: assert integer does not overflowMichael S. Tsirkin1-1/+5
QOM reference counting is not designed with an infinite amount of references in mind, trying to take a reference in a loop without dropping a reference will overflow the integer. It is generally a symptom of a reference leak (a missing deref, commonly as part of error handling - such as one fixed here: https://lore.kernel.org/r/20220228095058.27899-1-sgarzare%40redhat.com ). All this can lead to either freeing the object too early (memory corruption) or never freeing it (memory leak). If we happen to dereference at just the right time (when it's wrapping around to 0), we might eventually assert when dereferencing, but the real problem is an extra object_ref so let's assert there to make such issues cleaner and easier to debug. Some micro-benchmarking shows using fetch and add this is essentially free on x86. Since multiple threads could be incrementing in parallel, we assert around INT_MAX to make sure none of these approach the wrap around point: this way we get a memory leak and not a memory corruption, the former is generally easier to debug. Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2022-02-21Mark remaining global TypeInfo instances as constBernhard Beschow1-2/+2
More than 1k of TypeInfo instances are already marked as const. Mark the remaining ones, too. This commit was created with: git grep -z -l 'static TypeInfo' -- '*.c' | \ xargs -0 sed -i 's/static TypeInfo/static const TypeInfo/' Signed-off-by: Bernhard Beschow <shentey@gmail.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Acked-by: Corey Minyard <cminyard@mvista.com> Message-id: 20220117145805.173070-2-shentey@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2021-11-10monitor: Fix find_device_state() for IDs containing slashesMarkus Armbruster1-0/+11
Recent commit 6952026120 "monitor: Tidy up find_device_state()" assumed the function's argument is "the device's ID or QOM path" (as documented for device_del). It's actually either an absolute QOM path, or a QOM path relative to /machine/peripheral/. Such a relative path is a device ID when it doesn't contain a slash. When it does, the function now always fails. Broke iotest 200, which uses relative path "vda/virtio-backend". It fails because object_resolve_path_component() resolves just one component, not a relative path. The obvious function to resolve relative paths is object_resolve_path(). It picks a parent automatically. Too much magic, we want to specify the parent. Create new object_resolve_path_at() for that, and use it in find_device_state(). Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20211019085711.86377-1-armbru@redhat.com> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
2021-10-15qom: Reduce use of error_propagate()Kevin Wolf1-4/+3
ERRP_GUARD() makes debugging easier by making sure that &error_abort still fails at the real origin of the error instead of error_propagate(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20211008133442.141332-5-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Tested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-07-23qom: use correct field name when getting/setting alias propertiesPaolo Bonzini1-2/+7
Alias targets have a different name than the alias property itself (e.g. a machine's pflash0 might be an alias of a property named 'drive'). When the target's getter or setter invokes the visitor, it will use a different name than what the caller expects, and the visitor will not be able to find it (or will consume erroneously). The solution is for alias getters and setters to wrap the incoming visitor, and forward the sole field that the target is expecting while renaming it appropriately. This bug has been there forever, but it was exposed after -M parsing switched from QemuOptions and StringInputVisitor to keyval and QObjectInputVisitor. Before, the visitor ignored the name. Now, it checks "drive" against what was passed on the command line and finds that no such property exists. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/484 Reported-by: Alex Williamson <alex.williamson@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2021-02-25qom/object.c: Fix typoDoug Evans1-1/+1
A simple typo (noticed by inspection). Signed-off-by: Doug Evans <dje@google.com> Message-Id: <000000000000530c7105bb191b33@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Doug Evans <dje@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2021-02-08qom: Allow optional sugar propsGreg Kurz1-1/+3
Global properties have an @optional field, which allows to apply a given property to a given type even if one of its subclasses doesn't support it. This is especially used in the compat code when dealing with the "disable-modern" and "disable-legacy" properties and the "virtio-pci" type. Allow object_register_sugar_prop() to set this field as well. Signed-off-by: Greg Kurz <groug@kaod.org> Message-Id: <159738953558.377274.16617742952571083440.stgit@bahia.lan> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2021-01-02qom: Assert that objects being destroyed have no parentEduardo Habkost1-0/+1
QOM reference counting bugs are often hard to detect, but there's one kind of bug that's easier: if we are freeing an object but is still attached to a parent, it means the reference count is wrong (because the parent always hold a reference to their children). Add an assertion to make sure we detect those cases. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <20201215224133.3545901-3-ehabkost@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-12-19Revert "qobject: let object_property_get_str() use new API"Markus Armbruster1-3/+6
Commit aafb21a0b9 "qobject: let object_property_get_str() use new API" isn't much of a simplification. Not worth having object_property_get_str() differ from the other object_property_get_FOO(). Revert. This reverts commit aafb21a0b9cea5fa0fe52e68111bb6bd13837a02. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Daniel P. Berrangé <berrange@redhat.com> Cc: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201211171152.146877-12-armbru@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
2020-12-11Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into ↵Peter Maydell1-29/+7
staging * Fix for NULL segments (Bin Meng) * Support for 32768 CPUs on x86 without IOMMU (David) * PDEP/PEXT fix and testcase (myself) * Remove bios_name and ram_size globals (myself) * qemu_init rationalization (myself) * Update kernel-doc (myself + upstream patches) * Propagate MemTxResult across DMA and PCI functions (Philippe) * Remove master/slave when applicable (Philippe) * WHPX support for in-kernel irqchip (Sunil) # gpg: Signature made Thu 10 Dec 2020 17:21:50 GMT # gpg: using RSA key F13338574B662389866C7682BFFBD25F78C7AE83 # gpg: issuer "pbonzini@redhat.com" # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [full] # gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" [full] # Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4 E2F7 7E15 100C CD36 69B1 # Subkey fingerprint: F133 3857 4B66 2389 866C 7682 BFFB D25F 78C7 AE83 * remotes/bonzini-gitlab/tags/for-upstream: (113 commits) scripts: kernel-doc: remove unnecessary change wrt Linux Revert "docs: temporarily disable the kernel-doc extension" scripts: kernel-doc: use :c:union when needed scripts: kernel-doc: split typedef complex regex scripts: kernel-doc: fix typedef parsing Revert "kernel-doc: Handle function typedefs that return pointers" Revert "kernel-doc: Handle function typedefs without asterisks" scripts: kernel-doc: try to use c:function if possible scripts: kernel-doc: fix line number handling scripts: kernel-doc: allow passing desired Sphinx C domain dialect scripts: kernel-doc: don't mangle with parameter list scripts: kernel-doc: fix typedef identification scripts: kernel-doc: reimplement -nofunction argument scripts: kernel-doc: fix troubles with line counts scripts: kernel-doc: use a less pedantic markup for funcs on Sphinx 3.x scripts: kernel-doc: make it more compatible with Sphinx 3.x Revert "kernel-doc: Use c:struct for Sphinx 3.0 and later" Revert "scripts/kerneldoc: For Sphinx 3 use c:macro for macros with arguments" scripts: kernel-doc: add support for typedef enum kernel-doc: add support for ____cacheline_aligned attribute ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-12-10qom: code hardening - have bound checking while looping with integer valueAni Sinha1-2/+3
Object property insertion code iterates over an integer to get an unused index that can be used as an unique name for an object property. This loop increments the integer value indefinitely. Although very unlikely, this can still cause an integer overflow. In this change, we fix the above code by checking against INT16_MAX and making sure that the interger index does not overflow beyond that value. If no available index is found, the code would cause an assertion failure. This assertion failure is necessary because the callers of the function do not check the return value for NULL. Signed-off-by: Ani Sinha <ani@anisinha.ca> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20200921093325.25617-1-ani@anisinha.ca> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-12-10qom: eliminate identical functionsPaolo Bonzini1-29/+7
Most property release functions in qom/object.c only free the opaque value. Combine all of them into a single function. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-10-06qom: Improve error message displayed with missing object propertiesPhilippe Mathieu-Daudé1-1/+2
Instead of only displaying the property missing, also display the object name. This help developer to quickly figure out the mistake without opening a debugger. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Li Qiang <liq3ea@gmail.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200920155340.401482-1-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-09-24Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into ↵Peter Maydell1-10/+10
staging Pull request This includes the atomic_ -> qatomic_ rename that touches many files and is prone to conflicts. # gpg: Signature made Wed 23 Sep 2020 17:08:43 BST # gpg: using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8 # gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [full] # gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" [full] # Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35 775A 9CA4 ABB3 81AB 73C8 * remotes/stefanha/tags/block-pull-request: qemu/atomic.h: rename atomic_ to qatomic_ tests: add test-fdmon-epoll fdmon-poll: reset npfd when upgrading to fdmon-epoll gitmodules: add qemu.org vbootrom submodule gitmodules: switch to qemu.org meson mirror gitmodules: switch to qemu.org qboot mirror docs/system: clarify deprecation schedule virtio-crypto: don't modify elem->in/out_sg virtio-blk: undo destructive iov_discard_*() operations util/iov: add iov_discard_undo() virtio: add vhost-user-fs-ccw device libvhost-user: handle endianness as mandated by the spec MAINTAINERS: add Stefan Hajnoczi as block/nvme.c maintainer Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-09-23qemu/atomic.h: rename atomic_ to qatomic_Stefan Hajnoczi1-10/+10
clang's C11 atomic_fetch_*() functions only take a C11 atomic type pointer argument. QEMU uses direct types (int, etc) and this causes a compiler error when a QEMU code calls these functions in a source file that also included <stdatomic.h> via a system header file: $ CC=clang CXX=clang++ ./configure ... && make ../util/async.c:79:17: error: address argument to atomic operation must be a pointer to _Atomic type ('unsigned int *' invalid) Avoid using atomic_*() names in QEMU's atomic.h since that namespace is used by <stdatomic.h>. Prefix QEMU's APIs with 'q' so that atomic.h and <stdatomic.h> can co-exist. I checked /usr/include on my machine and searched GitHub for existing "qatomic_" users but there seem to be none. This patch was generated using: $ git grep -h -o '\<atomic\(64\)\?_[a-z0-9_]\+' include/qemu/atomic.h | \ sort -u >/tmp/changed_identifiers $ for identifier in $(</tmp/changed_identifiers); do sed -i "s%\<$identifier\>%q$identifier%g" \ $(git grep -I -l "\<$identifier\>") done I manually fixed line-wrap issues and misaligned rST tables. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200923105646.47864-1-stefanha@redhat.com>
2020-09-22qom: simplify object_find_property / object_class_find_propertyDaniel P. Berrangé1-25/+35
When debugging QEMU it is often useful to put a breakpoint on the error_setg_internal method impl. Unfortunately the object_property_add / object_class_property_add methods call object_property_find / object_class_property_find methods to check if a property exists already before adding the new property. As a result there are a huge number of calls to error_setg_internal on startup of most QEMU commands, making it very painful to set a breakpoint on this method. Most callers of object_find_property and object_class_find_property, however, pass in a NULL for the Error parameter. This simplifies the methods to remove the Error parameter entirely, and then adds some new wrapper methods that are able to raise an Error when needed. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200914135617.1493072-1-berrange@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-09-18qom: Allow objects to be allocated with increased alignmentRichard Henderson1-3/+33
It turns out that some hosts have a default malloc alignment less than that required for vectors. We assume that, with compiler annotation on CPUArchState, that we can properly align the vector portion of the guest state. Fix the alignment of the allocation by using qemu_memalloc when required. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20200916004638.2444147-3-richard.henderson@linaro.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-09-18qom: Clean up object_property_get_enum()'s error valueMarkus Armbruster1-3/+3
object_property_get_enum() is the only object_property_FOO() that is documented to return an undefined value on error. It does no such thing, actually: it returns 0 on some errors, and -1 on others. Needlessly complicated. Always return -1 on error, and adjust the contract. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20200917125540.597786-2-armbru@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-09-15object_initialize: try module loadGerd Hoffmann1-0/+6
Needed to allow virtio-gpu-pci initialize the virtio-gpu-device child device. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200914134224.29769-5-kraxel@redhat.com
2020-09-08qom: make object_ref/unref use a void * instead of Object *.Daniel P. Berrangé1-2/+4
The object_ref/unref methods are intended for use with any subclass of the base Object. Using "Object *" in the signature is not adding any meaningful level of type safety, since callers simply use "OBJECT(ptr)" and this expands to an unchecked cast "(Object *)". By using "void *" we enable the object_unref() method to be used to provide support for g_autoptr() with any subclass. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20200723181410.3145233-2-berrange@redhat.com> Message-Id: <20200831210740.126168-2-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-07-21qom: Change object_get_canonical_path_component() not to mallocMarkus Armbruster1-4/+3
object_get_canonical_path_component() returns a malloced copy of a property name on success, null on failure. 19 of its 25 callers immediately free the returned copy. Change object_get_canonical_path_component() to return the property name directly. Since modifying the name would be wrong, adjust the return type to const char *. Drop the free from the 19 callers become simpler, add the g_strdup() to the other six. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200714160202.3121879-4-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Li Qiang <liq3ea@gmail.com>
2020-07-10qom: Introduce object_property_try_add_child()Eric Auger1-5/+16
object_property_add() does not allow object_property_try_add() to gracefully fail as &error_abort is passed as an error handle. However such failure can easily be triggered from the QMP shell when, for instance, one attempts to create an object with an id that already exists. This is achieved from the following call path: qmp_object_add -> user_creatable_add_dict -> user_creatable_add_type -> object_property_add_child -> object_property_add For instance, from the qmp-shell, call twice: object-add qom-type=memory-backend-ram id=mem1 props.size=1073741824 and QEMU aborts. This behavior is undesired as a user/management application mistake in reusing a property ID shouldn't result in loss of the VM and live data within. This patch introduces a new function, object_property_try_add_child() which takes an error handle and turn object_property_try_add() into a non-static one. Now the call path becomes: user_creatable_add_type -> object_property_try_add_child -> object_property_try_add and the error is returned gracefully to the QMP client. (QEMU) object-add qom-type=memory-backend-ram id=mem2 props.size=4294967296 {"return": {}} (QEMU) object-add qom-type=memory-backend-ram id=mem2 props.size=4294967296 {"error": {"class": "GenericError", "desc": "attempt to add duplicate property 'mem2' to object (type 'container')"}} Signed-off-by: Eric Auger <eric.auger@redhat.com> Fixes: d2623129a7de ("qom: Drop parameter @errp of object_property_add() & friends") Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Greg Kurz <groug@kaod.org> Tested-by: Greg Kurz <groug@kaod.org> Message-Id: <20200629193424.30280-2-eric.auger@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-07-10error: Eliminate error_propagate() manuallyMarkus Armbruster1-20/+13
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous two commits did that for sufficiently simple cases with Coccinelle. Do it for several more manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-37-armbru@redhat.com>
2020-07-10error: Eliminate error_propagate() with Coccinelle, part 2Markus Armbruster1-3/+1
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous commit did that with a Coccinelle script I consider fairly trustworthy. This commit uses the same script with the matching of return taken out, i.e. we convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... } to if (!foo(..., errp)) { ... ... } This is unsound: @err could still be read between afterwards. I don't know how to express "no read of @err without an intervening write" in Coccinelle. Instead, I manually double-checked for uses of @err. Suboptimal line breaks tweaked manually. qdev_realize() simplified further to placate scripts/checkpatch.pl. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-36-armbru@redhat.com>
2020-07-10error: Eliminate error_propagate() with Coccinelle, part 1Markus Armbruster1-24/+8
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. Convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... return ... } to if (!foo(..., errp)) { ... ... return ... } where nothing else needs @err. Coccinelle script: @rule1 forall@ identifier fun, err, errp, lbl; expression list args, args2; binary operator op; constant c1, c2; symbol false; @@ if ( ( - fun(args, &err, args2) + fun(args, errp, args2) | - !fun(args, &err, args2) + !fun(args, errp, args2) | - fun(args, &err, args2) op c1 + fun(args, errp, args2) op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; ) } @rule2 forall@ identifier fun, err, errp, lbl; expression list args, args2; expression var; binary operator op; constant c1, c2; symbol false; @@ - var = fun(args, &err, args2); + var = fun(args, errp, args2); ... when != err if ( ( var | !var | var op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; | return var; ) } @depends on rule1 || rule2@ identifier err; @@ - Error *err = NULL; ... when != err Not exactly elegant, I'm afraid. The "when != lbl:" is necessary to avoid transforming if (fun(args, &err)) { goto out } ... out: error_propagate(errp, err); even though other paths to label out still need the error_propagate(). For an actual example, see sclp_realize(). Without the "when strict", Coccinelle transforms vfio_msix_setup(), incorrectly. I don't know what exactly "when strict" does, only that it helps here. The match of return is narrower than what I want, but I can't figure out how to express "return where the operand doesn't use @err". For an example where it's too narrow, see vfio_intx_enable(). Silently fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. Line breaks tidied up manually. One nested declaration of @local_err deleted manually. Preexisting unwanted blank line dropped in hw/riscv/sifive_e.c. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-35-armbru@redhat.com>
2020-07-10qom: Make functions taking Error ** return bool, not 0/-1Markus Armbruster1-7/+7
Just for consistency. Also fix the example in object_set_props()'s documentation. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-31-armbru@redhat.com>
2020-07-10qom: Use returned bool to check for failure, Coccinelle partMarkus Armbruster1-10/+5
The previous commit enables conversion of foo(..., &err); if (err) { ... } to if (!foo(..., errp)) { ... } for QOM functions that now return true / false on success / error. Coccinelle script: @@ identifier fun = { object_apply_global_props, object_initialize_child_with_props, object_initialize_child_with_propsv, object_property_get, object_property_get_bool, object_property_parse, object_property_set, object_property_set_bool, object_property_set_int, object_property_set_link, object_property_set_qobject, object_property_set_str, object_property_set_uint, object_set_props, object_set_propv, user_creatable_add_dict, user_creatable_complete, user_creatable_del }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-29-armbru@redhat.com>
2020-07-10qom: Make functions taking Error ** return bool, not voidMarkus Armbruster1-37/+62
See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-28-armbru@redhat.com>
2020-07-10qom: Put name parameter before value / visitor parameterMarkus Armbruster1-26/+26
The object_property_set_FOO() setters take property name and value in an unusual order: void object_property_set_FOO(Object *obj, FOO_TYPE value, const char *name, Error **errp) Having to pass value before name feels grating. Swap them. Same for object_property_set(), object_property_get(), and object_property_parse(). Convert callers with this Coccinelle script: @@ identifier fun = { object_property_get, object_property_parse, object_property_set_str, object_property_set_link, object_property_set_bool, object_property_set_int, object_property_set_uint, object_property_set, object_property_set_qobject }; expression obj, v, name, errp; @@ - fun(obj, v, name, errp) + fun(obj, name, v, errp) Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Convert that one manually. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused by RXCPU being used both as typedef and function-like macro there. Convert manually. The other files using RXCPU that way don't need conversion. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-27-armbru@redhat.com> [Straightforwad conflict with commit 2336172d9b "audio: set default value for pcspk.iobase property" resolved]
2020-07-10qom: Use return values to check for error where that's simplerMarkus Armbruster1-11/+14
When using the Error object to check for error, we need to receive it into a local variable, then propagate() it to @errp. Using the return value permits allows receiving it straight to @errp. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-26-armbru@redhat.com>
2020-07-10qom: Rename qdev_get_type() to object_get_type()Markus Armbruster1-2/+2
Commit 2f262e06f0 lifted qdev_get_type() from qdev to object without renaming it accordingly. Do that now. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-23-armbru@redhat.com>
2020-07-10qapi: Use returned bool to check for failure, Coccinelle partMarkus Armbruster1-28/+14
The previous commit enables conversion of visit_foo(..., &err); if (err) { ... } to if (!visit_foo(..., errp)) { ... } for visitor functions that now return true / false on success / error. Coccinelle script: @@ identifier fun =~ "check_list|input_type_enum|lv_start_struct|lv_type_bool|lv_type_int64|lv_type_str|lv_type_uint64|output_type_enum|parse_type_bool|parse_type_int64|parse_type_null|parse_type_number|parse_type_size|parse_type_str|parse_type_uint64|print_type_bool|print_type_int64|print_type_null|print_type_number|print_type_size|print_type_str|print_type_uint64|qapi_clone_start_alternate|qapi_clone_start_list|qapi_clone_start_struct|qapi_clone_type_bool|qapi_clone_type_int64|qapi_clone_type_null|qapi_clone_type_number|qapi_clone_type_str|qapi_clone_type_uint64|qapi_dealloc_start_list|qapi_dealloc_start_struct|qapi_dealloc_type_anything|qapi_dealloc_type_bool|qapi_dealloc_type_int64|qapi_dealloc_type_null|qapi_dealloc_type_number|qapi_dealloc_type_str|qapi_dealloc_type_uint64|qobject_input_check_list|qobject_input_check_struct|qobject_input_start_alternate|qobject_input_start_list|qobject_input_start_struct|qobject_input_type_any|qobject_input_type_bool|qobject_input_type_bool_keyval|qobject_input_type_int64|qobject_input_type_int64_keyval|qobject_input_type_null|qobject_input_type_number|qobject_input_type_number_keyval|qobject_input_type_size_keyval|qobject_input_type_str|qobject_input_type_str_keyval|qobject_input_type_uint64|qobject_input_type_uint64_keyval|qobject_output_start_list|qobject_output_start_struct|qobject_output_type_any|qobject_output_type_bool|qobject_output_type_int64|qobject_output_type_null|qobject_output_type_number|qobject_output_type_str|qobject_output_type_uint64|start_list|visit_check_list|visit_check_struct|visit_start_alternate|visit_start_list|visit_start_struct|visit_type_.*"; expression list args; typedef Error; Error *err; @@ - fun(args, &err); - if (err) + if (!fun(args, &err)) { ... } A few line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-19-armbru@redhat.com>
2020-07-07object: qom module supportGerd Hoffmann1-0/+14
Little helper function to load modules on demand. In most cases adding module loading support for devices and other objects is just s/object_class_by_name/module_object_class_by_name/ in the right spot. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200624131045.14512-3-kraxel@redhat.com
2020-06-15qom: Less verbose object_initialize_child()Markus Armbruster1-4/+15
All users of object_initialize_child() pass the obvious child size argument. Almost all pass &error_abort and no properties. Tiresome. Rename object_initialize_child() to object_initialize_child_with_props() to free the name. New convenience wrapper object_initialize_child() automates the size argument, and passes &error_abort and no properties. Rename object_initialize_childv() to object_initialize_child_with_propsv() for consistency. Convert callers with this Coccinelle script: @@ expression parent, propname, type; expression child, size; symbol error_abort; @@ - object_initialize_child(parent, propname, OBJECT(child), size, type, &error_abort, NULL) + object_initialize_child(parent, propname, child, size, type, &error_abort, NULL) @@ expression parent, propname, type; expression child; symbol error_abort; @@ - object_initialize_child(parent, propname, child, sizeof(*child), type, &error_abort, NULL) + object_initialize_child(parent, propname, child, type) @@ expression parent, propname, type; expression child; symbol error_abort; @@ - object_initialize_child(parent, propname, &child, sizeof(child), type, &error_abort, NULL) + object_initialize_child(parent, propname, &child, type) @@ expression parent, propname, type; expression child, size, err; expression list props; @@ - object_initialize_child(parent, propname, child, size, type, err, props) + object_initialize_child_with_props(parent, propname, child, size, type, err, props) Note that Coccinelle chokes on ARMSSE typedef vs. macro in hw/arm/armsse.c. Worked around by temporarily renaming the macro for the spatch run. Signed-off-by: Markus Armbruster <armbru@redhat.com> Acked-by: Alistair Francis <alistair.francis@wdc.com> [Rebased: machine opentitan is new (commit fe0fe4735e7)] Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-37-armbru@redhat.com>
2020-06-15qom: Constify object_get_canonical_path{,_component}()'s parameterMarkus Armbruster1-2/+2
Suggested-by: Eric Blake <eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200527084754.7531-2-armbru@redhat.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-06-10qom/object: pass (Object *) to object_initialize_with_type()Masahiro Yamada1-3/+1
object_new_with_type() already passes (Object *) pointer. Avoid casting back and forth. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Message-Id: <20200512173104.2293073-1-masahiroy@kernel.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-06-10qom/object: simplify type_initialize_interface()Masahiro Yamada1-2/+1
iface_impl->class is the same as new_iface. Make it more readable. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Message-Id: <20200512182501.2300530-1-masahiroy@kernel.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-06-10qom/object: factor out the initialization of hash table of propertiesMasahiro Yamada1-5/+3
Properties are not related to the initialization of interfaces. The initialization of the hash table can be moved after the if-block, and unified. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Message-Id: <20200512172615.2291999-1-masahiroy@kernel.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-06-10qom: remove index from object_resolve_abs_path()Masahiro Yamada1-10/+9
You can advance 'parts' to track the current path fragment. The 'index' parameter is unneeded. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Message-Id: <20200510013235.954906-1-masahiroy@kernel.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-06-10qom/object: Fix object_child_foreach_recursive() return valueCédric Le Goater1-1/+4
When recursing, the return value of do_object_child_foreach() is not taken into account. Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> Fixes: d714b8de7747 ("qom: Add recursive version of object_child_for_each") Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200404153340.164861-1-clg@kaod.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-05-15various: Remove unnecessary OBJECT() castPhilippe Mathieu-Daudé1-2/+2
The OBJECT() macro is defined as: #define OBJECT(obj) ((Object *)(obj)) Remove the unnecessary OBJECT() casts when we already know the pointer is of Object type. Patch created mechanically using spatch with this script: @@ typedef Object; Object *o; @@ - OBJECT(o) + o Acked-by: Cornelia Huck <cohuck@redhat.com> Acked-by: Corey Minyard <cminyard@mvista.com> Acked-by: John Snow <jsnow@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20200512070020.22782-3-f4bug@amsat.org> [Trivial rebase conflict in hw/s390x/sclp.c resolved]
2020-05-15qom: Drop @errp parameter of object_property_del()Markus Armbruster1-6/+1
Same story as for object_property_add(): the only way object_property_del() can fail is when the property with this name does not exist. Since our property names are all hardcoded, failure is a programming error, and the appropriate way to handle it is passing &error_abort. Most callers do that, the commit before previous fixed one that didn't (and got the error handling wrong), and the two remaining exceptions ignore errors. Drop the @errp parameter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-19-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-05-15qom: Drop parameter @errp of object_property_add() & friendsMarkus Armbruster1-202/+100
The only way object_property_add() can fail is when a property with the same name already exists. Since our property names are all hardcoded, failure is a programming error, and the appropriate way to handle it is passing &error_abort. Same for its variants, except for object_property_add_child(), which additionally fails when the child already has a parent. Parentage is also under program control, so this is a programming error, too. We have a bit over 500 callers. Almost half of them pass &error_abort, slightly fewer ignore errors, one test case handles errors, and the remaining few callers pass them to their own callers. The previous few commits demonstrated once again that ignoring programming errors is a bad idea. Of the few ones that pass on errors, several violate the Error API. 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. ich9_pm_add_properties(), sparc32_ledma_realize(), sparc32_dma_realize(), xilinx_axidma_realize(), xilinx_enet_realize() are wrong that way. When the one appropriate choice of argument is &error_abort, letting users pick the argument is a bad idea. Drop parameter @errp and assert the preconditions instead. There's one exception to "duplicate property name is a programming error": the way object_property_add() implements the magic (and undocumented) "automatic arrayification". Don't drop @errp there. Instead, rename object_property_add() to object_property_try_add(), and add the obvious wrapper object_property_add(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-15-armbru@redhat.com> [Two semantic rebase conflicts resolved]