Re: [PATCH 13/14] migration/multifd: Move header prepare/fill into send_prepare()

[Peter Xu]

On Wed, Jan 31, 2024 at 06:42:57PM -0300, Fabiano Rosas wrote:
> peterx@redhat.com writes:
> 
> > From: Peter Xu <peterx@redhat.com>
> >
> > This patch redefines the interfacing of ->send_prepare().  It further
> > simplifies multifd_send_thread() especially on zero copy.
> >
> > Now with the new interface, we require the hook to do all the work for
> > preparing the IOVs to send.  After it's completed, the IOVs should be ready
> > to be dumped into the specific multifd QIOChannel later.
> >
> > So now the API looks like:
> >
> >   p->pages ----------->  send_prepare() -------------> IOVs
> >
> > This also prepares for the case where the input can be extended to even not
> > any p->pages.  But that's for later.
> >
> > This patch will achieve similar goal of what Fabiano used to propose here:
> >
> > https://lore.kernel.org/r/20240126221943.26628-1-farosas@suse.de
> >
> > However the send() interface may not be necessary.  I'm boldly attaching a
> 
> So should I drop send() for fixed-ram as well? Or do you still want a
> separate layer just for send()?

Currently after the whole set applied, the IO side is pretty like before,
and IMHO straightforward enough:

            ret = qio_channel_writev_full_all(p->c, p->iov, p->iovs_num, NULL,
                                              0, p->write_flags, &local_err);
            if (ret != 0) {
                qemu_mutex_unlock(&p->mutex);
                break;
            }

IIRC with your file interface added, it could logically become something
like:

            if (use_socket()) {
                ret = qio_channel_writev_full_all(IOV, ...);
            } else {
                /*
                 * Assert "file:".  I forgot what we used to discuss here for
                 * the interface as name.. but I remember we need ramblock,
                 * so perhaps passing over "p" would work?  As then it is
                 * p->pages->ramblock, along with IOVs, etc.
                 */
                ret = qio_channel_XXX(p, ...);
            }

            if (ret != 0) {
                qemu_mutex_unlock(&p->mutex);
                break;
            }

So there's only one way or another.  We can add one helper to even wrap
these two.

IMHO a hook will be more helpful if there can be a bunch of "if, else if,
... else" things, so at least three options perhaps?  But if you prefer a
hook, that'll also work for me.  So.. your call. :)

But I hope if the send() will exist, it's a separate OPs, so that the
compiler accelerators should avoid worrying at all with how the data will
be dumped when they prepare their new MultiFDMethods (even though your
"file:" will need to block them all as of now, but only support no
compression, iiuc).

-- 
Peter Xu