Re: [Qemu-devel] [PATCH v7 30/42] Page request: Add MIG_RP_MSG_REQ_PAGES reverse command

[Amit Shah]

On (Tue) 16 Jun 2015 [11:26:43], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> Add MIG_RP_MSG_REQ_PAGES command on Return path for the postcopy
> destination to request a page from the source.
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>

> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -113,6 +113,36 @@ static void deferred_incoming_migration(Error **errp)
>      deferred_incoming = true;
>  }
>  
> +/* Request a range of pages from the source VM at the given
> + * start address.
> + *   rbname: Name of the RAMBlock to request the page in, if NULL it's the 
> same
> + *           as the last request (a name must have been given previously)

Why not just send the name all the time?

> @@ -1010,6 +1058,28 @@ static void *source_return_path_thread(void *opaque)
>              trace_source_return_path_thread_pong(tmp32);
>              break;
>  
> +        case MIG_RP_MSG_REQ_PAGES:
> +            start = be64_to_cpup((uint64_t *)buf);
> +            len = be64_to_cpup(((uint64_t *)buf)+1);
> +            tmpstr = NULL;
> +            if (len & 1) {
> +                len -= 1; /* Remove the flag */
> +                /* Now we expect an idstr */
> +                tmp32 = buf[16]; /* Length of the following idstr */
> +                tmpstr = (char *)&buf[17];
> +                buf[17+tmp32] = '\0';
> +                expected_len = 16+1+tmp32;

Whitespace missing around operators

> +            } else {
> +                expected_len = 16;
> +            }

This else can be removed if expected_len is set before the if


                Amit