On Fri, Aug 23, 2019 at 12:06:09PM +0100, Dr. David Alan Gilbert wrote: >(Copying Dan in) > >* Wei Yang (richardw.y...@linux.intel.com) wrote: >> In add_to_iovec(), qemu_fflush() will be called if iovec is full. If >> this happens, buf_index is reset. Currently, this is not checked and >> buf_index would always been adjust with buf size. >> >> This is not harmful, but will waste some space in file buffer. > >That's a nice find. > >> This patch make add_to_iovec() return 1 when it has flushed the file. >> Then the caller could check the return value to see whether it is >> necessary to adjust the buf_index any more. >> >> Signed-off-by: Wei Yang <richardw.y...@linux.intel.com> > >Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > >(I wonder if there's a way to wrap that little add_to_iovec, check, add >to index, flush in a little wrapper). > >Dave > >> --- >> migration/qemu-file.c | 42 ++++++++++++++++++++++++++++-------------- >> 1 file changed, 28 insertions(+), 14 deletions(-) >> >> diff --git a/migration/qemu-file.c b/migration/qemu-file.c >> index 35c22605dd..05d9f42ddb 100644 >> --- a/migration/qemu-file.c >> +++ b/migration/qemu-file.c >> @@ -343,8 +343,16 @@ int qemu_fclose(QEMUFile *f) >> return ret; >> } >> >> -static void add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size, >> - bool may_free) >> +/* >> + * Add buf to iovec. Do flush if iovec is full. >> + * >> + * Return values: >> + * 1 iovec is full and flushed >> + * 0 iovec is not flushed >> + * >> + */ >> +static int add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size, >> + bool may_free) >> { >> /* check for adjacent buffer and coalesce them */ >> if (f->iovcnt > 0 && buf == f->iov[f->iovcnt - 1].iov_base + >> @@ -362,7 +370,10 @@ static void add_to_iovec(QEMUFile *f, const uint8_t >> *buf, size_t size, >> >> if (f->iovcnt >= MAX_IOV_SIZE) { >> qemu_fflush(f); >> + return 1; >> } >> + >> + return 0; >> } >> >> void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size, >> @@ -391,10 +402,11 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, >> size_t size) >> } >> memcpy(f->buf + f->buf_index, buf, l); >> f->bytes_xfer += l; >> - add_to_iovec(f, f->buf + f->buf_index, l, false); >> - f->buf_index += l; >> - if (f->buf_index == IO_BUF_SIZE) { >> - qemu_fflush(f); >> + if (!add_to_iovec(f, f->buf + f->buf_index, l, false)) { >> + f->buf_index += l; >> + if (f->buf_index == IO_BUF_SIZE) { >> + qemu_fflush(f); >> + }
You mean put these four lines into a wrapper? Name it as add_buf_to_iovec? >> } >> if (qemu_file_get_error(f)) { >> break; >> @@ -412,10 +424,11 @@ void qemu_put_byte(QEMUFile *f, int v) >> >> f->buf[f->buf_index] = v; >> f->bytes_xfer++; >> - add_to_iovec(f, f->buf + f->buf_index, 1, false); >> - f->buf_index++; >> - if (f->buf_index == IO_BUF_SIZE) { >> - qemu_fflush(f); >> + if (!add_to_iovec(f, f->buf + f->buf_index, 1, false)) { >> + f->buf_index++; >> + if (f->buf_index == IO_BUF_SIZE) { >> + qemu_fflush(f); >> + } >> } >> } >> >> @@ -717,10 +730,11 @@ ssize_t qemu_put_compression_data(QEMUFile *f, >> z_stream *stream, >> } >> >> qemu_put_be32(f, blen); >> - add_to_iovec(f, f->buf + f->buf_index, blen, false); >> - f->buf_index += blen; >> - if (f->buf_index == IO_BUF_SIZE) { >> - qemu_fflush(f); >> + if (!add_to_iovec(f, f->buf + f->buf_index, blen, false)) { >> + f->buf_index += blen; >> + if (f->buf_index == IO_BUF_SIZE) { >> + qemu_fflush(f); >> + } >> } >> return blen + sizeof(int32_t); >> } >> -- >> 2.17.1 >> >-- >Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- Wei Yang Help you, Help me