* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> 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>
> > ---
> > include/migration/migration.h | 4 +++
> > migration/migration.c | 70
> > +++++++++++++++++++++++++++++++++++++++++++
> > trace-events | 1 +
> > 3 files changed, 75 insertions(+)
> >
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 68a1731..8742d53 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -47,6 +47,8 @@ enum mig_rp_message_type {
> > MIG_RP_MSG_INVALID = 0, /* Must be 0 */
> > MIG_RP_MSG_SHUT, /* sibling will not send any more RP messages
> > */
> > MIG_RP_MSG_PONG, /* Response to a PING; data (seq: be32 ) */
> > +
> > + MIG_RP_MSG_REQ_PAGES, /* data (start: be64, len: be64) */
>
> Not that I really care, buht I think that leng could be 32bits. I am
> not seing networking getting good at multigigabytes transfers soon O:-)
Done.
> > +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char*
> > rbname,
> > + ram_addr_t start, ram_addr_t len);
>
> Shouldn't len be a size_t?
> (yes, I know that migration code is not really consistent about that)
Done (fun combination with the change above, but still)
> > void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
> > void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 3e5a7c8..0373b77 100644
> > --- 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)
> > + * Start: Address offset within the RB
> > + * Len: Length in bytes required - must be a multiple of pagesize
> > + */
> > +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char
> > *rbname,
> > + ram_addr_t start, ram_addr_t len)
> > +{
> > + uint8_t bufc[16+1+255]; /* start (8 byte), len (8 byte), rbname upto
> > 256 */
> > + uint64_t *buf64 = (uint64_t *)bufc;
> > + size_t msglen = 16; /* start + len */
> > +
> > + assert(!(len & 1));
>
> ohhhh, why can't we get a real flags field?
>
> Scratch that. Seeing the rest of the code, can't we have two commands:
>
> MIG_RP_MSG_REQ_PAGES
> MIG_RP_MSG_REQ_PAGES_WITH_ID
>
> I am not really sure that it makes sense getting a command that can be
> of two different lengths only for that?
>
> I am not sure, but having a command with two different payloads look
> strange.
Done (I made it _ID rather than WITH_ID - it was getting a bit long).
Dave
>
> Later, Juan.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
