summary refs log tree commit diff stats
path: root/tests/unit (follow)
Commit message (Collapse)AuthorAgeFilesLines
...
| * tests/unit/test-seccomp: Remove mentions of softmmu in test namesPhilippe Mathieu-Daudé2023-11-071-12/+12
| | | | | | | | | | | | | | | | | | | | | | Wether we are using a software MMU or not is irrelevant for the seccomp facility. The facility is restricted to system emulation, but such detail isn't really helpful, so directly drop the 'softmmu' mention from the test names. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20231002145104.52193-3-philmd@linaro.org>
* | Merge tag 'for_upstream' of https://git.kernel.org/pub/scm/virt/kvm/mst/qemu ↵Stefan Hajnoczi2023-11-071-13/+54
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | into staging virtio,pc,pci: features, fixes virtio sound card support vhost-user: back-end state migration cxl: line length reduction enabling fabric management vhost-vdpa: shadow virtqueue hash calculation Support shadow virtqueue RSS Support tests: CPU topology related smbios test cases Fixes, cleanups all over the place Signed-off-by: Michael S. Tsirkin <mst@redhat.com> # -----BEGIN PGP SIGNATURE----- # # iQFDBAABCAAtFiEEXQn9CHHI+FuUyooNKB8NuNKNVGkFAmVKDDoPHG1zdEByZWRo # YXQuY29tAAoJECgfDbjSjVRpF08H/0Zts8uvkHbgiOEJw4JMHU6/VaCipfIYsp01 # GSfwYOyEsXJ7GIxKWaCiMnWXEm7tebNCPKf3DoUtcAojQj3vuF9XbWBKw/bfRn83 # nGO/iiwbYViSKxkwqUI+Up5YiN9o0M8gBFrY0kScPezbnYmo5u2bcADdEEq6gH68 # D0Ea8i+WmszL891ypvgCDBL2ObDk3qX3vA5Q6J2I+HKX2ofJM59BwaKwS5ghw+IG # BmbKXUZJNjUQfN9dQ7vJuiuqdknJ2xUzwW2Vn612ffarbOZB1DZ6ruWlrHty5TjX # 0w4IXEJPBgZYbX9oc6zvTQnbLDBJbDU89mnme0TcmNMKWmQKTtc= # =vEv+ # -----END PGP SIGNATURE----- # gpg: Signature made Tue 07 Nov 2023 18:06:50 HKT # 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 * tag 'for_upstream' of https://git.kernel.org/pub/scm/virt/kvm/mst/qemu: (63 commits) acpi/tests/avocado/bits: enable console logging from bits VM acpi/tests/avocado/bits: enforce 32-bit SMBIOS entry point hw/cxl: Add tunneled command support to mailbox for switch cci. hw/cxl: Add dummy security state get hw/cxl/type3: Cleanup multiple CXL_TYPE3() calls in read/write functions hw/cxl/mbox: Add Get Background Operation Status Command hw/cxl: Add support for device sanitation hw/cxl/mbox: Wire up interrupts for background completion hw/cxl/mbox: Add support for background operations hw/cxl: Implement Physical Ports status retrieval hw/pci-bridge/cxl_downstream: Set default link width and link speed hw/cxl/mbox: Add Physical Switch Identify command. hw/cxl/mbox: Add Information and Status / Identify command hw/cxl: Add a switch mailbox CCI function hw/pci-bridge/cxl_upstream: Move defintion of device to header. hw/cxl/mbox: Generalize the CCI command processing hw/cxl/mbox: Pull the CCI definition out of the CXLDeviceState hw/cxl/mbox: Split mailbox command payload into separate input and output hw/cxl/mbox: Pull the payload out of struct cxl_cmd and make instances constant hw/cxl: Fix a QEMU_BUILD_BUG_ON() in switch statement scope issue. ... Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * tests: test-smp-parse: Add the test for cores/threads per socket helpersZhao Liu2023-11-071-13/+54
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use the different ways to calculate cores/threads per socket, so that the new CPU topology levels won't be missed in these 2 helpes: * machine_topo_get_cores_per_socket() * machine_topo_get_threads_per_socket() Test the commit a1d027be95bc3 ("machine: Add helpers to get cores/ threads per socket"). Suggested-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Zhao Liu <zhao1.liu@intel.com> Acked-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <20231023094635.1588282-2-zhao1.liu@linux.intel.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
* | util/uuid: Add UUID_STR_LEN definitionCédric Le Goater2023-11-031-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | qemu_uuid_unparse() includes a trailing NUL when writing the uuid string and the buffer size should be UUID_FMT_LEN + 1 bytes. Add a define for this size and use it where required. Cc: Fam Zheng <fam@euphon.net> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Juan Quintela <quintela@redhat.com> Reviewed-by: "Denis V. Lunev" <den@openvz.org> Signed-off-by: Cédric Le Goater <clg@redhat.com>
* | test: Add some tests for range and resv-mem helpersEric Auger2023-11-032-0/+317
|/ | | | | | | | | | Add unit tests for both resv_region_list_insert() and range_inverse_array(). Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: "Michael S. Tsirkin" <mst@redhat.com> [ clg: Removal of unused variable in compare_ranges() ] Signed-off-by: Cédric Le Goater <clg@redhat.com>
* tests/coroutine: Clean up global variable shadowingPhilippe Mathieu-Daudé2023-10-271-6/+6
| | | | | | | | | | | | | | | | | | | Rename the global variable to avoid: tests/unit/test-coroutine.c:430:11: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] bool *done = opaque; ^ tests/unit/test-coroutine.c:438:10: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] bool done = false; ^ tests/unit/test-coroutine.c:198:12: note: previous declaration is here static int done; ^ Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-ID: <20231009100251.56019-11-philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
* tests/aio: Clean up global variable shadowingPhilippe Mathieu-Daudé2023-10-271-2/+2
| | | | | | | | | | | | | | | | Rename the argument to fix: tests/unit/test-aio.c:130:44: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] static void set_event_notifier(AioContext *ctx, EventNotifier *notifier, ^ tests/unit/test-aio.c:22:20: note: previous declaration is here static AioContext *ctx; ^ Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-ID: <20231009100251.56019-9-philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
* tests/throttle: Clean up global variable shadowingPhilippe Mathieu-Daudé2023-10-271-1/+0
| | | | | | | | | | | | | | | | | Follow all other tests pattern from this file, use the global 'cfg' variable to fix: tests/unit/test-throttle.c:621:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] ThrottleConfig cfg; ^ tests/unit/test-throttle.c:28:23: note: previous declaration is here static ThrottleConfig cfg; ^ Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Acked-by: Alberto Garcia <berto@igalia.com> Message-ID: <20231009100251.56019-3-philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com>
* meson: do not use set10Paolo Bonzini2023-10-171-1/+1
| | | | | | | Make all items of config-host.h consistent. To keep the --disable-coroutine-pool code visible to the compiler, mutuate the IS_ENABLED() macro from Linux. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* block: Protect bs->children with graph_lockKevin Wolf2023-10-121-0/+4
| | | | | | | | | | | Almost all functions that access the child links already take the graph lock now. Add locking to the remaining users and finally annotate the struct field itself as protected by the graph lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-22-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Protect bs->parents with graph_lockKevin Wolf2023-10-121-0/+5
| | | | | | | | | | | Almost all functions that access the parent link already take the graph lock now. Add locking to the remaining user in a test case and finally annotate the struct field itself as protected by the graph lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-21-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Mark drain related functions GRAPH_RDLOCKEmanuele Giuseppe Esposito2023-10-121-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Draining recursively traverses the graph, therefore we need to make sure that also such accesses to the graph are protected by the graph rdlock. There are 3 different drain callers to consider: 1. drain in the main loop: no issue at all, rdlock is nop. 2. drain in an iothread: rdlock only works in main loop or coroutines, so disallow it. 3. drain in a coroutine (regardless of AioContext): the drain mechanism takes care of scheduling a BH in the bs->aio_context that will then take care of perform the actual draining. This is wrong, because as pointed in (2) if bs->aio_context is an iothread then rdlock won't work. Therefore change bdrv_co_yield_to_drain to schedule the BH in the main loop. Caller (2) also implies that we need to modify test-bdrv-drain.c to disallow draining in the iothreads. For some places, we know that they will hold the lock, but we don't have the GRAPH_RDLOCK annotations yet. In this case, add assume_graph_lock() with a FIXME comment. These places will be removed once everything is properly annotated. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-6-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCKKevin Wolf2023-10-121-0/+3
| | | | | | | | | | | | This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_first_blk() and bdrv_is_root_node() need to hold a reader lock for the graph. These functions are the only functions in block-backend.c that access the parent list of a node. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-5-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* test-bdrv-drain: Don't call bdrv_graph_wrlock() in coroutine contextKevin Wolf2023-10-121-3/+4
| | | | | | | | | | | | | | | | AIO callbacks are effectively coroutine_mixed_fn. If AIO requests don't return immediately, their callback is called from the request coroutine. This means that in AIO callbacks, we can't call no_coroutine_fns such as bdrv_graph_wrlock(). Unfortunately test-bdrv-drain does so. Change the test to use a BH to drop out of coroutine context, and add coroutine_mixed_fn and no_coroutine_fn markers to clarify the context each function runs in. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-2-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* system: Rename softmmu/ directory as system/Philippe Mathieu-Daudé2023-10-081-1/+1
| | | | | | | | | | The softmmu/ directory contains files specific to system emulation. Rename it as system/. Update meson rules, the MAINTAINERS file and all the documentation and comments. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-ID: <20231004090629.37473-14-philmd@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* Merge tag 'for_upstream' of https://git.kernel.org/pub/scm/virt/kvm/mst/qemu ↵Stefan Hajnoczi2023-10-053-0/+165
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | into staging virtio,pci: features, cleanups vdpa: shadow vq vlan support net migration with cvq cxl: support emulating 4 HDM decoders serial number extended capability virtio: hared dma-buf Fixes, cleanups all over the place. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> * tag 'for_upstream' of https://git.kernel.org/pub/scm/virt/kvm/mst/qemu: (53 commits) libvhost-user: handle shared_object msg vhost-user: add shared_object msg hw/display: introduce virtio-dmabuf util/uuid: add a hash function virtio: remove unused next argument from virtqueue_split_read_next_desc() virtio: remove unnecessary thread fence while reading next descriptor virtio: use shadow_avail_idx while checking number of heads libvhost-user.c: add assertion to vu_message_read_default pcie_sriov: unregister_vfs(): fix error path hw/i386/pc: improve physical address space bound check for 32-bit x86 systems amd_iommu: Fix APIC address check vdpa net: follow VirtIO initialization properly at cvq isolation probing vdpa net: stop probing if cannot set features vdpa net: fix error message setting virtio status hw/pci-bridge/cxl-upstream: Add serial number extended capability support hw/cxl: Support 4 HDM decoders at all levels of topology hw/cxl: Fix and use same calculation for HDM decoder block size everywhere hw/cxl: Add utility functions decoder interleave ways and target count. hw/cxl: Push cxl_decoder_count_enc() and cxl_decode_ig() into .c vdpa net: zero vhost_vdpa iova_tree pointer at cleanup ... Conflicts: hw/core/machine.c Context conflict with commit 314e0a84cd5d ("hw/core: remove needless includes") because it removed an adjacent #include.
| * hw/display: introduce virtio-dmabufAlbert Esteve2023-10-042-0/+138
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This API manages objects (in this iteration, dmabuf fds) that can be shared along different virtio devices, associated to a UUID. The API allows the different devices to add, remove and/or retrieve the objects by simply invoking the public functions that reside in the virtio-dmabuf file. For vhost backends, the API stores the pointer to the backend holding the object. Suggested-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Albert Esteve <aesteve@redhat.com> Message-Id: <20231002065706.94707-3-aesteve@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
| * util/uuid: add a hash functionAlbert Esteve2023-10-041-0/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Add hash function to uuid module using the djb2 hash algorithm. Add a couple simple unit tests for the hash function, checking collisions for similar UUIDs. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Albert Esteve <aesteve@redhat.com> Message-Id: <20231002065706.94707-2-aesteve@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
* | test-throttle: don't shadow 'index' variable in do_test_accounting()Alberto Garcia2023-09-291-1/+1
|/ | | | | | | | | Fixes build with -Wshadow=local Signed-off-by: Alberto Garcia <berto@igalia.com> Message-ID: <20230922105742.81317-1-berto@igalia.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
* Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into stagingStefan Hajnoczi2023-09-213-1/+53
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Block layer patches - Graph locking part 4 (node management) - qemu-img map: report compressed data blocks - block-backend: process I/O in the current AioContext # -----BEGIN PGP SIGNATURE----- # # iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmULHnURHGt3b2xmQHJl # ZGhhdC5jb20ACgkQfwmycsiPL9aB5hAAqH8To7WIUtg1rj1PY809ck78ghm18PKg # TNdN7IbrXQghX5foh2VgPwVVl+JaW2CSrJYWQcAO6AbvFduNIi9iKzI6RT0xKXpb # b8oQXS7zntFzwBv8ohOU5NSVJOgVmNP4h5qJIMmXgB9ZcLFG40zggVH2qQT7guUf # 9MAc81kI/d5vvSHY0ZjdHjNOgwG4q1j8yytL7OFqWUfB8sXloUCA9lT7w4jIYD8L # v2StUOLWB01Zts2o8SCNaFxuajs6wUee8b/DM1cyPyLy4KtOdXvLKhq2NlXpLo2i # aZFr4PtizTVwrQZIJttA9jqM+QCsDOsiSat3BLNNsKUaCWHZB0rOGLCzMCtisyOo # 4PzuL4UI21ik2zieO1qVM+Thqvw16kHtp6dD9pGk4X4ogGreGYEIxzBl79luR+AV # NCRizoeFWTHKymS1tSoKrWT9ZNHcLmwemO6Tt1rMYk9jV3T4uY5e1NwxaUavEfsX # f8dLfQjhNiySOoDknT1OSerBOVdTXURS2ri5H3GZxrxvJ4jOeFkn52C8r3YlZ3Wp # Cr9LCUJZeXgwY+Q1JQ3D4VLY8aZ83txpw6XKEy0eTEv5wxkBj5LWhXx7hNb5F3lg # bqaRYijVJn+P82wVxlftIzMfNeVBFHzFE90taPV5grJjr8lgrGBFmD7Puc97kfDX # oTDBwRxJeew= # =qTNA # -----END PGP SIGNATURE----- # gpg: Signature made Wed 20 Sep 2023 12:31:49 EDT # gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6 # gpg: issuer "kwolf@redhat.com" # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full] # Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6 * tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (28 commits) block: mark aio_poll as non-coroutine block-backend: process zoned requests in the current AioContext block-backend: process I/O in the current AioContext test-bdrv-drain: avoid race with BH in IOThread drain test block: remove AIOCBInfo->get_aio_context() qemu-img: map: report compressed data blocks block: add BDRV_BLOCK_COMPRESSED flag for bdrv_block_status() block: Mark bdrv_add/del_child() and caller GRAPH_WRLOCK block: Mark bdrv_unref_child() GRAPH_WRLOCK block: Mark bdrv_root_unref_child() GRAPH_WRLOCK block: Take graph rdlock in bdrv_change_aio_context() block: Take graph rdlock in bdrv_drop_intermediate() block: Mark bdrv_parent_cb_change_media() GRAPH_RDLOCK block: Mark bdrv_child_perm() GRAPH_RDLOCK block: Mark bdrv_get_cumulative_perm() and callers GRAPH_RDLOCK block: Mark bdrv_parent_perms_conflict() and callers GRAPH_RDLOCK block: Mark bdrv_attach_child() GRAPH_WRLOCK block: Call transaction callbacks with lock held block: Mark bdrv_attach_child_common() GRAPH_WRLOCK block: Mark bdrv_replace_child_tran() GRAPH_WRLOCK ... Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * test-bdrv-drain: avoid race with BH in IOThread drain testStefan Hajnoczi2023-09-201-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch fixes a race condition in test-bdrv-drain that is difficult to reproduce. test-bdrv-drain sometimes fails without an error message on the block pull request sent by Kevin Wolf on Sep 4, 2023. I was able to reproduce it locally and found that "block-backend: process I/O in the current AioContext" (in this patch series) is the first commit where it reproduces. I do not know why "block-backend: process I/O in the current AioContext" exposes this bug. It might be related to the fact that the test's preadv request runs in the main thread instead of IOThread a after my commit. That might simply change the timing of the test. Now on to the race condition in test-bdrv-drain. The main thread schedules a BH in IOThread a and then drains the BDS: aio_bh_schedule_oneshot(ctx_a, test_iothread_main_thread_bh, &data); /* The request is running on the IOThread a. Draining its block device * will make sure that it has completed as far as the BDS is concerned, * but the drain in this thread can continue immediately after * bdrv_dec_in_flight() and aio_ret might be assigned only slightly * later. */ do_drain_begin(drain_type, bs); If the BH completes before do_drain_begin() then there is nothing to worry about. If the BH invokes bdrv_flush() before do_drain_begin(), then do_drain_begin() waits for it to complete. The problematic case is when do_drain_begin() runs before the BH enters bdrv_flush(). Then do_drain_begin() misses the BH and the drain mechanism has failed in quiescing I/O. Fix this by incrementing the in_flight counter so that do_drain_begin() waits for test_iothread_main_thread_bh(). Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230912231037.826804-3-stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * block: Mark bdrv_unref_child() GRAPH_WRLOCKKevin Wolf2023-09-201-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_unref_child(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-21-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * block: Mark bdrv_parent_perms_conflict() and callers GRAPH_RDLOCKKevin Wolf2023-09-201-0/+4
| | | | | | | | | | | | | | | | | | | | The function reads the parents list, so it needs to hold the graph lock. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-14-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * block: Mark bdrv_attach_child() GRAPH_WRLOCKKevin Wolf2023-09-202-0/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of taking the writer lock internally, require callers to already hold it when calling bdrv_attach_child_common(). These callers will typically already hold the graph lock once the locking work is completed, which means that they can't call functions that take it internally. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-13-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * block: Take AioContext lock for bdrv_append() more consistentlyKevin Wolf2023-09-203-0/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | The documentation for bdrv_append() says that the caller must hold the AioContext lock for bs_top. Change all callers to actually adhere to the contract. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20230911094620.45040-5-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | Merge tag 'pull-block-2023-09-01' of https://gitlab.com/hreitz/qemu into stagingStefan Hajnoczi2023-09-211-5/+71
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Block patches - Fix for file-posix's zoning code crashing on I/O errors - Throttling refactoring # -----BEGIN PGP SIGNATURE----- # # iQJGBAABCAAwFiEEy2LXoO44KeRfAE00ofpA0JgBnN8FAmTxnMISHGhyZWl0ekBy # ZWRoYXQuY29tAAoJEKH6QNCYAZzfYkUP+gMG9hhzvgjj/tw9rEBQjciihzcQmqQJ # 2Mm37RH2jj5bnnTdaTbMkcRRwVhncYSCwK9q5EYVbZmU9C/v4YJmsSEQlcl7wVou # hbPUv6NHaBrJZX9nxNSa2RHui6pZMLKa/D0rJVB7NjYBrrRtiPo7kiLVQYjYXa2g # kcCCfY4t3Z2RxOP31mMXRjYlhJE9bIuZdTEndrKme8KS2JGPZEJ9xjkoW1tj96EX # oc/Cg2vk7AEtsFYA0bcD8fTFkBDJEwyYl3usu7Tk24pvH16jk7wFSqRVSsDMfnER # tG8X3mHLIY0hbSkpzdHJdXINvZ6FWpQb0CGzIKr+pMiuWVdWr1HglBr0m4pVF+Y4 # A6AI6VX2JJgtacypoDyCZC9mzs1jIdeiwq9v5dyuikJ6ivTwEEoeoSLnLTN3AjXn # 0mtQYzgCg5Gd6+rTo7XjSO9SSlbaVrDl/B2eXle6tmIFT5k+86fh0hc+zTmP8Rkw # Knbc+5Le95wlMrOUNx2GhXrTGwX510hLxKboho/LITxtAzqvXnEJKrYbnkm3WPnw # wfHnR5VQH1NKEpiH/p33og6OV/vu9e7vgp0ZNZV136SnzC90C1zMUwg2simJW701 # 34EtN0XBX8XBKrxfe7KscV9kRE8wrWWJVbhp+WOcQEomGI8uraxzWqDIk/v7NZXv # m4XBscaB+Iri # =oKgk # -----END PGP SIGNATURE----- # gpg: Signature made Fri 01 Sep 2023 04:11:46 EDT # gpg: using RSA key CB62D7A0EE3829E45F004D34A1FA40D098019CDF # gpg: issuer "hreitz@redhat.com" # gpg: Good signature from "Hanna Reitz <hreitz@redhat.com>" [unknown] # gpg: WARNING: The key's User ID is not certified with a trusted signature! # gpg: There is no indication that the signature belongs to the owner. # Primary key fingerprint: CB62 D7A0 EE38 29E4 5F00 4D34 A1FA 40D0 9801 9CDF * tag 'pull-block-2023-09-01' of https://gitlab.com/hreitz/qemu: tests/file-io-error: New test file-posix: Simplify raw_co_prw's 'out' zone code file-posix: Fix zone update in I/O error path file-posix: Check bs->bl.zoned for zone info file-posix: Clear bs->bl.zoned on error block/throttle-groups: Use ThrottleDirection instread of bool is_write fsdev: Use ThrottleDirection instread of bool is_write throttle: use THROTTLE_MAX/ARRAY_SIZE for hard code throttle: use enum ThrottleDirection instead of bool is_write cryptodev: use NULL throttle timer cb for read direction test-throttle: test read only and write only throttle: support read-only and write-only test-throttle: use enum ThrottleDirection throttle: introduce enum ThrottleDirection Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * throttle: use enum ThrottleDirection instead of bool is_writezhenwei pi2023-08-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | enum ThrottleDirection is already there, use ThrottleDirection instead of 'bool is_write' for throttle API, also modify related codes from block, fsdev, cryptodev and tests. Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Message-Id: <20230728022006.1098509-7-pizhenwei@bytedance.com> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
| * test-throttle: test read only and write onlyzhenwei pi2023-08-291-0/+66
| | | | | | | | | | | | | | | | Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Message-Id: <20230728022006.1098509-5-pizhenwei@bytedance.com> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
| * test-throttle: use enum ThrottleDirectionzhenwei pi2023-08-291-3/+3
| | | | | | | | | | | | | | | | | | | | Use enum ThrottleDirection instead in the throttle test codes. Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Message-Id: <20230728022006.1098509-3-pizhenwei@bytedance.com> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
* | tests/: spelling fixesMichael Tokarev2023-09-088-16/+16
| | | | | | | | | | | | | | | | | | | | | | with some rewording in tests/qemu-iotests/298 tests/qtest/fuzz/generic_fuzz.c tests/unit/test-throttle.c as suggested by Eric. Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Eric Blake <eblake@redhat.com>
* | configure, meson: remove target OS symbols from config-host.makPaolo Bonzini2023-09-071-3/+3
|/ | | | | | | Stop applying config-host.mak to the sourcesets, since it does not have any more CONFIG_* symbols coming from the command line. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* qga: Add tests for --allow-rpcs optionKonstantin Kostiuk2023-07-101-0/+31
| | | | | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
* test-block-iothread: Lock AioContext for blk_insert_bs()Kevin Wolf2023-06-281-1/+6
| | | | | | | | | | blk_insert_bs() requires that callers hold the AioContext lock for the node that should be inserted. Take it. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230605085711.21261-4-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* atomics: eliminate mb_read/mb_setPaolo Bonzini2023-06-061-1/+1
| | | | | | | | | | | | | | | qatomic_mb_read and qatomic_mb_set were the very first atomic primitives introduced for QEMU; their semantics are unclear and they provide a false sense of safety. The last use of qatomic_mb_read() has been removed, so delete it. qatomic_mb_set() instead can survive as an optimized qatomic_set()+smp_mb(), similar to Linux's smp_store_mb(), but rename it to qatomic_set_mb() to match the order of the two operations. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* cutils: Improve qemu_strtosz handling of fractionsEric Blake2023-06-021-31/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have several limitations and bugs worth fixing; they are inter-related enough that it is not worth splitting this patch into smaller pieces: * ".5k" should work to specify 512, just as "0.5k" does * "1.9999k" and "1." + "9"*50 + "k" should both produce the same result of 2048 after rounding * "1." + "0"*350 + "1B" should not be treated the same as "1.0B"; underflow in the fraction should not be lost * "7.99e99" and "7.99e999" look similar, but our code was doing a read-out-of-bounds on the latter because it was not expecting ERANGE due to overflow. While we document that scientific notation is not supported, and the previous patch actually fixed qemu_strtod_finite() to no longer return ERANGE overflows, it is easier to pre-filter than to try and determine after the fact if strtod() consumed more than we wanted. Note that this is a low-level semantic change (when endptr is not NULL, we can now successfully parse with a scale of 'E' and then report trailing junk, instead of failing outright with EINVAL); but an earlier commit already argued that this is not a high-level semantic change since the only caller passing in a non-NULL endptr also checks that the tail is whitespace-only. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1629 Fixes: cf923b78 ("utils: Improve qemu_strtosz() to have 64 bits of precision", 6.0.0) Fixes: 7625a1ed ("utils: Use fixed-point arithmetic in qemu_strtosz", 6.0.0) Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-20-eblake@redhat.com> [eblake: tweak function comment for accuracy]
* cutils: Improve qemu_strtod* error pathsEric Blake2023-06-021-27/+36
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previous patches changed all integral qemu_strto*() error paths to guarantee that *value is never left uninitialized. Do likewise for qemu_strtod. Also, tighten qemu_strtod_finite() to never return a non-finite value (prior to this patch, we were rejecting "inf" with -EINVAL and unspecified result 0.0, but failing "9e999" with -ERANGE and HUGE_VAL - which is infinite on IEEE machines - despite our function claiming to recognize only finite values). Auditing callers, we have no external callers of qemu_strtod, and among the callers of qemu_strtod_finite: - qapi/qobject-input-visitor.c:qobject_input_type_number_keyval() and qapi/string-input-visitor.c:parse_type_number() which reject all errors (does not matter what we store) - utils/cutils.c:do_strtosz() incorrectly assumes that *endptr points to '.' on all failures (that is, it is not distinguishing between EINVAL and ERANGE; and therefore still does the WRONG THING for "9.9e999". The change here does not entirely fix that (a later patch will tackle this more systematically), but at least it fixes the read-out-of-bounds first diagnosed in https://gitlab.com/qemu-project/qemu/-/issues/1629 - our testsuite, which we can update to match what we document Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> CC: qemu-stable@nongnu.org Message-Id: <20230522190441.64278-19-eblake@redhat.com>
* cutils: Use parse_uint in qemu_strtosz for negative rejectionEric Blake2023-06-021-4/+3
| | | | | | | | | | | | | | | | | | Rather than open-coding two different ways to check for an unwanted negative sign, reuse the same code in both functions. That way, if we decide down the road to accept "-0" instead of rejecting it, we have fewer places to change. Also, it means we now get ERANGE instead of EINVAL for negative values in qemu_strtosz, which is reasonable for what it represents. This in turn changes the expected output of a couple of iotests. The change is not quite complete: negative fractional scaled values can trip us up. This will be fixed in a later patch addressing other issues with fractional scaled values. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-18-eblake@redhat.com>
* cutils: Set value in all integral qemu_strto* error pathsEric Blake2023-06-021-12/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | Our goal in writing qemu_strtoi() and friends is to have an interface harder to abuse than libc's strtol(). Leaving the return value uninitialized on some but not all error paths does not lend itself well to this goal; and our documentation wasn't helpful on what to expect. Note that the previous patch changed all qemu_strtosz() EINVAL error paths to slam value to 0 rather than stay uninitialized, even when the EINVAL eror occurs because of trailing junk. But for the remaining integral qemu_strto*, it's easier to return the parsed value than to force things back to zero, in part because of how check_strtox_error works; in part because people expect that from libc strto* (while there is no libc strtosz to compare to), and in part because doing so creates less churn in the testsuite. Here, the list of affected callers is much longer ('git grep "qemu_strto[ui]" "*.c" "**/*.c" | grep -v tests/ |wc -l' outputs 107, although a few of those are the implementation in in cutils.c), so touching as little as possible is the wisest course of action. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-17-eblake@redhat.com>
* cutils: Set value in all qemu_strtosz* error pathsEric Blake2023-06-021-55/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Making callers determine whether or not *value was populated on error is not nice for usability. Pre-patch, we have unit tests that check that *result is left unchanged on most EINVAL errors and set to 0 on many ERANGE errors. This is subtly different from libc strtoumax() behavior which returns UINT64_MAX on ERANGE errors, as well as different from our parse_uint() which slams to 0 on EINVAL on the grounds that we want our functions to be harder to mis-use than strtoumax(). Let's audit callers: - hw/core/numa.c:parse_numa() fixed in the previous patch to check for errors - migration/migration-hmp-cmds.c:hmp_migrate_set_parameter(), monitor/hmp.c:monitor_parse_arguments(), qapi/opts-visitor.c:opts_type_size(), qapi/qobject-input-visitor.c:qobject_input_type_size_keyval(), qemu-img.c:cvtnum_full(), qemu-io-cmds.c:cvtnum(), target/i386/cpu.c:x86_cpu_parse_featurestr(), and util/qemu-option.c:parse_option_size() appear to reject all failures (although some with distinct messages for ERANGE as opposed to EINVAL), so it doesn't matter what is in the value parameter on error. - All remaining callers are in the testsuite, where we can tweak our expectations to match our new desired behavior. Advancing to the end of the string parsed on overflow (ERANGE), while still returning 0, makes sense (UINT64_MAX as a size is unlikely to be useful); likewise, our size parsing code is complex enough that it's easier to always return 0 when endptr is NULL but trailing garbage was found, rather than trying to return the value of the prefix actually parsed (no current caller cared about the value of the prefix). Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-16-eblake@redhat.com>
* test-cutils: Add more coverage to qemu_strtoszEric Blake2023-06-021-11/+129
| | | | | | | | | | Add some more strings that the user might send our way. In particular, some of these additions include FIXME comments showing where our parser doesn't quite behave the way we want. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-15-eblake@redhat.com>
* cutils: Allow NULL str in qemu_strtoszEric Blake2023-06-021-1/+9
| | | | | | | | | | | All the other qemu_strto* and parse_uint allow a NULL str. Having qemu_strtosz not crash on qemu_strtosz(NULL, NULL, &value) is an easy fix that adds some consistency between our string parsers. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-13-eblake@redhat.com>
* test-cutils: Refactor qemu_strtosz tests for less boilerplateEric Blake2023-06-021-404/+101
| | | | | | | | | | | | | | | | | | | | | | No need to copy-and-paste lots of boilerplate per string tested, when we can consolidate that behind helper functions. Plus, this adds a bit more coverage (we now test all strings both with and without endptr, whereas before some tests skipped the NULL endptr case), which exposed a SEGFAULT on qemu_strtosz(NULL, NULL, &val) that will be fixed in an upcoming patch. Note that duplicating boilerplate has one advantage lost here - a failed test tells you which line number failed; but a helper function does not show the call stack that reached the failure. Since we call the helper more than once within many of the "unit tests", even the unit test name doesn't point out which call is failing. But that only matters when tests fail (they normally pass); at which point I'm debugging the failures under gdb anyways, so I'm not too worried about it. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-12-eblake@redhat.com>
* test-cutils: Prepare for upcoming semantic change in qemu_strtoszEric Blake2023-06-021-15/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A quick search for 'qemu_strtosz' in the code base shows that outside of the testsuite, the ONLY place that passes a non-NULL pointer to @endptr of any variant of a size parser is in hmp.c (the 'o' parser of monitor_parse_arguments), and that particular caller warns of "extraneous characters at the end of line" unless the trailing bytes are purely whitespace. Thus, it makes no semantic difference at the high level whether we parse "1.5e1k" as "1" + ".5e1" + "k" (an attempt to use scientific notation in strtod with a scaling suffix of 'k' with no trailing junk, but which qemu_strtosz says should fail with EINVAL), or as "1.5e" + "1k" (a valid size with scaling suffix of 'e' for exabytes, followed by two junk bytes) - either way, any user passing such a string will get an error message about a parse failure. However, an upcoming patch to qemu_strtosz will fix other corner case bugs in handling the fractional portion of a size, and in doing so, it is easier to declare that qemu_strtosz() itself stops parsing at the first 'e' rather than blindly consuming whatever strtod() will recognize. Once that is fixed, the difference will be visible at the low level (getting a valid parse with trailing garbage when @endptr is non-NULL, while continuing to get -EINVAL when @endptr is NULL); this is easier to demonstrate by moving the affected strings from test_qemu_strtosz_invalid() (which declares them as always -EINVAL) to test_qemu_strtosz_trailing() (where @endptr affects behavior, for now with FIXME comments). Note that a similar argument could be made for having "0x1.5" or "0x1M" parse as 0x1 with ".5" or "M" as trailing junk, instead of blindly treating it as -EINVAL; however, as these cases do not suffer from the same problems as floating point, they are not worth changing at this time. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-11-eblake@redhat.com>
* test-cutils: Add coverage of qemu_strtodEric Blake2023-06-021-0/+512
| | | | | | | | | | | | | | | | | | | | | | | | | | | It's hard to tweak code for consistency if I can't prove what will or won't break from those tweaks. Time to add unit tests for qemu_strtod() and qemu_strtod_finite(). Among other things, I wrote a check whether we have C99 semantics for strtod("0x1") (which MUST parse hex numbers) rather than C89 (which must stop parsing at 'x'). These days, I suspect that is okay; but if it fails CI checks, knowing the difference will help us decide what we want to do about it. Note that C2x, while not final at the time of this patch, has been considering whether to make strtol("0b1") parse as 1 with no slop instead of the C17 parse of 0 with slop "b1"; that decision may also bleed over to strtod(). But for now, I didn't think it worth adding unit tests on that front (to strtol or strtod) as things may still change. Likewise, there are plenty more corner cases of strtod proper that I don't explicitly test here, but there are enough unit tests added here that it covers all the branches reached in our wrappers. In particular, it demonstrates the difference on when *value is left uninitialized, which an upcoming patch will normalize. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-10-eblake@redhat.com>
* cutils: Allow NULL endptr in parse_uint()Eric Blake2023-06-021-2/+16
| | | | | | | | | | | | | | | | All the qemu_strto*() functions permit a NULL endptr, just like their libc counterparts, leaving parse_uint() as the oddball that caused SEGFAULT on NULL and required the user to call parse_uint_full() instead. Relax things for consistency, even though the testsuite is the only impacted caller. Add one more unit test to ensure even parse_uint_full(NULL, 0, &value) works. This also fixes our code to uniformly favor EINVAL over ERANGE when both apply. Also fixes a doc mismatch @v vs. a parameter named value. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-9-eblake@redhat.com>
* cutils: Adjust signature of parse_uint[_full]Eric Blake2023-06-021-66/+53
| | | | | | | | | | | | | | | | | | | | | | | | It's already confusing that we have two very similar functions for wrapping the parse of a 64-bit unsigned value, differing mainly on whether they permit leading '-'. Adjust the signature of parse_uint() and parse_uint_full() to be like all of qemu_strto*(): put the result parameter last, use the same types (uint64_t and unsigned long long have the same width, but are not always the same type), and mark endptr const (this latter change only affects the rare caller of parse_uint). Adjust all callers in the tree. While at it, note that since cutils.c already includes: QEMU_BUILD_BUG_ON(sizeof(int64_t) != sizeof(long long)); we are guaranteed that the result of parse_uint* cannot exceed UINT64_MAX (or the build would have failed), so we can drop pre-existing dead comparisons in opts-visitor.c that were never false. Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-8-eblake@redhat.com> [eblake: Drop dead code spotted by Markus] Signed-off-by: Eric Blake <eblake@redhat.com>
* cutils: Fix wraparound parsing in qemu_strtouiEric Blake2023-06-021-11/+9
| | | | | | | | | | | | | | | | | | | | | | | | While we were matching 32-bit strtol in qemu_strtoi, our use of a 64-bit parse was leaking through for some inaccurate answers in qemu_strtoui in comparison to a 32-bit strtoul (see the unit test for examples). The comment for that function even described what we have to do for a correct parse, but didn't implement it correctly: since strtoull checks for overflow against the wrong values and then negates, we have to temporarily undo negation before checking for overflow against our desired value. Our int wrappers would be a lot easier to write if libc had a guaranteed 32-bit parser even on platforms with 64-bit long. Whether we parse C2x binary strings like "0b1000" is currently up to what libc does; our unit tests intentionally don't cover that at the moment, though. Fixes: 473a2a331e ("cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types", v2.12.0) Signed-off-by: Eric Blake <eblake@redhat.com> CC: qemu-stable@nongnu.org Message-Id: <20230522190441.64278-6-eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
* test-cutils: Test more integer corner casesEric Blake2023-06-021-63/+862
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have quite a few undertested and underdocumented integer parsing corner cases. To ensure that any changes we make in the code are intentional rather than accidental semantic changes, it is time to add more unit tests of existing behavior. In particular, this demonstrates that parse_uint() and qemu_strtou64() behave differently. For "-0", it's hard to argue why parse_uint needs to reject it (it's not a negative integer), but the documentation sort of mentions it; but it is intentional that all other negative values are treated as ERANGE with value 0 (compared to qemu_strtou64() treating "-2" as success and UINT64_MAX-1, for example). Also, when mixing overflow/underflow with a check for no trailing junk, parse_uint_full favors ERANGE over EINVAL, while qemu_strto[iu]* favor EINVAL. This behavior is outside the C standard, so we can pick whatever we want, but it would be nice to be consistent. Note that C requires that "9223372036854775808" fail strtoll() with ERANGE/INT64_MAX, but "-9223372036854775808" pass with INT64_MIN; we weren't testing this. For strtol(), the behavior depends on whether long is 32- or 64-bits (the cutoff point either being the same as strtoll() or at "-2147483648"). Meanwhile, C is clear that "-18446744073709551615" pass stroull() (but not strtoll) with value 1, even though we want it to fail parse_uint(). And although qemu_strtoui() has no C counterpart, it makes more sense if we design it like 32-bit strtoul() (that is, where "-4294967296" be an alternate acceptable spelling for "1", but "-0xffffffff00000001" should be treated as overflow and return 0xffffffff rather than 1). We aren't there yet, so some of the tests added in this patch have FIXME comments. However, note that C2x will (likely) be adding a SILENT semantic change, where C17 strtol("0b1", &ep, 2) returns 0 with ep="b1", but C2x will have it return 1 with ep="". I did not feel like adding testing for those corner cases, in part because the next version of C is not standard and libc support for binary parsing is not yet wide-spread (as of this patch, glibc.git still misparses bare "0b": https://sourceware.org/bugzilla/show_bug.cgi?id=30371). Message-Id: <20230522190441.64278-5-eblake@redhat.com> [eblake: fix a few typos spotted by Hanna] Reviewed-by: Hanna Czenczek <hreitz@redhat.com> [eblake: fix typo on platforms with 32-bit long] Signed-off-by: Eric Blake <eblake@redhat.com>
* test-cutils: Test integral qemu_strto* value on failuresEric Blake2023-06-021-7/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We are inconsistent on the contents of *value after a strto* parse failure. I found the following behaviors: - parse_uint() and parse_uint_full(), which document that *value is slammed to 0 on all EINVAL failures and 0 or UINT_MAX on ERANGE failures, and has unit tests for that (note that parse_uint requires non-NULL endptr, and does not fail with EINVAL for trailing junk) - qemu_strtosz(), which leaves *value untouched on all failures (both EINVAL and ERANGE), and has unit tests but not documentation for that - qemu_strtoi() and other integral friends, which document *value on ERANGE failures but is unspecified on EINVAL (other than implicitly by comparison to libc strto*); there, *value is untouched for NULL string, slammed to 0 on no conversion, and left at the prefix value on NULL endptr; unit tests do not consistently check the value - qemu_strtod(), which documents *value on ERANGE failures but is unspecified on EINVAL; there, *value is untouched for NULL string, slammed to 0.0 for no conversion, and left at the prefix value on NULL endptr; there are no unit tests (other than indirectly through qemu_strtosz) - qemu_strtod_finite(), which documents *value on ERANGE failures but is unspecified on EINVAL; there, *value is left at the prefix for 'inf' or 'nan' and untouched in all other cases; there are no unit tests (other than indirectly through qemu_strtosz) Upcoming patches will change behaviors for consistency, but it's best to first have more unit test coverage to see the impact of those changes. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-4-eblake@redhat.com>
* test-cutils: Use g_assert_cmpuint where appropriateEric Blake2023-06-021-74/+74
| | | | | | | | | | | | | | | | | When debugging test failures, seeing unsigned values as large positive values rather than negative values matters (assuming glib 2.78+; given that I just fixed a bug in glib 2.76 [1] where g_assert_cmpuint displays signed instead of unsigned values). No impact when the test is passing, but using a consistent style will matter more in upcoming test additions. Also, some tests are better with cmphex. While at it, fix some spacing and minor typing issues spotted nearby. [1] https://gitlab.gnome.org/GNOME/glib/-/issues/2997 Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-3-eblake@redhat.com>