On Tue, Nov 16, 2021 at 05:08:06PM +0100, Juan Quintela wrote:
> Leonardo Bras <leob...@redhat.com> wrote:
> > Implement zerocopy on nocomp_send_write(), by making use of QIOChannel
> > zerocopy interface.
> >
> > Change multifd_send_sync_main() so it can distinguish each iteration sync
> > from
> > the setup and the completion, so a flush_zerocopy() can be called
> > at the after each iteration in order to make sure all dirty pages are sent
> > before a new iteration is started.
> >
> > Also make it return -1 if flush_zerocopy() fails, in order to cancel
> > the migration process, and avoid resuming the guest in the target host
> > without receiving all current RAM.
> >
> > This will work fine on RAM migration because the RAM pages are not usually
> > freed,
> > and there is no problem on changing the pages content between async_send()
> > and
> > the actual sending of the buffer, because this change will dirty the page
> > and
> > cause it to be re-sent on a next iteration anyway.
> >
> > Given a lot of locked memory may be needed in order to use multid migration
> > with zerocopy enabled, make it optional by creating a new migration
> > parameter
> > "zerocopy" on qapi, so low-privileged users can still perform multifd
> > migrations.
>
> How much memory can a non-root program use by default?
>
>
> > static void *multifd_send_thread(void *opaque)
> > @@ -853,6 +875,10 @@ static void multifd_new_send_channel_async(QIOTask
> > *task, gpointer opaque)
> > goto cleanup;
> > }
> >
> > + if (migrate_use_zerocopy()) {
> > + p->write_flags = QIO_CHANNEL_WRITE_FLAG_ZEROCOPY;
> > + }
>
> This belongs
>
>
> > p->c = QIO_CHANNEL(sioc);
> > qio_channel_set_delay(p->c, false);
> > p->running = true;
> > @@ -918,6 +944,7 @@ int multifd_save_setup(Error **errp)
> > p->packet->version = cpu_to_be32(MULTIFD_VERSION);
> > p->name = g_strdup_printf("multifdsend_%d", i);
> > p->tls_hostname = g_strdup(s->hostname);
> > + p->write_flags = 0;
>
> here?
>
> > socket_send_channel_create(multifd_new_send_channel_async, p);
> > }
> > diff --git a/migration/socket.c b/migration/socket.c
> > index e26e94aa0c..8e40e0a3fd 100644
> > --- a/migration/socket.c
> > +++ b/migration/socket.c
> > @@ -78,8 +78,13 @@ static void socket_outgoing_migration(QIOTask *task,
> > trace_migration_socket_outgoing_connected(data->hostname);
> > }
> >
> > - if (migrate_use_zerocopy()) {
> > - error_setg(&err, "Zerocopy not available in migration");
> > + if (migrate_use_zerocopy() &&
> > + (!migrate_use_multifd() ||
> > + !qio_channel_has_feature(sioc,
> > QIO_CHANNEL_FEATURE_WRITE_ZEROCOPY) ||
> > + migrate_multifd_compression() != MULTIFD_COMPRESSION_NONE ||
> > + migrate_use_tls())) {
> > + error_setg(&err,
> > + "Zerocopy only available for non-compressed non-TLS
> > multifd migration");
> > }
> >
> > migration_channel_connect(data->s, sioc, data->hostname, err);
>
> Do we really want to do this check here? I think this is really too
> late.
>
> You are not patching migrate_params_check().
>
> I think that the proper way of doing this is something like:
>
> if (params->zerocopy &&
> (params->parameters.multifd_compression != MULTIFD_COMPRESSION_NONE ||
> migrate_use_tls())) {
> error_setg(&err,
> "Zerocopy only available for non-compressed non-TLS
> multifd migration");
> return false;
> }
>
> You have to do the equivalent of multifd_compression and tls enablement,
> to see that zerocopy is not enabled, of course.
>
> I would prefer to check for QIO_CHANNEL_FEATUR_WRITE_ZEROCPY there, but
> I can't see a way of doing that without a qio.
I don't think you need to check that feature flag.
The combination of zerocopy and compression is simply illogical
and can be rejected unconditionally.
The combination of zerocopy and TLS is somewhat questionable.
You're always going to have a copy between the plain text and
cipher text. Avoiding an extra copy for the cipher text will
require kerenl work which has no ETA. If it does ever arise,
QEMU will need more work again to actually support it. So
today we can just reject it unconditonally again.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
