c05dfcb7 | 29-Feb-2024 |
Fabiano Rosas <farosas@suse.de> |
io: fsync before closing a file channel
Make sure the data is flushed to disk before closing file channels. This is to ensure data is on disk and not lost in the event of a host crash.
This is curr
io: fsync before closing a file channel
Make sure the data is flushed to disk before closing file channels. This is to ensure data is on disk and not lost in the event of a host crash.
This is currently being implemented to affect the migration code when migrating to a file, but all QIOChannelFile users should benefit from the change.
Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com> Acked-by: "Daniel P. Berrangé" <berrange@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20240229153017.2221-6-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com>
show more ...
|
0478b030 | 29-Feb-2024 |
Nikolay Borisov <nborisov@suse.com> |
io: implement io_pwritev/preadv for QIOChannelFile
The upcoming 'mapped-ram' feature will require qemu to write data to (and restore from) specific offsets of the migration file.
Add a minimal impl
io: implement io_pwritev/preadv for QIOChannelFile
The upcoming 'mapped-ram' feature will require qemu to write data to (and restore from) specific offsets of the migration file.
Add a minimal implementation of pwritev/preadv and expose them via the io_pwritev and io_preadv interfaces.
Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20240229153017.2221-5-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com>
show more ...
|
f1cfe394 | 29-Feb-2024 |
Nikolay Borisov <nborisov@suse.com> |
io: Add generic pwritev/preadv interface
Introduce basic pwritev/preadv support in the generic channel layer. Specific implementation will follow for the file channel as this is required in order to
io: Add generic pwritev/preadv interface
Introduce basic pwritev/preadv support in the generic channel layer. Specific implementation will follow for the file channel as this is required in order to support migration streams with fixed location of each ram page.
Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20240229153017.2221-4-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com>
show more ...
|
06e0f098 | 30-Aug-2023 |
Stefan Hajnoczi <stefanha@redhat.com> |
io: follow coroutine AioContext in qio_channel_yield()
The ongoing QEMU multi-queue block layer effort makes it possible for multiple threads to process I/O in parallel. The nbd block driver is not
io: follow coroutine AioContext in qio_channel_yield()
The ongoing QEMU multi-queue block layer effort makes it possible for multiple threads to process I/O in parallel. The nbd block driver is not compatible with the multi-queue block layer yet because QIOChannel cannot be used easily from coroutines running in multiple threads. This series changes the QIOChannel API to make that possible.
In the current API, calling qio_channel_attach_aio_context() sets the AioContext where qio_channel_yield() installs an fd handler prior to yielding:
qio_channel_attach_aio_context(ioc, my_ctx); ... qio_channel_yield(ioc); // my_ctx is used here ... qio_channel_detach_aio_context(ioc);
This API design has limitations: reading and writing must be done in the same AioContext and moving between AioContexts involves a cumbersome sequence of API calls that is not suitable for doing on a per-request basis.
There is no fundamental reason why a QIOChannel needs to run within the same AioContext every time qio_channel_yield() is called. QIOChannel only uses the AioContext while inside qio_channel_yield(). The rest of the time, QIOChannel is independent of any AioContext.
In the new API, qio_channel_yield() queries the AioContext from the current coroutine using qemu_coroutine_get_aio_context(). There is no need to explicitly attach/detach AioContexts anymore and qio_channel_attach_aio_context() and qio_channel_detach_aio_context() are gone. One coroutine can read from the QIOChannel while another coroutine writes from a different AioContext.
This API change allows the nbd block driver to use QIOChannel from any thread. It's important to keep in mind that the block driver already synchronizes QIOChannel access and ensures that two coroutines never read simultaneously or write simultaneously.
This patch updates all users of qio_channel_attach_aio_context() to the new API. Most conversions are simple, but vhost-user-server requires a new qemu_coroutine_yield() call to quiesce the vu_client_trip() coroutine when not attached to any AioContext.
While the API is has become simpler, there is one wart: QIOChannel has a special case for the iohandler AioContext (used for handlers that must not run in nested event loops). I didn't find an elegant way preserve that behavior, so I added a new API called qio_channel_set_follow_coroutine_ctx(ioc, true|false) for opting in to the new AioContext model. By default QIOChannel uses the iohandler AioHandler. Code that formerly called qio_channel_attach_aio_context() now calls qio_channel_set_follow_coroutine_ctx(ioc, true) once after the QIOChannel is created.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: Daniel P. Berrangé <berrange@redhat.com> Message-ID: <20230830224802.493686-5-stefanha@redhat.com> [eblake: also fix migration/rdma.c] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
1dd91b22 | 16-Dec-2022 |
Paolo Bonzini <pbonzini@redhat.com> |
io: mark mixed functions that can suspend
There should be no paths from a coroutine_fn to aio_poll, however in practice coroutine_mixed_fn will call aio_poll in the !qemu_in_coroutine() path. By ma
io: mark mixed functions that can suspend
There should be no paths from a coroutine_fn to aio_poll, however in practice coroutine_mixed_fn will call aio_poll in the !qemu_in_coroutine() path. By marking mixed functions, we can track accurately the call paths that execute entirely in coroutine context, and find more missing coroutine_fn markers. This results in more accurate checks that coroutine code does not end up blocking.
If the marking were extended transitively to all functions that call these ones, static analysis could be done much more efficiently. However, this is a start and makes it possible to use vrc's path-based searches to find potential bugs where coroutine_fns call blocking functions.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
c3a2c84a | 06-Mar-2023 |
Matheus Tavares Bernardino <quic_mathbern@quicinc.com> |
io/channel-tls: plug memory leakage on GSource
This leakage can be seen through test-io-channel-tls:
$ ../configure --target-list=aarch64-softmmu --enable-sanitizers $ make ./tests/unit/test-io-cha
io/channel-tls: plug memory leakage on GSource
This leakage can be seen through test-io-channel-tls:
$ ../configure --target-list=aarch64-softmmu --enable-sanitizers $ make ./tests/unit/test-io-channel-tls $ ./tests/unit/test-io-channel-tls
Indirect leak of 104 byte(s) in 1 object(s) allocated from: #0 0x7f81d1725808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144 #1 0x7f81d135ae98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98) #2 0x55616c5d4c1b in object_new_with_propv ../qom/object.c:795 #3 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768 #4 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70 #5 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158 #6 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
Indirect leak of 32 byte(s) in 1 object(s) allocated from: #0 0x7f81d1725a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153 #1 0x7f81d1472a20 in gnutls_dh_params_init (/lib/x86_64-linux-gnu/libgnutls.so.30+0x46a20) #2 0x55616c6485ff in qcrypto_tls_creds_x509_load ../crypto/tlscredsx509.c:634 #3 0x55616c648ba2 in qcrypto_tls_creds_x509_complete ../crypto/tlscredsx509.c:694 #4 0x55616c5e1fea in user_creatable_complete ../qom/object_interfaces.c:28 #5 0x55616c5d4c8c in object_new_with_propv ../qom/object.c:807 #6 0x55616c5d4a83 in object_new_with_props ../qom/object.c:768 #7 0x55616c5c5415 in test_tls_creds_create ../tests/unit/test-io-channel-tls.c:70 #8 0x55616c5c5a6b in test_io_channel_tls ../tests/unit/test-io-channel-tls.c:158 #9 0x7f81d137d58d (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7a58d)
...
SUMMARY: AddressSanitizer: 49143 byte(s) leaked in 184 allocation(s).
The docs for `g_source_add_child_source(source, child_source)` says "source will hold a reference on child_source while child_source is attached to it." Therefore, we should unreference the child source at `qio_channel_tls_read_watch()` after attaching it to `source`. With this change, ./tests/unit/test-io-channel-tls shows no leakages.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
show more ...
|
25657fc6 | 21-Feb-2023 |
Marc-André Lureau <marcandre.lureau@redhat.com> |
win32: replace closesocket() with close() wrapper
Use a close() wrapper instead, so that we don't need to worry about closesocket() vs close() anymore, let's hope.
Signed-off-by: Marc-André Lureau
win32: replace closesocket() with close() wrapper
Use a close() wrapper instead, so that we don't need to worry about closesocket() vs close() anymore, let's hope.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Message-Id: <20230221124802.4103554-17-marcandre.lureau@redhat.com>
show more ...
|
abe34282 | 21-Feb-2023 |
Marc-André Lureau <marcandre.lureau@redhat.com> |
win32: avoid mixing SOCKET and file descriptor space
Until now, a win32 SOCKET handle is often cast to an int file descriptor, as this is what other OS use for sockets. When necessary, QEMU eventual
win32: avoid mixing SOCKET and file descriptor space
Until now, a win32 SOCKET handle is often cast to an int file descriptor, as this is what other OS use for sockets. When necessary, QEMU eventually queries whether it's a socket with the help of fd_is_socket(). However, there is no guarantee of conflict between the fd and SOCKET space. Such conflict would have surprising consequences, we shouldn't mix them.
Also, it is often forgotten that SOCKET must be closed with closesocket(), and not close().
Instead, let's make the win32 socket wrapper functions return and take a file descriptor, and let util/ wrappers do the fd/SOCKET conversion as necessary. A bit of adaptation is necessary in io/ as well.
Unfortunately, we can't drop closesocket() usage, despite _open_osfhandle() documentation claiming transfer of ownership, testing shows bad behaviour if you forget to call closesocket().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Message-Id: <20230221124802.4103554-15-marcandre.lureau@redhat.com>
show more ...
|
a4aafea2 | 21-Feb-2023 |
Marc-André Lureau <marcandre.lureau@redhat.com> |
win32/socket: introduce qemu_socket_unselect() helper
A more explicit version of qemu_socket_select() with no events.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Ste
win32/socket: introduce qemu_socket_unselect() helper
A more explicit version of qemu_socket_select() with no events.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Message-Id: <20230221124802.4103554-8-marcandre.lureau@redhat.com>
show more ...
|
f5fd677a | 21-Feb-2023 |
Marc-André Lureau <marcandre.lureau@redhat.com> |
win32/socket: introduce qemu_socket_select() helper
This is a wrapper for WSAEventSelect, with Error handling. By default, it will produce a warning, so callers don't have to be modified now, and ye
win32/socket: introduce qemu_socket_select() helper
This is a wrapper for WSAEventSelect, with Error handling. By default, it will produce a warning, so callers don't have to be modified now, and yet we can spot potential mis-use.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Message-Id: <20230221124802.4103554-7-marcandre.lureau@redhat.com>
show more ...
|
23f77f05 | 06-Oct-2022 |
Bin Meng <bin.meng@windriver.com> |
io/channel-watch: Fix socket watch on Windows
Random failure was observed when running qtests on Windows due to "Broken pipe" detected by qmp_fd_receive(). What happened is that the qtest executable
io/channel-watch: Fix socket watch on Windows
Random failure was observed when running qtests on Windows due to "Broken pipe" detected by qmp_fd_receive(). What happened is that the qtest executable sends testing data over a socket to the QEMU under test but no response is received. The errno of the recv() call from the qtest executable indicates ETIMEOUT, due to the qmp chardev's tcp_chr_read() is never called to receive testing data hence no response is sent to the other side.
tcp_chr_read() is registered as the callback of the socket watch GSource. The reason of the callback not being called by glib, is that the source check fails to indicate the source is ready. There are two socket watch sources created to monitor the same socket event object from the char-socket backend in update_ioc_handlers(). During the source check phase, qio_channel_socket_source_check() calls WSAEnumNetworkEvents() to discover occurrences of network events for the indicated socket, clear internal network event records, and reset the event object. Testing shows that if we don't reset the event object by not passing the event handle to WSAEnumNetworkEvents() the symptom goes away and qtest runs very stably.
It seems we don't need to call WSAEnumNetworkEvents() at all, as we don't parse the result of WSANETWORKEVENTS returned from this API. We use select() to poll the socket status. Fix this instability by dropping the WSAEnumNetworkEvents() call.
Some side notes:
During the testing, I removed the following codes in update_ioc_handlers():
remove_hup_source(s); s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP); g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup, chr, NULL); g_source_attach(s->hup_source, chr->gcontext);
and such change also makes the symptom go away.
And if I moved the above codes to the beginning, before the call to io_add_watch_poll(), the symptom also goes away.
It seems two sources watching on the same socket event object is the key that leads to the instability. The order of adding a source watch seems to also play a role but I can't explain why. Hopefully a Windows and glib expert could explain this behavior.
Signed-off-by: Bin Meng <bin.meng@windriver.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
show more ...
|