RE: [PATCH v5 2/7] migration/multifd: put IOV initialization into compression method

["Liu, Yuan1" <yuan1.liu@intel.com>]

> -----Original Message-----
> From: Fabiano Rosas <farosas@suse.de>
> Sent: Wednesday, March 20, 2024 11:19 PM
> To: Liu, Yuan1 <yuan1.liu@intel.com>; peterx@redhat.com
> Cc: qemu-devel@nongnu.org; hao.xiang@bytedance.com;
> bryan.zhang@bytedance.com; Liu, Yuan1 <yuan1.liu@intel.com>; Zou, Nanhai
> <nanhai.zou@intel.com>
> Subject: Re: [PATCH v5 2/7] migration/multifd: put IOV initialization into
> compression method
> 
> 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>
> > ---
> >  migration/multifd-zlib.c | 4 ++++
> >  migration/multifd-zstd.c | 6 +++++-
> >  migration/multifd.c      | 8 +++++---
> >  3 files changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> > index 99821cd4d5..8095ef8e28 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;
> > +
> > +    assert(p->iov == NULL);
> > +    /* For packet header and zlib streaming compression block */
> > +    p->iov = g_new0(struct iovec, 2);
> >      return 0;
> >
> >  err_free_zbuff:
> > diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
> > index 02112255ad..9c9217794e 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,11 @@ 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;
> > +
> > +    assert(p->iov == NULL);
> > +    /* For packet header and zstd streaming compression block */
> > +    p->iov = g_new0(struct iovec, 2);
> >      return 0;
> >  }
> >
> > diff --git a/migration/multifd.c b/migration/multifd.c
> > index 0179422f6d..5155e02ae3 100644
> > --- a/migration/multifd.c
> > +++ b/migration/multifd.c
> > @@ -1181,9 +1181,11 @@ 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);
> > +            /* IOVs are initialized in send_setup of compression method
> */
> > +            if (!migrate_multifd_compression()) {
> > +                /* We need one extra place for the packet header */
> > +                p->iov = g_new0(struct iovec, page_count + 1);
> > +            }
> 
> This^ should go into nocomp_send_setup:
> 
> static int nocomp_send_setup(MultiFDSendParams *p, Error **errp)
> {
>     if (migrate_zero_copy_send()) {
>         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;
> }

Yes, this is better, I will fix this in the next version,
thanks for your comments.

> >          } else {
> >              p->iov = g_new0(struct iovec, page_count);
> >          }