On Mon, May 06, 2024 at 12:57:46AM +0800, Yuan Liu wrote:
> Different compression methods may require different numbers of IOVs.
> Based on streaming compression of zlib and zstd, all pages will be
> compressed to a data block, so two IOVs are needed for packet header
> and compressed data block.
>
> Signed-off-by: Yuan Liu <yuan1....@intel.com>
> Reviewed-by: Nanhai Zou <nanhai....@intel.com>
> ---
> migration/multifd-zlib.c | 7 +++++++
> migration/multifd-zstd.c | 8 +++++++-
> migration/multifd.c | 22 ++++++++++++----------
> 3 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> index 737a9645d2..2ced69487e 100644
> --- a/migration/multifd-zlib.c
> +++ b/migration/multifd-zlib.c
> @@ -70,6 +70,10 @@ static int zlib_send_setup(MultiFDSendParams *p, Error
> **errp)
> goto err_free_zbuff;
> }
> p->compress_data = z;
> +
> + /* Needs 2 IOVs, one for packet header and one for compressed data */
> + p->iov = g_new0(struct iovec, 2);
> +
> return 0;
>
> err_free_zbuff:
> @@ -101,6 +105,9 @@ static void zlib_send_cleanup(MultiFDSendParams *p, Error
> **errp)
> z->buf = NULL;
> g_free(p->compress_data);
> p->compress_data = NULL;
> +
> + g_free(p->iov);
> + p->iov = NULL;
> }
>
> /**
> diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
> index 256858df0a..ca17b7e310 100644
> --- a/migration/multifd-zstd.c
> +++ b/migration/multifd-zstd.c
> @@ -52,7 +52,6 @@ static int zstd_send_setup(MultiFDSendParams *p, Error
> **errp)
> struct zstd_data *z = g_new0(struct zstd_data, 1);
> int res;
>
> - p->compress_data = z;
> z->zcs = ZSTD_createCStream();
> if (!z->zcs) {
> g_free(z);
> @@ -77,6 +76,10 @@ static int zstd_send_setup(MultiFDSendParams *p, Error
> **errp)
> error_setg(errp, "multifd %u: out of memory for zbuff", p->id);
> return -1;
> }
> + p->compress_data = z;
> +
> + /* Needs 2 IOVs, one for packet header and one for compressed data */
> + p->iov = g_new0(struct iovec, 2);
> return 0;
> }
>
> @@ -98,6 +101,9 @@ static void zstd_send_cleanup(MultiFDSendParams *p, Error
> **errp)
> z->zbuff = NULL;
> g_free(p->compress_data);
> p->compress_data = NULL;
> +
> + g_free(p->iov);
> + p->iov = NULL;
> }
>
> /**
> diff --git a/migration/multifd.c b/migration/multifd.c
> index f317bff077..d82885fdbb 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -137,6 +137,13 @@ static int nocomp_send_setup(MultiFDSendParams *p, Error
> **errp)
> p->write_flags |= QIO_CHANNEL_WRITE_FLAG_ZERO_COPY;
> }
>
> + if (multifd_use_packets()) {
> + /* We need one extra place for the packet header */
> + p->iov = g_new0(struct iovec, p->page_count + 1);
> + } else {
> + p->iov = g_new0(struct iovec, p->page_count);
> + }
> +
> return 0;
> }
>
> @@ -150,6 +157,8 @@ static int nocomp_send_setup(MultiFDSendParams *p, Error
> **errp)
> */
> static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
> {
> + g_free(p->iov);
> + p->iov = NULL;
> return;
> }
>
> @@ -228,6 +237,7 @@ static int nocomp_send_prepare(MultiFDSendParams *p,
> Error **errp)
> */
> static int nocomp_recv_setup(MultiFDRecvParams *p, Error **errp)
> {
> + p->iov = g_new0(struct iovec, p->page_count);
> return 0;
> }
>
> @@ -240,6 +250,8 @@ static int nocomp_recv_setup(MultiFDRecvParams *p, Error
> **errp)
> */
> static void nocomp_recv_cleanup(MultiFDRecvParams *p)
> {
> + g_free(p->iov);
> + p->iov = NULL;
> }
Are recv_setup()/recv_cleanup() for zstd/zlib missing?
If the iov will be managed by the compressors, I'm wondering whether we
should start assert(p->iov) after send|recv_setup(), and assert(!p->iov)
after send|recv_cleanup(). That'll guard all these.
>
> /**
> @@ -783,8 +795,6 @@ static bool
> multifd_send_cleanup_channel(MultiFDSendParams *p, Error **errp)
> p->packet_len = 0;
> g_free(p->packet);
> p->packet = NULL;
> - g_free(p->iov);
> - p->iov = NULL;
> multifd_send_state->ops->send_cleanup(p, errp);
>
> return *errp == NULL;
> @@ -1179,11 +1189,6 @@ bool multifd_send_setup(void)
> p->packet = g_malloc0(p->packet_len);
> p->packet->magic = cpu_to_be32(MULTIFD_MAGIC);
> p->packet->version = cpu_to_be32(MULTIFD_VERSION);
> -
> - /* We need one extra place for the packet header */
> - p->iov = g_new0(struct iovec, page_count + 1);
> - } else {
> - p->iov = g_new0(struct iovec, page_count);
> }
> p->name = g_strdup_printf("multifdsend_%d", i);
> p->page_size = qemu_target_page_size();
> @@ -1353,8 +1358,6 @@ static void
> multifd_recv_cleanup_channel(MultiFDRecvParams *p)
> p->packet_len = 0;
> g_free(p->packet);
> p->packet = NULL;
> - g_free(p->iov);
> - p->iov = NULL;
> g_free(p->normal);
> p->normal = NULL;
> g_free(p->zero);
> @@ -1602,7 +1605,6 @@ int multifd_recv_setup(Error **errp)
> p->packet = g_malloc0(p->packet_len);
> }
> p->name = g_strdup_printf("multifdrecv_%d", i);
> - p->iov = g_new0(struct iovec, page_count);
> p->normal = g_new0(ram_addr_t, page_count);
> p->zero = g_new0(ram_addr_t, page_count);
> p->page_count = page_count;
> --
> 2.39.3
>
--
Peter Xu
