On Fri, Mar 13, 2015 at 11:51:42AM +0000, Dr. David Alan Gilbert wrote: > * David Gibson (da...@gibson.dropbear.id.au) wrote: > > On Wed, Feb 25, 2015 at 04:51:41PM +0000, Dr. David Alan Gilbert (git) > > wrote: > > > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > > > > > > MIG_CMD_PACKAGED is a migration command that allows a chunk > > > of migration stream to be sent in one go, and be received by > > > a separate instance of the loadvm loop while not interacting > > > with the migration stream. > > > > Hrm. I'd be more comfortable if the semantics of CMD_PACKAGED were > > defined in terms of visible effects on the other end, rather than in > > terms of how it's implemented internally. > > > > > This is used by postcopy to load device state (from the package) > > > while loading memory pages from the main stream. > > > > Which makes the above paragraph a bit misleading - the whole point > > here is that loading the package data *does* interact with the > > migration stream - just that it's the migration stream after the end > > of the package. > > Hmm, how about: > > > MIG_CMD_PACKAGED is a migration command that wraps a chunk of migration > stream inside a package whose length can be determined purely by reading > its header. The destination guarantees that the whole MIG_CMD_PACKAGED is > read off the stream prior to parsing the contents. > > This is used by postcopy to load device state (from the package) > while leaving the main stream free to receive memory pages.
It's an improvement.
I'm still a bit concerned that the semantics of CMD_PACKAGED are
unhealthily bound up with hw the current implementation works.
[snip]
> > > +/* Immediately following this command is a blob of data containing an
> > > embedded
> > > + * chunk of migration stream; read it and load it.
> > > + */
> > > +static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis,
> > > + uint32_t length)
> > > +{
> > > + int ret;
> > > + uint8_t *buffer;
> > > + QEMUSizedBuffer *qsb;
> > > +
> > > + trace_loadvm_handle_cmd_packaged(length);
> > > +
> > > + if (length > MAX_VM_CMD_PACKAGED_SIZE) {
> > > + error_report("Unreasonably large packaged state: %u", length);
> > > + return -1;
> >
> > It would be a good idea to check this on the send side as well as
> > receive, wouldn't it?
>
> Yes, I had been doing that in the postcopy code that called the
> send code; but I've now moved it down into savevm_send_packaged.
Good, better symmetry that way.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgp9J4PsMG8l3.pgp
Description: PGP signature
