* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@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.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > Reviewed-by: Amit Shah <amit.s...@redhat.com>
>
>
> Reviewed-by: Juan Quintela <quint...@redhat.com>
>
> But I propose the change below
>
>
> > + size_t len = qsb_get_length(qsb);
>
> ....
>
> > + /* 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;
>
> Or something have been very wrong here, or qsb->iov[cur_iov].iov_len can
> never be > len. So this should be the same than:
>
> size_t towrite = MIN(qsb->iov[cur_iov].iov_len, len);
>
> right?
Done.
> > + len -= towrite;
> > +
> > + if (!towrite) {
> > + break;
> > + }
>
> This should never happen, right? And if we want to be extra safe,
qsb_get_length() returns the amount of data in the qsb, not the
amount of allocated space; so it's legal for the qsb to have
allocated an iov entry but not actually put any data in it yet.
Will it have done that in our case? I don't think so, but no reason
to make assumptions.
> > + QEMUFile *packf = qemu_bufopen("r", qsb);
> > +
> > + ret = qemu_loadvm_state_main(packf, mis);
> > + trace_loadvm_handle_cmd_packaged_main(ret);
> > + qemu_fclose(packf);
> > + qsb_free(qsb);
>
> Migration code is re-entrant!!!!! Who would have guessed O:-)
To a very limited degree; there's global state shotgunned around everywhere
(e.g. in the RAM code).
Dave
> Later, Juan.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
