* Amit Shah (amit.s...@redhat.com) wrote:
> On (Tue) 16 Jun 2015 [11:26:43], Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >
> > 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 <dgilb...@redhat.com>
>
> > --- 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 / dgilb...@redhat.com / Manchester, UK
