Peter Xu <pet...@redhat.com> wrote: > On Tue, Aug 08, 2017 at 11:19:35AM +0200, Juan Quintela wrote: >> Peter Xu <pet...@redhat.com> wrote: >> > On Mon, Jul 17, 2017 at 03:42:30PM +0200, Juan Quintela wrote: >> > >> > [...] >> > >> >> int multifd_load_setup(void) >> >> { >> >> int thread_count; >> >> - uint8_t i; >> >> >> >> if (!migrate_use_multifd()) { >> >> return 0; >> >> } >> >> thread_count = migrate_multifd_threads(); >> >> multifd_recv_state = g_malloc0(sizeof(*multifd_recv_state)); >> >> - multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count); >> >> + multifd_recv_state->params = g_new0(MultiFDRecvParams *, >> >> thread_count); >> >> multifd_recv_state->count = 0; >> >> - for (i = 0; i < thread_count; i++) { >> >> - char thread_name[16]; >> >> - MultiFDRecvParams *p = &multifd_recv_state->params[i]; >> >> - >> >> - qemu_mutex_init(&p->mutex); >> >> - qemu_sem_init(&p->sem, 0); >> >> - p->quit = false; >> >> - p->id = i; >> >> - snprintf(thread_name, sizeof(thread_name), "multifdrecv_%d", i); >> >> - qemu_thread_create(&p->thread, thread_name, multifd_recv_thread, >> >> p, >> >> - QEMU_THREAD_JOINABLE); >> >> - multifd_recv_state->count++; >> >> - } >> > >> > Could I ask why we explicitly switched from MultiFDRecvParams[] array >> > into a pointer array? Can we still use the old array? Thanks, >> >> Now, we could receive the channels out of order (the wonders of >> networking). So, we have two options that I can see: >> >> * Add interesting global locking to be able to modify inplace (I know >> that it should be safe, but yet). >> * Create a new struct in the new connection, and then atomically switch >> the pointer to the right instruction. >> >> I can assure you that the second one makes it much more easier to detect >> when you use the "channel" before you have fully created it O:-) > > Oh, so it's possible that we start to recv pages even if the recv > channel has not yet been established... > > Then would current code be problematic? Like in multifd_recv_page() we > have: > > static void multifd_recv_page(uint8_t *address, uint16_t fd_num) > { > ... > p = multifd_recv_state->params[fd_num]; > qemu_sem_wait(&p->ready); > ... > } > > Here can p==NULL if channel is not ready yet? > > (If so, I think a static array makes more sense...)
Yeap. If we make an error (and believe me that I did), we get a "nice" segmentation fault, where we can see what fd_num is. Otherwise we receive a hang qemu. I know what I preffer O:-) Later, Juan.