[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL 10/16] migration: Create ram_save_multifd_page

From: Peter Maydell
Subject: Re: [Qemu-devel] [PULL 10/16] migration: Create ram_save_multifd_page
Date: Fri, 6 Jul 2018 11:57:37 +0100

On 27 June 2018 at 13:55, Juan Quintela <address@hidden> 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 <address@hidden>
> Reviewed-by: Dr. David Alan Gilbert <address@hidden>
> --
> 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)
> now pages member is a pointer
> Fix off-by-one in number of pages in one packet
> 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.

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]