On Tue, Sep 03, 2019 at 07:43:24PM +0100, Dr. David Alan Gilbert wrote: >* Wei Yang ([email protected]) wrote: >> On Fri, Aug 23, 2019 at 12:06:09PM +0100, Dr. David Alan Gilbert wrote: >> >(Copying Dan in) >> > >> >* Wei Yang ([email protected]) 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 <[email protected]> >> > >> >Reviewed-by: Dr. David Alan Gilbert <[email protected]> >> > >> >(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? > >Yes.
Sure, Let me prepare v2 with this. -- Wei Yang Help you, Help me
