- Oct 11, 2023
-
-
Markus Armbruster authored
Functions that use an Error **errp parameter to return errors should not also report them to the user, because reporting is the caller's job. When the caller does, the error is reported twice. When it doesn't (because it recovered from the error), there is no error to report, i.e. the report is bogus. qemu_rdma_write_flush() violates this principle: it calls error_report() via qemu_rdma_write_one(). I elected not to investigate how callers handle the error, i.e. precise impact is not known. Clean this up by converting qemu_rdma_write_one() to Error. Bonus: resolves a FIXME about problematic use of errno. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-41-armbru@redhat.com>
-
Markus Armbruster authored
Functions that use an Error **errp parameter to return errors should not also report them to the user, because reporting is the caller's job. When the caller does, the error is reported twice. When it doesn't (because it recovered from the error), there is no error to report, i.e. the report is bogus. qio_channel_rdma_writev() violates this principle: it calls error_report() via qemu_rdma_write_flush(). I elected not to investigate how callers handle the error, i.e. precise impact is not known. Clean this up by converting qemu_rdma_write_flush() to Error. Necessitates setting an error when qemu_rdma_write_one() failed. Since this error will go away later in this series, simply use "FIXME temporary error message" there. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-40-armbru@redhat.com>
-
Markus Armbruster authored
Functions that use an Error **errp parameter to return errors should not also report them to the user, because reporting is the caller's job. When the caller does, the error is reported twice. When it doesn't (because it recovered from the error), there is no error to report, i.e. the report is bogus. qemu_rdma_exchange_send() violates this principle: it calls error_report() via callback qemu_rdma_reg_whole_ram_blocks(). I elected not to investigate how callers handle the error, i.e. precise impact is not known. Clean this up by converting the callback to Error. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-39-armbru@redhat.com>
-
Markus Armbruster authored
Functions that use an Error **errp parameter to return errors should not also report them to the user, because reporting is the caller's job. When the caller does, the error is reported twice. When it doesn't (because it recovered from the error), there is no error to report, i.e. the report is bogus. qemu_rdma_exchange_send() and qemu_rdma_exchange_recv() violate this principle: they call error_report() via qemu_rdma_exchange_get_response(). I elected not to investigate how callers handle the error, i.e. precise impact is not known. Clean this up by converting qemu_rdma_exchange_get_response() to Error. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-38-armbru@redhat.com>
-
Markus Armbruster authored
Functions that use an Error **errp parameter to return errors should not also report them to the user, because reporting is the caller's job. When the caller does, the error is reported twice. When it doesn't (because it recovered from the error), there is no error to report, i.e. the report is bogus. qio_channel_rdma_writev() violates this principle: it calls error_report() via qemu_rdma_exchange_send(). I elected not to investigate how callers handle the error, i.e. precise impact is not known. Clean this up by converting qemu_rdma_exchange_send() to Error. Necessitates setting an error when qemu_rdma_post_recv_control(), callback(), or qemu_rdma_exchange_get_response() failed. Since these errors will go away later in this series, simply use "FIXME temporary error message" there. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-37-armbru@redhat.com>
-
Markus Armbruster authored
Functions that use an Error **errp parameter to return errors should not also report them to the user, because reporting is the caller's job. When the caller does, the error is reported twice. When it doesn't (because it recovered from the error), there is no error to report, i.e. the report is bogus. qio_channel_rdma_readv() violates this principle: it calls error_report() via qemu_rdma_exchange_recv(). I elected not to investigate how callers handle the error, i.e. precise impact is not known. Clean this up by converting qemu_rdma_exchange_recv() to Error. Necessitates setting an error when qemu_rdma_exchange_get_response() failed. Since this error will go away later in this series, simply use "FIXME temporary error message" there. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-36-armbru@redhat.com>
-
Markus Armbruster authored
These guards are all redundant now. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-35-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_resolve_host() and qemu_rdma_dest_init() iterate over addresses to find one that works, holding onto the first Error from qemu_rdma_broken_ipv6_kernel() for use when no address works. Issues: 1. If @errp was &error_abort or &error_fatal, we'd terminate instead of trying the next address. Can't actually happen, since no caller passes these arguments. 2. When @errp is a pointer to a variable containing NULL, and qemu_rdma_broken_ipv6_kernel() fails, the variable no longer contains NULL. Subsequent iterations pass it again, violating Error usage rules. Dangerous, as setting an error would then trip error_setv()'s assertion. Works only because qemu_rdma_broken_ipv6_kernel() and the code following the loops carefully avoids setting a second error. 3. If qemu_rdma_broken_ipv6_kernel() fails, and then a later iteration finds a working address, @errp still holds the first error from qemu_rdma_broken_ipv6_kernel(). If we then run into another error, we report the qemu_rdma_broken_ipv6_kernel() failure instead. 4. If we don't run into another error, we leak the Error object. Use a local error variable, and propagate to @errp. This fixes 3. and also cleans up 1 and partly 2. Free this error when we have a working address. This fixes 4. Pass the local error variable to qemu_rdma_broken_ipv6_kernel() only until it fails. Pass null on any later iterations. This cleans up the remainder of 2. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-34-armbru@redhat.com>
-
Markus Armbruster authored
ERROR() has become "error_setg() unless an error has been set already". Hiding the conditional in the macro is in the way of further work. Replace the macro uses by their expansion, and delete the macro. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-33-armbru@redhat.com>
-
Markus Armbruster authored
Functions that use an Error **errp parameter to return errors should not also report them to the user, because reporting is the caller's job. When the caller does, the error is reported twice. When it doesn't (because it recovered from the error), there is no error to report, i.e. the report is bogus. Macro ERROR() violates this principle. Delete the error_report() there. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Tested-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-32-armbru@redhat.com>
-
Markus Armbruster authored
When migration capability @rdma-pin-all is true, but the server cannot honor it, qemu_rdma_connect() calls macro ERROR(), then returns success. ERROR() sets an error. Since qemu_rdma_connect() returns success, its caller rdma_start_outgoing_migration() duly assumes @errp is still clear. The Error object leaks. ERROR() additionally reports the situation to the user as an error: RDMA ERROR: Server cannot support pinning all memory. Will register memory dynamically. Is this an error or not? It actually isn't; we disable @rdma-pin-all and carry on. "Correcting" the user's configuration decisions that way feels problematic, but that's a topic for another day. Replace ERROR() by warn_report(). This plugs the memory leak, and emits a clearer message to the user. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-31-armbru@redhat.com>
-
Markus Armbruster authored
When a function returns 0 on success, negative value on error, checking for non-zero suffices, but checking for negative is clearer. So do that. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-30-armbru@redhat.com>
-
Markus Armbruster authored
Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-29-armbru@redhat.com>
-
Markus Armbruster authored
All we do with the value of RDMAContext member @error_state is test whether it's zero. Change to bool and rename to @errored. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-28-armbru@redhat.com>
-
Markus Armbruster authored
This is just to make the error value more obvious. Callers don't mind. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-27-armbru@redhat.com>
-
Markus Armbruster authored
Several functions return negative errno codes on failure. Callers check for specific codes exactly never. For some of the functions, callers couldn't check even if they wanted to, because the functions also return negative values that aren't errno codes, leaving readers confused on what the function actually returns. Clean up and simplify: return -1 instead of negative errno code. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-26-armbru@redhat.com>
-
Markus Armbruster authored
rdma_getaddrinfo() returns 0 on success. On error, it returns one of the EAI_ error codes like getaddrinfo() does, or -1 with errno set. This is broken by design: POSIX implicitly specifies the EAI_ error codes to be non-zero, no more. They could clash with -1. Nothing we can do about this design flaw. Both callers of rdma_getaddrinfo() only recognize negative values as error. Works only because systems elect to make the EAI_ error codes negative. Best not to rely on that: change the callers to treat any non-zero value as failure. Also change them to return -1 instead of the value received from getaddrinfo() on failure, to avoid positive error values. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-25-armbru@redhat.com>
-
Markus Armbruster authored
The QEMUFileHooks methods don't come with a written contract. Digging through the code calling them, we find: * save_page(): Negative values RAM_SAVE_CONTROL_DELAYED and RAM_SAVE_CONTROL_NOT_SUPP are special. Any other negative value is an unspecified error. qemu_rdma_save_page() returns -EIO or rdma->error_state on error. I believe the latter is always negative. Nothing stops either of them to clash with the special values, though. Feels unlikely, but fix it anyway to return only the special values and -1. * before_ram_iterate(), after_ram_iterate(): Negative value means error. qemu_rdma_registration_start() and qemu_rdma_registration_stop() comply as far as I can tell. Make them comply *obviously*, by returning -1 on error. * hook_ram_load: Negative value means error. rdma_load_hook() already returns -1 on error. Leave it alone. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-24-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_data_init() neglects to set an Error when it fails because @host_port is null. Fortunately, no caller passes null, so this is merely a latent bug. Drop the flawed code handling null argument. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-23-armbru@redhat.com>
-
Markus Armbruster authored
qemu_get_cm_event_timeout() neglects to set an error when it fails because rdma_get_cm_event() fails. Harmless, as its caller qemu_rdma_connect() substitutes a generic error then. Fix it anyway. qemu_rdma_connect() also sets the generic error when its own call of rdma_get_cm_event() fails. Make the error handling more obvious: set a specific error right after rdma_get_cm_event() fails. Delete the generic error. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Peter Xu <peterx@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-22-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_resolve_host() and qemu_rdma_dest_init() try addresses until they find on that works. If none works, they return the first Error set by qemu_rdma_broken_ipv6_kernel(), or else return a generic one. qemu_rdma_broken_ipv6_kernel() neglects to set an Error when ibv_open_device() fails. If a later address fails differently, we use that Error instead, or else the generic one. Harmless enough, but needs fixing all the same. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-21-armbru@redhat.com>
-
Markus Armbruster authored
Hiding return statements in macros is a bad idea. Use a function instead, and open code the return part. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-20-armbru@redhat.com>
-
Markus Armbruster authored
QIOChannelClass methods qio_channel_rdma_readv() and qio_channel_rdma_writev() violate their method contract when rdma->error_state is non-zero: 1. They return whatever is in rdma->error_state then. Only -1 will be fine. -2 will be misinterpreted as "would block". Anything less than -2 isn't defined in the contract. A positive value would be misinterpreted as success, but I believe that's not actually possible. 2. They neglect to set an error then. If something up the call stack dereferences the error when failure is returned, it will crash. If it ignores the return value and checks the error instead, it will miss the error. Crap like this happens when return statements hide in macros, especially when their uses are far away from the definition. I elected not to investigate how callers are impacted. Expand the two bad macro uses, so we can set an error and return -1. The next commit will then get rid of the macro altogether. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-19-armbru@redhat.com>
-
Markus Armbruster authored
Several error messages include numeric error codes returned by failed functions: * ibv_poll_cq() returns an unspecified negative value. Useless. * rdma_accept and rdma_get_cm_event() return -1. Useless. * qemu_rdma_poll() returns either -1 or an unspecified negative value. Useless. * qemu_rdma_block_for_wrid(), qemu_rdma_write_flush(), qemu_rdma_exchange_send(), qemu_rdma_exchange_recv(), qemu_rdma_write() return a negative value that may or may not be an errno value. While reporting human-readable errno information (which a number is not) can be useful, reporting an error code that may or may not be an errno value is useless. Drop these error codes from the error messages. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-18-armbru@redhat.com>
-
Markus Armbruster authored
We use errno after calling Libibverbs functions that are not documented to set errno (manual page does not mention errno), or where the documentation is unclear ("returns [...] the value of errno on failure"). While this could be read as "sets errno and returns it", a glance at the source code[*] kills that hope: static inline int ibv_post_send(struct ibv_qp *qp, struct ibv_send_wr *wr, struct ibv_send_wr **bad_wr) { return qp->context->ops.post_send(qp, wr, bad_wr); } The callback can be static int mana_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, struct ibv_send_wr **bad) { /* This version of driver supports RAW QP only. * Posting WR is done directly in the application. */ return EOPNOTSUPP; } Neither of them touches errno. One of these errno uses is easy to fix, so do that now. Several more will go away later in the series; add temporary FIXME commments. Three will remain; add TODO comments. TODO, not FIXME, because the bug might be in Libibverbs documentation. [*] https://github.com/linux-rdma/rdma-core.git commit 55fa316b4b18f258d8ac1ceb4aa5a7a35b094dcf Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-17-armbru@redhat.com>
-
Markus Armbruster authored
@error_reported and @received_error are flags. The latter is even assigned bool true. Change them from int to bool. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-16-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_buffer_mergeable() is semantically a predicate. It returns int 0 or 1. Return bool instead, and fix the function name's spelling. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-15-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_search_ram_block() can't fail. Return void, and drop the unreachable error handling. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-14-armbru@redhat.com>
-
Markus Armbruster authored
rdma_add_block() can't fail. Return void, and drop the unreachable error handling. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-13-armbru@redhat.com>
-
Markus Armbruster authored
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. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-12-armbru@redhat.com>
-
Markus Armbruster authored
include/qapi/error.h demands: * - Functions that use Error to report errors have an Error **errp * parameter. It should be the last parameter, except for functions * taking variable arguments. qemu_rdma_connect() does not conform. Clean it up. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-11-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_accept() returns 0 in some cases even when it didn't complete its job due to errors. Impact is not obvious. I figure the caller will soon fail again with a misleading error message. Fix it to return -1 on any failure. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-10-armbru@redhat.com>
-
Markus Armbruster authored
Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-9-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_exchange_get_response() compares int parameter @expecting with uint32_t head->type. Actual arguments are non-negative enumeration constants, RDMAControlHeader uint32_t member type, or qemu_rdma_exchange_recv() int parameter expecting. Actual arguments for the latter are non-negative enumeration constants. Change both parameters to uint32_t. In qio_channel_rdma_readv(), loop control variable @i is ssize_t, and counts from 0 up to @niov, which is size_t. Change @i to size_t. While there, make qio_channel_rdma_readv() and qio_channel_rdma_writev() more consistent: change the former's @done to ssize_t, and delete the latter's useless initialization of @len. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-8-armbru@redhat.com>
-
Markus Armbruster authored
qio_channel_rdma_readv() assigns the size_t value of qemu_rdma_fill() to an int variable before it adds it to @done / subtracts it from @want, both size_t. Truncation when qemu_rdma_fill() copies more than INT_MAX bytes. Seems vanishingly unlikely, but needs fixing all the same. Fixes: 6ddd2d76 (migration: convert RDMA to use QIOChannel interface) Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-7-armbru@redhat.com>
-
Markus Armbruster authored
We use int instead of uint64_t in a few places. Change them to uint64_t. This cleans up a comparison of signed qemu_rdma_block_for_wrid() parameter @wrid_requested with unsigned @wr_id. Harmless, because the actual arguments are non-negative enumeration constants. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-6-armbru@redhat.com>
-
Markus Armbruster authored
wrid_desc[] uses 4001 pointers to map four integer values to strings. print_wrid() accesses wrid_desc[] out of bounds when passed a negative argument. It returns null for values 2..1999 and 2001..3999. qemu_rdma_poll() and qemu_rdma_block_for_wrid() print wrid_desc[wr_id] and passes print_wrid(wr_id) to tracepoints. Could conceivably crash trying to format a null string. I believe access out of bounds is not possible. Not worth cleaning up. Dumb down to show just numeric wr_id. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-5-armbru@redhat.com>
-
Markus Armbruster authored
rdma_delete_block() always returns 0, which its only caller ignores. Return void instead. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-4-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_data_init() return type is void *. It actually returns RDMAContext *, and all its callers assign the value to an RDMAContext *. Unclean. Return RDMAContext * instead. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-3-armbru@redhat.com>
-
Markus Armbruster authored
qemu_rdma_poll()'s return type is uint64_t, even though it returns 0, -1, or @ret, which is int. Its callers assign the return value to int variables, then check whether it's negative. Unclean. Return int instead. Signed-off-by:
Markus Armbruster <armbru@redhat.com> Reviewed-by:
Fabiano Rosas <farosas@suse.de> Reviewed-by:
Li Zhijian <lizhijian@fujitsu.com> Reviewed-by:
Juan Quintela <quintela@redhat.com> Signed-off-by:
Juan Quintela <quintela@redhat.com> Message-ID: <20230928132019.2544702-2-armbru@redhat.com>
-