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...) Thanks, -- Peter Xu