* [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file
@ 2019-09-11 13:28 Wei Yang
2019-09-11 13:28 ` [Qemu-devel] [PATCH V2 1/2] migration/qemu-file: remove check on writev_buffer in qemu_put_compression_data Wei Yang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Wei Yang @ 2019-09-11 13:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
Two cleanup:
Patch #1 make code consistent on calling add_to_iovec
Patch #2 refine the code to handle the case when buf already flushed
v2:
* wrap common steps into add_buf_to_iovec()
Wei Yang (2):
migration/qemu-file: remove check on writev_buffer in
qemu_put_compression_data
migration/qemu-file: fix potential buf waste for extra buf_index
adjustment
migration/qemu-file.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH V2 1/2] migration/qemu-file: remove check on writev_buffer in qemu_put_compression_data
2019-09-11 13:28 [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file Wei Yang
@ 2019-09-11 13:28 ` Wei Yang
2019-09-11 13:28 ` [Qemu-devel] [PATCH V2 2/2] migration/qemu-file: fix potential buf waste for extra buf_index adjustment Wei Yang
2019-09-12 10:23 ` [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file Dr. David Alan Gilbert
2 siblings, 0 replies; 5+ messages in thread
From: Wei Yang @ 2019-09-11 13:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
From: Wei Yang <richardw.yang@linux.intel.com>
The check of writev_buffer is in qemu_fflush, which means it is not
harmful if it is NULL.
And removing it will make the code consistent since all other
add_to_iovec() is called without the check.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
migration/qemu-file.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index e33c46764f..47f16d0e54 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -761,9 +761,7 @@ ssize_t qemu_put_compression_data(QEMUFile *f, z_stream *stream,
}
qemu_put_be32(f, blen);
- if (f->ops->writev_buffer) {
- add_to_iovec(f, f->buf + f->buf_index, blen, false);
- }
+ 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);
--
2.15.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH V2 2/2] migration/qemu-file: fix potential buf waste for extra buf_index adjustment
2019-09-11 13:28 [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file Wei Yang
2019-09-11 13:28 ` [Qemu-devel] [PATCH V2 1/2] migration/qemu-file: remove check on writev_buffer in qemu_put_compression_data Wei Yang
@ 2019-09-11 13:28 ` Wei Yang
2019-09-11 19:01 ` Dr. David Alan Gilbert
2019-09-12 10:23 ` [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file Dr. David Alan Gilbert
2 siblings, 1 reply; 5+ messages in thread
From: Wei Yang @ 2019-09-11 13:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
From: Wei Yang <richardw.yang@linux.intel.com>
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.
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.yang@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
v2:
* wrap these common steps into add_buf_to_iovec()
---
migration/qemu-file.c | 43 ++++++++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 47f16d0e54..417eeba64b 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -382,8 +382,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 +
@@ -401,6 +409,19 @@ 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;
+}
+
+static void add_buf_to_iovec(QEMUFile *f, size_t len)
+{
+ if (!add_to_iovec(f, f->buf + f->buf_index, len, false)) {
+ f->buf_index += len;
+ if (f->buf_index == IO_BUF_SIZE) {
+ qemu_fflush(f);
+ }
}
}
@@ -430,11 +451,7 @@ 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);
- }
+ add_buf_to_iovec(f, l);
if (qemu_file_get_error(f)) {
break;
}
@@ -451,11 +468,7 @@ 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);
- }
+ add_buf_to_iovec(f, 1);
}
void qemu_file_skip(QEMUFile *f, int size)
@@ -761,11 +774,7 @@ 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);
- }
+ add_buf_to_iovec(f, blen);
return blen + sizeof(int32_t);
}
--
2.15.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH V2 2/2] migration/qemu-file: fix potential buf waste for extra buf_index adjustment
2019-09-11 13:28 ` [Qemu-devel] [PATCH V2 2/2] migration/qemu-file: fix potential buf waste for extra buf_index adjustment Wei Yang
@ 2019-09-11 19:01 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 5+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-11 19:01 UTC (permalink / raw)
To: Wei Yang; +Cc: Wei Yang, qemu-devel, quintela
* Wei Yang (richard.weiyang@gmail.com) wrote:
> From: Wei Yang <richardw.yang@linux.intel.com>
>
> 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.
>
> 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.yang@linux.intel.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> ---
> v2:
> * wrap these common steps into add_buf_to_iovec()
> ---
> migration/qemu-file.c | 43 ++++++++++++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 47f16d0e54..417eeba64b 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -382,8 +382,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 +
> @@ -401,6 +409,19 @@ 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;
> +}
> +
> +static void add_buf_to_iovec(QEMUFile *f, size_t len)
> +{
> + if (!add_to_iovec(f, f->buf + f->buf_index, len, false)) {
> + f->buf_index += len;
> + if (f->buf_index == IO_BUF_SIZE) {
> + qemu_fflush(f);
> + }
Yep, that's better.
Dave
> }
> }
>
> @@ -430,11 +451,7 @@ 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);
> - }
> + add_buf_to_iovec(f, l);
> if (qemu_file_get_error(f)) {
> break;
> }
> @@ -451,11 +468,7 @@ 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);
> - }
> + add_buf_to_iovec(f, 1);
> }
>
> void qemu_file_skip(QEMUFile *f, int size)
> @@ -761,11 +774,7 @@ 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);
> - }
> + add_buf_to_iovec(f, blen);
> return blen + sizeof(int32_t);
> }
>
> --
> 2.15.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file
2019-09-11 13:28 [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file Wei Yang
2019-09-11 13:28 ` [Qemu-devel] [PATCH V2 1/2] migration/qemu-file: remove check on writev_buffer in qemu_put_compression_data Wei Yang
2019-09-11 13:28 ` [Qemu-devel] [PATCH V2 2/2] migration/qemu-file: fix potential buf waste for extra buf_index adjustment Wei Yang
@ 2019-09-12 10:23 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 5+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-12 10:23 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, quintela
* Wei Yang (richard.weiyang@gmail.com) wrote:
> Two cleanup:
>
> Patch #1 make code consistent on calling add_to_iovec
> Patch #2 refine the code to handle the case when buf already flushed
Queued
> v2:
> * wrap common steps into add_buf_to_iovec()
>
> Wei Yang (2):
> migration/qemu-file: remove check on writev_buffer in
> qemu_put_compression_data
> migration/qemu-file: fix potential buf waste for extra buf_index
> adjustment
>
> migration/qemu-file.c | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> --
> 2.15.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-12 10:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11 13:28 [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file Wei Yang
2019-09-11 13:28 ` [Qemu-devel] [PATCH V2 1/2] migration/qemu-file: remove check on writev_buffer in qemu_put_compression_data Wei Yang
2019-09-11 13:28 ` [Qemu-devel] [PATCH V2 2/2] migration/qemu-file: fix potential buf waste for extra buf_index adjustment Wei Yang
2019-09-11 19:01 ` Dr. David Alan Gilbert
2019-09-12 10:23 ` [Qemu-devel] [PATCH V2 0/2] migration/qemu-file: cleanup and refine qemu-file Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.