On Mon, Jul 22, 2024 at 05:21:48PM -0300, Fabiano Rosas wrote:
> Peter Xu <pet...@redhat.com> writes:
>
> > On Mon, Jul 22, 2024 at 02:59:05PM -0300, Fabiano Rosas wrote:
> >> Hi,
> >>
> >> In this v2 I took Peter's suggestion of keeping the channels' pointers
> >> and moving only the extra slot. The major changes are in patches 5 and
> >> 9. Patch 3 introduces the structure:
> >>
> >> typedef enum {
> >> MULTIFD_PAYLOAD_NONE,
> >> MULTIFD_PAYLOAD_RAM,
> >> } MultiFDPayloadType;
> >>
> >> struct MultiFDSendData {
> >> MultiFDPayloadType type;
> >> union {
> >> MultiFDPages_t ram;
> >> } u;
> >> };
> >>
> >> I added a NONE type so we can use it to tell when the channel has
> >> finished sending a packet, since we'll need to switch types between
> >> clients anyway. This avoids having to introduce a 'size', or 'free'
> >> variable.
> >
> > This at least looks better to me, thanks.
> >
> >>
> >> WHAT'S MISSING:
> >>
> >> - The support for calling multifd_send() concurrently. Maciej has this
> >> in his series so I didn't touch it.
> >>
> >> - A way of adding methods for the new payload type. Currently, the
> >> compression methods are somewhat coupled with ram migration, so I'm
> >> not sure how to proceed.
> >
> > What is this one? Why compression methods need new payload? Aren't they
> > ram-typed?
>
> The data we transport is MultiFDPages_t, yes, but the MultiFDMethods are
> either nocomp, or the compression-specific methods
> (e.g. zlib_send_prepare).
>
> How do we add methods for the upcoming new payload types? I don't expect
> us to continue using nocomp and then do "if (ram)... else if
> (device_state) ..." inside of them. I would expect us to rename
> s/nocomp/ram/ and add a new set of MultiFDMethods for the new data type
> (e.g. vfio_send_prepare, vmstate_send_prepare, etc).
>
> multifd_nocomp_ops -> multifd_ram_ops // rename
> multifd_zlib_ops // existing
> multifd_device_ops // new
>
> The challenge here is that the current framework is nocomp
> vs. compression. It needs to become ram + compression vs. other types.
IMHO we can keep multifd_ops[] only for RAM. There's only send_prepare()
that device state will need, and so far it's only (referring Maciej's
code):
static int nocomp_send_prepare_device_state(MultiFDSendParams *p,
Error **errp)
{
multifd_send_prepare_header_device_state(p);
assert(!(p->flags & MULTIFD_FLAG_SYNC));
p->next_packet_size = p->device_state->buf_len;
if (p->next_packet_size > 0) {
p->iov[p->iovs_num].iov_base = p->device_state->buf;
p->iov[p->iovs_num].iov_len = p->next_packet_size;
p->iovs_num++;
}
p->flags |= MULTIFD_FLAG_NOCOMP | MULTIFD_FLAG_DEVICE_STATE;
multifd_send_fill_packet_device_state(p);
return 0;
}
None of other multifd_ops are used.
I think we can directly invoke this part of device state code in
multifd_send_thread() for now. So far I think it should be ok.
--
Peter Xu
