* Wei Yang (richard.weiy...@gmail.com) wrote:
> 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?
Yes.
Dave
> >> }
> >> 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
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
