Re: [Qemu-devel] [PATCH v7 19/42] MIG_CMD_PACKAGED: Send a packaged chunk of migration stream

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Amit Shah (amit.shah@redhat.com) wrote:
> On (Tue) 16 Jun 2015 [11:26:32], Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > 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.
> 
> Not sure why this is necessary.  I suppose I'll have to go read the
> documentation in patch 1..

Yep - or one of the previous replies where I explained it.

> However:
> 
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -718,6 +718,50 @@ void qemu_savevm_send_open_return_path(QEMUFile *f)
> >      qemu_savevm_command_send(f, MIG_CMD_OPEN_RETURN_PATH, 0, NULL);
> >  }
> >  
> > +/* We have a buffer of data to send; we don't want that all to be loaded
> > + * by the command itself, so the command contains just the length of the
> > + * extra buffer that we then send straight after it.
> > + * TODO: Must be a better way to organise that
> > + *
> > + * Returns:
> > + *    0 on success
> > + *    -ve on error
> > + */
> > +int qemu_savevm_send_packaged(QEMUFile *f, const QEMUSizedBuffer *qsb)
> > +{
> > +    size_t cur_iov;
> > +    size_t len = qsb_get_length(qsb);
> > +    uint32_t tmp;
> > +
> > +    if (len > MAX_VM_CMD_PACKAGED_SIZE) {
> > +        error_report("%s: Unreasonably large packaged state: %zu",
> > +                     __func__, len);
> > +        return -1;
> > +    }
> > +
> > +    tmp = cpu_to_be32(len);
> > +
> > +    trace_qemu_savevm_send_packaged();
> > +    qemu_savevm_command_send(f, MIG_CMD_PACKAGED, 4, (uint8_t *)&tmp);
> > +
> > +    /* all the data follows (concatinating the iov's) */
> > +    for (cur_iov = 0; cur_iov < qsb->n_iov; cur_iov++) {
> > +        /* The iov entries are partially filled */
> > +        size_t towrite = (qsb->iov[cur_iov].iov_len > len) ?
> > +                              len :
> > +                              qsb->iov[cur_iov].iov_len;
> 
> If iov_len was > len, we only wrote part of the current buffer, and we
> skip to the next?

Yes; this is just the end case; the qsb allocates iov entries in 'chunks'
but then the data that gets added often doesn't use the whole chunk.
'len'  - set above from qsb_get_length - gives the used contents of
the qsb, and that's all we want to write.  This is normally the case
on the last entry in the qsb anyway; however since 'len' gets
decremented by the amount written we might go once more around the loop
and the 'if (!towrite) { break; }' might break us out of the loop instead.

Dave

> 
> 
> 		Amit
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK