Zhimin Feng <fengzhim...@huawei.com> wrote:
> Creation of the multifd send threads for RDMA migration,
> nothing inside yet.
>
> Signed-off-by: Zhimin Feng <fengzhim...@huawei.com>
> ---
> migration/multifd.c | 33 +++++++++++++---
> migration/multifd.h | 2 +
> migration/qemu-file.c | 5 +++
> migration/qemu-file.h | 1 +
> migration/rdma.c | 88 ++++++++++++++++++++++++++++++++++++++++++-
> migration/rdma.h | 3 ++
> 6 files changed, 125 insertions(+), 7 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index b3e8ae9bcc..63678d7fdd 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -424,7 +424,7 @@ void multifd_send_sync_main(QEMUFile *f)
> {
> int i;
>
> - if (!migrate_use_multifd()) {
> + if (!migrate_use_multifd() || migrate_use_rdma()) {
You don't need sync with main channel on rdma?
> +static void rdma_send_channel_create(MultiFDSendParams *p)
> +{
> + Error *local_err = NULL;
> +
> + if (p->quit) {
> + error_setg(&local_err, "multifd: send id %d already quit", p->id);
> + return ;
> + }
> + p->running = true;
> +
> + qemu_thread_create(&p->thread, p->name, multifd_rdma_send_thread, p,
> + QEMU_THREAD_JOINABLE);
> +}
> +
> static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
> {
> MultiFDSendParams *p = opaque;
> @@ -621,7 +635,11 @@ int multifd_save_setup(Error **errp)
> p->packet->magic = cpu_to_be32(MULTIFD_MAGIC);
> p->packet->version = cpu_to_be32(MULTIFD_VERSION);
> p->name = g_strdup_printf("multifdsend_%d", i);
> - socket_send_channel_create(multifd_new_send_channel_async, p);
> + if (!migrate_use_rdma()) {
> + socket_send_channel_create(multifd_new_send_channel_async, p);
> + } else {
> + rdma_send_channel_create(p);
> + }
This is what we are trying to avoid. Just create a struct ops, where we
have a
ops->create_channel(new_channel_async, p)
or whatever, and fill it differently for rdma and for tcp.
> }
> return 0;
> }
> @@ -720,7 +738,7 @@ void multifd_recv_sync_main(void)
> {
> int i;
>
> - if (!migrate_use_multifd()) {
> + if (!migrate_use_multifd() || migrate_use_rdma()) {
> return;
> }
Ok. you can just put an empty function for you here.
> for (i = 0; i < migrate_multifd_channels(); i++) {
> @@ -890,8 +908,13 @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error
> **errp)
> p->num_packets = 1;
>
> p->running = true;
> - qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
> - QEMU_THREAD_JOINABLE);
> + if (!migrate_use_rdma()) {
> + qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
> + QEMU_THREAD_JOINABLE);
> + } else {
> + qemu_thread_create(&p->thread, p->name, multifd_rdma_recv_thread, p,
> + QEMU_THREAD_JOINABLE);
> + }
new_recv_chanel() member function.
> atomic_inc(&multifd_recv_state->count);
> return atomic_read(&multifd_recv_state->count) ==
> migrate_multifd_channels();
> diff --git a/migration/multifd.h b/migration/multifd.h
> index d8b0205977..c9c11ad140 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -13,6 +13,8 @@
> #ifndef QEMU_MIGRATION_MULTIFD_H
> #define QEMU_MIGRATION_MULTIFD_H
>
> +#include "migration/rdma.h"
> +
> int multifd_save_setup(Error **errp);
> void multifd_save_cleanup(void);
> int multifd_load_setup(Error **errp);
You are not exporting anything rdma related from here, are you?
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 1c3a358a14..f0ed8f1381 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -248,6 +248,11 @@ void qemu_fflush(QEMUFile *f)
> f->iovcnt = 0;
> }
>
> +void *getQIOChannel(QEMUFile *f)
> +{
> + return f->opaque;
> +}
> +
We really want this to return a void? and not a better type?
> +static void migration_rdma_process_incoming(QEMUFile *f, Error **errp)
> +{
> + MigrationIncomingState *mis = migration_incoming_get_current();
> + Error *local_err = NULL;
> + QIOChannel *ioc = NULL;
> + bool start_migration;
> +
> + if (!mis->from_src_file) {
> + mis->from_src_file = f;
> + qemu_file_set_blocking(f, false);
> +
> + start_migration = migrate_use_multifd();
> + } else {
> + ioc = QIO_CHANNEL(getQIOChannel(f));
> + /* Multiple connections */
> + assert(migrate_use_multifd());
I am not sure that you can make this incompatible change.
You need to have *both*, old method and new multifd one.
I would have been happy to remove old precopy tcp method, but we
*assure* backwards compatibility.
> @@ -4003,8 +4032,12 @@ static void rdma_accept_incoming_migration(void
> *opaque)
> return;
> }
>
> - rdma->migration_started_on_destination = 1;
> - migration_fd_process_incoming(f, errp);
> + if (migrate_use_multifd()) {
> + migration_rdma_process_incoming(f, errp);
> + } else {
> + rdma->migration_started_on_destination = 1;
> + migration_fd_process_incoming(f, errp);
> + }
But here you allow that multifd is not defined?
> +
> +void *multifd_rdma_recv_thread(void *opaque)
> +{
Why can't you use the multifd_recv_thread() directly, just creating
different ops when you need them?
Later, Juan.
