| Message ID | 20231012134343.23757-3-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | migration/multifd: General cleanups | expand |
Fabiano Rosas <farosas@suse.de> wrote: > The preferred usage of the Error type is to always set both the return > code and the error when a failure happens. As all code called from the > send thread follows this pattern, we'll always have the return code > and the error set at the same time. > > Aside from the convention, in this piece of code this must be the > case, otherwise the if (ret != 0) would be exiting the thread without > calling multifd_send_terminate_threads() which is incorrect. > > Unify both paths to make it clear that both are taken when there's an > error. > > Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Juan Quintela <quintela@redhat.com> queued.
diff --git a/migration/multifd.c b/migration/multifd.c index a7c7a947e3..bc3994c567 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -752,19 +752,13 @@ static void *multifd_send_thread(void *opaque) } out: - if (local_err) { + if (ret) { + assert(local_err); trace_multifd_send_error(p->id); multifd_send_terminate_threads(local_err); - error_free(local_err); - } - - /* - * Error happen, I will exit, but I can't just leave, tell - * who pay attention to me. - */ - if (ret != 0) { qemu_sem_post(&p->sem_sync); qemu_sem_post(&multifd_send_state->channels_ready); + error_free(local_err); } qemu_mutex_lock(&p->mutex);
The preferred usage of the Error type is to always set both the return code and the error when a failure happens. As all code called from the send thread follows this pattern, we'll always have the return code and the error set at the same time. Aside from the convention, in this piece of code this must be the case, otherwise the if (ret != 0) would be exiting the thread without calling multifd_send_terminate_threads() which is incorrect. Unify both paths to make it clear that both are taken when there's an error. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- migration/multifd.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)