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

[Dr. David Alan Gilbert]

* Amit Shah (address@hidden) wrote:
> 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?

It does shrink the messages quite a bit, and you have to do a name lookup
on them when you receive it, rather than just knowing it's the same one.

> > @@ -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

Done.

> > +            } else {
> > +                expected_len = 16;
> > +            }
> 
> This else can be removed if expected_len is set before the if

That's simplified out with the change Juan suggested.

Dave

> 
>               Amit
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK