On 27 June 2018 at 13:55, Juan Quintela <quint...@redhat.com> wrote: > The function still don't use multifd, but we have simplified > ram_save_page, xbzrle and RDMA stuff is gone. We have added a new > counter. > > Signed-off-by: Juan Quintela <quint...@redhat.com> > Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > > -- > Add last_page parameter > Add commets for done and address > Remove multifd field, it is the same than normal pages > Merge next patch, now we send multiple pages at a time > Remove counter for multifd pages, it is identical to normal pages > Use iovec's instead of creating the equivalent. > Clear memory used by pages (dave) > Use g_new0(danp) > define MULTIFD_CONTINUE > now pages member is a pointer > Fix off-by-one in number of pages in one packet > Remove RAM_SAVE_FLAG_MULTIFD_PAGE > s/multifd_pages_t/MultiFDPages_t/ > add comment explaining what it means > ---
Coverity has flagged an issue with this code... > +static void multifd_send_pages(void) > +{ > + int i; > + static int next_channel; > + MultiFDSendParams *p = NULL; /* make happy gcc */ > + MultiFDPages_t *pages = multifd_send_state->pages; > + uint64_t transferred; [...] > + transferred = pages->used * TARGET_PAGE_SIZE + p->packet_len; This is the usual "32-bit multiply that we then put into a 64-bit result" -- needs a cast to force the multiply to give you a 64-bit result. (CID 1393783) > + ram_counters.multifd_bytes += transferred; > + ram_counters.transferred += transferred;; Stray double semicolon, I just noticed while writing this email. Coverity is also unconvinced by the locking strategies this code is using -- eg CID 1393775 (which I'm pretty sure is a false positive), and CID 1393778 (which is less clear), plus a lot of others we've silenced as false-positives. Somebody who understands the locking should probably go through the coverity issues for the 'migration' component. thanks -- PMM