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.liu@intel.com>
Reviewed-by: Nanhai Zou <nanhai.zou@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;
 }
 
 /**
@@ -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

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.liu@intel.com>
> Reviewed-by: Nanhai Zou <nanhai.zou@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

> -----Original Message-----
> From: Peter Xu <peterx@redhat.com>
> Sent: Tuesday, May 28, 2024 4:51 AM
> To: Liu, Yuan1 <yuan1.liu@intel.com>
> Cc: farosas@suse.de; qemu-devel@nongnu.org; Zou, Nanhai
> <nanhai.zou@intel.com>
> Subject: Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into
> compression method
> 
> 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.liu@intel.com>
> > Reviewed-by: Nanhai Zou <nanhai.zou@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?

Zstd/zlib does not request the IOVs on the receiving side.
The zstd/zlib reads the compressed data into the buffer directly

qio_channel_read_all(p->c, (void *)z->zbuff, in_size, errp);

> 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.

Yes, I agree with adding a check of IOV in multifd.c since different compressors 
may have different requirements for IOV.

We can add assert(p->iov) after send_setup, not recv_setup. The sending side always
uses IOV to send pages by qio_channel_writev_full_all in multifd.c, but the receiving
side may not require the IOV for reading pages.

It is better to add assert(!p->iov) after send|recv_cleanup()

> >
> >  /**
> > @@ -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

On Tue, May 28, 2024 at 01:36:38PM +0000, Liu, Yuan1 wrote:
> > -----Original Message-----
> > From: Peter Xu <peterx@redhat.com>
> > Sent: Tuesday, May 28, 2024 4:51 AM
> > To: Liu, Yuan1 <yuan1.liu@intel.com>
> > Cc: farosas@suse.de; qemu-devel@nongnu.org; Zou, Nanhai
> > <nanhai.zou@intel.com>
> > Subject: Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into
> > compression method
> > 
> > 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.liu@intel.com>
> > > Reviewed-by: Nanhai Zou <nanhai.zou@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?
> 
> Zstd/zlib does not request the IOVs on the receiving side.
> The zstd/zlib reads the compressed data into the buffer directly
> 
> qio_channel_read_all(p->c, (void *)z->zbuff, in_size, errp);

Hmm indeed, thanks.

> 
> > 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.
> 
> Yes, I agree with adding a check of IOV in multifd.c since different compressors 
> may have different requirements for IOV.
> 
> We can add assert(p->iov) after send_setup, not recv_setup. The sending side always
> uses IOV to send pages by qio_channel_writev_full_all in multifd.c, but the receiving
> side may not require the IOV for reading pages.
> 
> It is better to add assert(!p->iov) after send|recv_cleanup()

In that case maybe we should at some time move iov out of MultiFDRecvParams
as that's not a shared object, making MultiFDSendParams.compress_data
points to iov for nocomp case.  But we can leave that for later.

Then I've no question on this patch, feel free to take (with/without the
conditional assertions):

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

> 
> > >
> > >  /**
> > > @@ -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
> 

-- 
Peter Xu

Yuan Liu <yuan1.liu@intel.com> writes:

> 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.liu@intel.com>
> Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>