From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34400) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJn3s-0006hB-Bj for qemu-devel@nongnu.org; Mon, 27 Jul 2015 14:23:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZJn3p-0000de-5v for qemu-devel@nongnu.org; Mon, 27 Jul 2015 14:23:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44922) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJn3o-0000dR-Uq for qemu-devel@nongnu.org; Mon, 27 Jul 2015 14:23:37 -0400 Date: Mon, 27 Jul 2015 18:28:39 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20150727172838.GB27613@work-vm> References: <1434450415-11339-1-git-send-email-dgilbert@redhat.com> <1434450415-11339-20-git-send-email-dgilbert@redhat.com> <20150721061129.GC10280@grmbl.mre> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150721061129.GC10280@grmbl.mre> Subject: Re: [Qemu-devel] [PATCH v7 19/42] MIG_CMD_PACKAGED: Send a packaged chunk of migration stream List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, quintela@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org, luis@cs.umu.se, pbonzini@redhat.com, david@gibson.dropbear.id.au * 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" > > > > 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