From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEeKe-0004mx-7V for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:31:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YEeKb-0002Q9-0f for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:31:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34724) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEeKa-0002Q3-P7 for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:31:24 -0500 Date: Fri, 23 Jan 2015 13:31:18 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20150123133117.GE2370@work-vm> References: <1418347746-15829-1-git-send-email-liang.z.li@intel.com> <1418347746-15829-5-git-send-email-liang.z.li@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1418347746-15829-5-git-send-email-liang.z.li@intel.com> Subject: Re: [Qemu-devel] [v3 04/13] qemu-file: Add tow function will be used in migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liang Li Cc: quintela@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, lcapitulino@redhat.com, yang.z.zhang@intel.com * Liang Li (liang.z.li@intel.com) wrote: > migrate_qemu_add_compression_data() compress the data > and put it to QEMUFile. migrate_qemu_flush() put the > data in the source QEMUFile to destination QEMUFile. > > The two function can help to do live migration. Typo in the title 'tow->two' - but perhaps a better title would be 'Add compression functions to QEMUFile' > Signed-off-by: Liang Li > Signed-off-by: Yang Zhang > --- > include/migration/qemu-file.h | 3 +++ > qemu-file.c | 32 ++++++++++++++++++++++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h > index 401676b..d70fb8d 100644 > --- a/include/migration/qemu-file.h > +++ b/include/migration/qemu-file.h > @@ -150,6 +150,9 @@ void qemu_put_be32(QEMUFile *f, unsigned int v); > void qemu_put_be64(QEMUFile *f, uint64_t v); > int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset); > int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size); > +size_t migrate_qemu_add_compression_data(QEMUFile *f, > + const uint8_t *p, size_t size, int level); > +int migrate_qemu_flush(QEMUFile *f_des, QEMUFile *f_src); > /* > * Note that you can only peek continuous bytes from where the current pointer > * is; you aren't guaranteed to be able to peak to +n bytes unless you've > diff --git a/qemu-file.c b/qemu-file.c > index f938e36..2668ad0 100644 > --- a/qemu-file.c > +++ b/qemu-file.c > @@ -21,6 +21,7 @@ > * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > * THE SOFTWARE. > */ > +#include > #include "qemu-common.h" > #include "qemu/iov.h" > #include "qemu/sockets.h" > @@ -993,3 +994,34 @@ QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input) > } > return s->file; > } > + > +/* compress size bytes of data start at p with specific compression > + * leve and store the compressed data to the buffer of f. > + */ > + > +size_t migrate_qemu_add_compression_data(QEMUFile *f, > + const uint8_t *p, size_t size, int level) It's an odd name, QEMUFile is only used by migration anyway; maybe qemufile_add_compression_data ? > +{ > + size_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int); > + > + if (compress2(f->buf + f->buf_index + sizeof(int), &blen, (Bytef *)p, > + size, level) != Z_OK) { > + error_report("Compress Failed!"); > + return 0; > + } What are the 'sizeof(int)'s for? It's unusual because we normally keep the format of the stream the same even if one side was a 32bit qemu and the other 64bit. How do you know that there is enough space for the compress - i.e. what happens if f->buf_index is too high? > + qemu_put_be32(f, blen); > + f->buf_index += blen; > + return blen + sizeof(int); > +} > + > +int migrate_qemu_flush(QEMUFile *f_des, QEMUFile *f_src) > +{ > + int len = 0; > + > + if (f_src->buf_index > 0) { > + len = f_src->buf_index; > + qemu_put_buffer(f_des, f_src->buf, f_src->buf_index); > + f_src->buf_index = 0; > + } > + return len; > +} Can you explain a bit more here how you're using the src file; I think you're using it as kind of a dummy buffer; but it needs to be documented somewhere. Again I'm also not sure of the name of this function. Dave > -- > 1.8.3.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK