* [PATCH v3 01/13] migration: Create migrate_rdma()
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 7:58 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 02/13] migration/rdma: Unfold ram_control_before_iterate() Juan Quintela
` (11 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Helper to say if we are doing a migration over rdma.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration.h | 2 ++
migration/options.h | 1 +
migration/migration.c | 1 +
migration/options.c | 7 +++++++
migration/rdma.c | 4 +++-
5 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/migration/migration.h b/migration/migration.h
index cd5534337c..96260138d1 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -469,6 +469,8 @@ struct MigrationState {
* switchover has been received.
*/
bool switchover_acked;
+ /* Is this a rdma migration */
+ bool rdma_migration;
};
void migrate_set_state(int *state, int old_state, int new_state);
diff --git a/migration/options.h b/migration/options.h
index 045e2a41a2..a26fd1680b 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -56,6 +56,7 @@ bool migrate_zero_copy_send(void);
bool migrate_multifd_flush_after_each_section(void);
bool migrate_postcopy(void);
+bool migrate_rdma(void);
bool migrate_tls(void);
/* capabilities helpers */
diff --git a/migration/migration.c b/migration/migration.c
index 1c6c81ad49..4213c645c6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1451,6 +1451,7 @@ int migrate_init(MigrationState *s, Error **errp)
s->iteration_initial_bytes = 0;
s->threshold_size = 0;
s->switchover_acked = false;
+ s->rdma_migration = false;
/*
* set mig_stats compression_counters memory to zero for a
* new migration
diff --git a/migration/options.c b/migration/options.c
index 6bbfd4853d..da379e7f7a 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -376,6 +376,13 @@ bool migrate_postcopy(void)
return migrate_postcopy_ram() || migrate_dirty_bitmaps();
}
+bool migrate_rdma(void)
+{
+ MigrationState *s = migrate_get_current();
+
+ return s->rdma_migration;
+}
+
bool migrate_tls(void)
{
MigrationState *s = migrate_get_current();
diff --git a/migration/rdma.c b/migration/rdma.c
index f6fc226c9b..f155f3e1c8 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4113,6 +4113,7 @@ static void rdma_accept_incoming_migration(void *opaque)
void rdma_start_incoming_migration(const char *host_port, Error **errp)
{
+ MigrationState *s = migrate_get_current();
int ret;
RDMAContext *rdma;
@@ -4144,7 +4145,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
}
trace_rdma_start_incoming_migration_after_rdma_listen();
-
+ s->rdma_migration = true;
qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration,
NULL, (void *)(intptr_t)rdma);
return;
@@ -4220,6 +4221,7 @@ void rdma_start_outgoing_migration(void *opaque,
trace_rdma_start_outgoing_migration_after_rdma_connect();
s->to_dst_file = rdma_new_output(rdma);
+ s->rdma_migration = true;
migrate_fd_connect(s, NULL);
return;
return_path_err:
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 01/13] migration: Create migrate_rdma()
2023-10-11 20:35 ` [PATCH v3 01/13] migration: Create migrate_rdma() Juan Quintela
@ 2023-10-13 7:58 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 7:58 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Helper to say if we are doing a migration over rdma.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> migration/migration.h | 2 ++
> migration/options.h | 1 +
> migration/migration.c | 1 +
> migration/options.c | 7 +++++++
> migration/rdma.c | 4 +++-
> 5 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/migration/migration.h b/migration/migration.h
> index cd5534337c..96260138d1 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -469,6 +469,8 @@ struct MigrationState {
> * switchover has been received.
> */
> bool switchover_acked;
> + /* Is this a rdma migration */
> + bool rdma_migration;
> };
>
> void migrate_set_state(int *state, int old_state, int new_state);
> diff --git a/migration/options.h b/migration/options.h
> index 045e2a41a2..a26fd1680b 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -56,6 +56,7 @@ bool migrate_zero_copy_send(void);
>
> bool migrate_multifd_flush_after_each_section(void);
> bool migrate_postcopy(void);
> +bool migrate_rdma(void);
> bool migrate_tls(void);
>
> /* capabilities helpers */
> diff --git a/migration/migration.c b/migration/migration.c
> index 1c6c81ad49..4213c645c6 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1451,6 +1451,7 @@ int migrate_init(MigrationState *s, Error **errp)
> s->iteration_initial_bytes = 0;
> s->threshold_size = 0;
> s->switchover_acked = false;
> + s->rdma_migration = false;
> /*
> * set mig_stats compression_counters memory to zero for a
> * new migration
> diff --git a/migration/options.c b/migration/options.c
> index 6bbfd4853d..da379e7f7a 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -376,6 +376,13 @@ bool migrate_postcopy(void)
> return migrate_postcopy_ram() || migrate_dirty_bitmaps();
> }
>
> +bool migrate_rdma(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + return s->rdma_migration;
> +}
> +
> bool migrate_tls(void)
> {
> MigrationState *s = migrate_get_current();
> diff --git a/migration/rdma.c b/migration/rdma.c
> index f6fc226c9b..f155f3e1c8 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -4113,6 +4113,7 @@ static void rdma_accept_incoming_migration(void *opaque)
>
> void rdma_start_incoming_migration(const char *host_port, Error **errp)
> {
> + MigrationState *s = migrate_get_current();
> int ret;
> RDMAContext *rdma;
>
> @@ -4144,7 +4145,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
> }
>
> trace_rdma_start_incoming_migration_after_rdma_listen();
> -
> + s->rdma_migration = true;
> qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration,
> NULL, (void *)(intptr_t)rdma);
> return;
> @@ -4220,6 +4221,7 @@ void rdma_start_outgoing_migration(void *opaque,
> trace_rdma_start_outgoing_migration_after_rdma_connect();
>
> s->to_dst_file = rdma_new_output(rdma);
> + s->rdma_migration = true;
> migrate_fd_connect(s, NULL);
> return;
> return_path_err:
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 02/13] migration/rdma: Unfold ram_control_before_iterate()
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
2023-10-11 20:35 ` [PATCH v3 01/13] migration: Create migrate_rdma() Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-12 14:21 ` Fabiano Rosas
2023-10-13 8:24 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 03/13] migration/rdma: Unfold ram_control_after_iterate() Juan Quintela
` (10 subsequent siblings)
12 siblings, 2 replies; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Once there:
- Remove unused data parameter
- unfold it in its callers.
- change all callers to call qemu_rdma_registration_start()
- We need to call QIO_CHANNEL_RDMA() after we check for migrate_rdma()
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
--
initilazize rioc after checknig that rdma is enabled.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.h | 2 --
migration/rdma.h | 7 +++++++
migration/qemu-file.c | 13 +------------
migration/ram.c | 16 +++++++++++++---
migration/rdma.c | 12 ++++--------
5 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 03e718c264..d6a370c569 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -55,7 +55,6 @@ typedef int (QEMURamSaveFunc)(QEMUFile *f,
size_t size);
typedef struct QEMUFileHooks {
- QEMURamHookFunc *before_ram_iterate;
QEMURamHookFunc *after_ram_iterate;
QEMURamHookFunc *hook_ram_load;
QEMURamSaveFunc *save_page;
@@ -127,7 +126,6 @@ void qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
-void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
diff --git a/migration/rdma.h b/migration/rdma.h
index de2ba09dc5..670c67a8cb 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -22,4 +22,11 @@ void rdma_start_outgoing_migration(void *opaque, const char *host_port,
void rdma_start_incoming_migration(const char *host_port, Error **errp);
+
+#ifdef CONFIG_RDMA
+int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
+#else
+static inline
+int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
+#endif
#endif
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 7fb659296f..5e2d73fd68 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -32,6 +32,7 @@
#include "trace.h"
#include "options.h"
#include "qapi/error.h"
+#include "rdma.h"
#define IO_BUF_SIZE 32768
#define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
@@ -297,18 +298,6 @@ void qemu_fflush(QEMUFile *f)
f->iovcnt = 0;
}
-void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
-{
- int ret = 0;
-
- if (f->hooks && f->hooks->before_ram_iterate) {
- ret = f->hooks->before_ram_iterate(f, flags, NULL);
- if (ret < 0) {
- qemu_file_set_error(f, ret);
- }
- }
-}
-
void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
{
int ret = 0;
diff --git a/migration/ram.c b/migration/ram.c
index 2f5ce4d60b..ab590a983f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -59,6 +59,7 @@
#include "qemu/iov.h"
#include "multifd.h"
#include "sysemu/runstate.h"
+#include "rdma.h"
#include "options.h"
#include "sysemu/dirtylimit.h"
#include "sysemu/kvm.h"
@@ -3062,7 +3063,10 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
}
}
- ram_control_before_iterate(f, RAM_CONTROL_SETUP);
+ ret = qemu_rdma_registration_start(f, RAM_CONTROL_SETUP);
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
ram_control_after_iterate(f, RAM_CONTROL_SETUP);
migration_ops = g_malloc0(sizeof(MigrationOps));
@@ -3122,7 +3126,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
/* Read version before ram_list.blocks */
smp_rmb();
- ram_control_before_iterate(f, RAM_CONTROL_ROUND);
+ ret = qemu_rdma_registration_start(f, RAM_CONTROL_ROUND);
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
t0 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
i = 0;
@@ -3227,7 +3234,10 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
migration_bitmap_sync_precopy(rs, true);
}
- ram_control_before_iterate(f, RAM_CONTROL_FINISH);
+ ret = qemu_rdma_registration_start(f, RAM_CONTROL_FINISH);
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
/* try transferring iterative blocks of memory */
diff --git a/migration/rdma.c b/migration/rdma.c
index f155f3e1c8..a8bfc052c4 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3850,18 +3850,15 @@ static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
}
}
-static int qemu_rdma_registration_start(QEMUFile *f,
- uint64_t flags, void *data)
+int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
{
- QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
- RDMAContext *rdma;
-
- if (migration_in_postcopy()) {
+ if (!migrate_rdma () || migration_in_postcopy()) {
return 0;
}
+ QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
RCU_READ_LOCK_GUARD();
- rdma = qatomic_rcu_read(&rioc->rdmaout);
+ RDMAContext *rdma = qatomic_rcu_read(&rioc->rdmaout);
if (!rdma) {
return -1;
}
@@ -4002,7 +3999,6 @@ static const QEMUFileHooks rdma_read_hooks = {
};
static const QEMUFileHooks rdma_write_hooks = {
- .before_ram_iterate = qemu_rdma_registration_start,
.after_ram_iterate = qemu_rdma_registration_stop,
.save_page = qemu_rdma_save_page,
};
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 02/13] migration/rdma: Unfold ram_control_before_iterate()
2023-10-11 20:35 ` [PATCH v3 02/13] migration/rdma: Unfold ram_control_before_iterate() Juan Quintela
@ 2023-10-12 14:21 ` Fabiano Rosas
2023-10-13 8:24 ` Zhijian Li (Fujitsu)
1 sibling, 0 replies; 32+ messages in thread
From: Fabiano Rosas @ 2023-10-12 14:21 UTC (permalink / raw)
To: Juan Quintela, qemu-devel
Cc: Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Juan Quintela <quintela@redhat.com> writes:
> Once there:
> - Remove unused data parameter
> - unfold it in its callers.
> - change all callers to call qemu_rdma_registration_start()
> - We need to call QIO_CHANNEL_RDMA() after we check for migrate_rdma()
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> --
>
> initilazize rioc after checknig that rdma is enabled.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 02/13] migration/rdma: Unfold ram_control_before_iterate()
2023-10-11 20:35 ` [PATCH v3 02/13] migration/rdma: Unfold ram_control_before_iterate() Juan Quintela
2023-10-12 14:21 ` Fabiano Rosas
@ 2023-10-13 8:24 ` Zhijian Li (Fujitsu)
1 sibling, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:24 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Once there:
> - Remove unused data parameter
> - unfold it in its callers.
> - change all callers to call qemu_rdma_registration_start()
> - We need to call QIO_CHANNEL_RDMA() after we check for migrate_rdma()
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
>
> --
>
> initilazize rioc after checknig that rdma is enabled.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> migration/qemu-file.h | 2 --
> migration/rdma.h | 7 +++++++
> migration/qemu-file.c | 13 +------------
> migration/ram.c | 16 +++++++++++++---
> migration/rdma.c | 12 ++++--------
> 5 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 03e718c264..d6a370c569 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -55,7 +55,6 @@ typedef int (QEMURamSaveFunc)(QEMUFile *f,
> size_t size);
>
> typedef struct QEMUFileHooks {
> - QEMURamHookFunc *before_ram_iterate;
> QEMURamHookFunc *after_ram_iterate;
> QEMURamHookFunc *hook_ram_load;
> QEMURamSaveFunc *save_page;
> @@ -127,7 +126,6 @@ void qemu_fflush(QEMUFile *f);
> void qemu_file_set_blocking(QEMUFile *f, bool block);
> int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
>
> -void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
> void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
> void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
>
> diff --git a/migration/rdma.h b/migration/rdma.h
> index de2ba09dc5..670c67a8cb 100644
> --- a/migration/rdma.h
> +++ b/migration/rdma.h
> @@ -22,4 +22,11 @@ void rdma_start_outgoing_migration(void *opaque, const char *host_port,
>
> void rdma_start_incoming_migration(const char *host_port, Error **errp);
>
> +
> +#ifdef CONFIG_RDMA
> +int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
> +#else
> +static inline
> +int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
> +#endif
> #endif
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 7fb659296f..5e2d73fd68 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -32,6 +32,7 @@
> #include "trace.h"
> #include "options.h"
> #include "qapi/error.h"
> +#include "rdma.h"
>
> #define IO_BUF_SIZE 32768
> #define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
> @@ -297,18 +298,6 @@ void qemu_fflush(QEMUFile *f)
> f->iovcnt = 0;
> }
>
> -void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
> -{
> - int ret = 0;
> -
> - if (f->hooks && f->hooks->before_ram_iterate) {
> - ret = f->hooks->before_ram_iterate(f, flags, NULL);
> - if (ret < 0) {
> - qemu_file_set_error(f, ret);
> - }
> - }
> -}
> -
> void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
> {
> int ret = 0;
> diff --git a/migration/ram.c b/migration/ram.c
> index 2f5ce4d60b..ab590a983f 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -59,6 +59,7 @@
> #include "qemu/iov.h"
> #include "multifd.h"
> #include "sysemu/runstate.h"
> +#include "rdma.h"
> #include "options.h"
> #include "sysemu/dirtylimit.h"
> #include "sysemu/kvm.h"
> @@ -3062,7 +3063,10 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> }
> }
>
> - ram_control_before_iterate(f, RAM_CONTROL_SETUP);
> + ret = qemu_rdma_registration_start(f, RAM_CONTROL_SETUP);
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
> ram_control_after_iterate(f, RAM_CONTROL_SETUP);
>
> migration_ops = g_malloc0(sizeof(MigrationOps));
> @@ -3122,7 +3126,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> /* Read version before ram_list.blocks */
> smp_rmb();
>
> - ram_control_before_iterate(f, RAM_CONTROL_ROUND);
> + ret = qemu_rdma_registration_start(f, RAM_CONTROL_ROUND);
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
>
> t0 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> i = 0;
> @@ -3227,7 +3234,10 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
> migration_bitmap_sync_precopy(rs, true);
> }
>
> - ram_control_before_iterate(f, RAM_CONTROL_FINISH);
> + ret = qemu_rdma_registration_start(f, RAM_CONTROL_FINISH);
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
>
> /* try transferring iterative blocks of memory */
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index f155f3e1c8..a8bfc052c4 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3850,18 +3850,15 @@ static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
> }
> }
>
> -static int qemu_rdma_registration_start(QEMUFile *f,
> - uint64_t flags, void *data)
> +int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
> {
> - QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> - RDMAContext *rdma;
> -
> - if (migration_in_postcopy()) {
> + if (!migrate_rdma () || migration_in_postcopy()) {
> return 0;
> }
>
> + QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> RCU_READ_LOCK_GUARD();
> - rdma = qatomic_rcu_read(&rioc->rdmaout);
> + RDMAContext *rdma = qatomic_rcu_read(&rioc->rdmaout);
> if (!rdma) {
> return -1;
> }
> @@ -4002,7 +3999,6 @@ static const QEMUFileHooks rdma_read_hooks = {
> };
>
> static const QEMUFileHooks rdma_write_hooks = {
> - .before_ram_iterate = qemu_rdma_registration_start,
> .after_ram_iterate = qemu_rdma_registration_stop,
> .save_page = qemu_rdma_save_page,
> };
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 03/13] migration/rdma: Unfold ram_control_after_iterate()
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
2023-10-11 20:35 ` [PATCH v3 01/13] migration: Create migrate_rdma() Juan Quintela
2023-10-11 20:35 ` [PATCH v3 02/13] migration/rdma: Unfold ram_control_before_iterate() Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 7:59 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 04/13] migration/rdma: Remove all uses of RAM_CONTROL_HOOK Juan Quintela
` (9 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Once there:
- Remove unused data parameter
- unfold it in its callers
- change all callers to call qemu_rdma_registration_stop()
- We need to call QIO_CHANNEL_RDMA() after we check for migrate_rdma()
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
This function has goto's. So I don't change the place where we
declare variables, althought I think that it is correct, just don't
start that discussion.
---
migration/qemu-file.h | 2 --
migration/rdma.h | 3 +++
migration/qemu-file.c | 12 ------------
migration/ram.c | 17 ++++++++++++++---
migration/rdma.c | 9 ++++-----
5 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index d6a370c569..35e671a01e 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -55,7 +55,6 @@ typedef int (QEMURamSaveFunc)(QEMUFile *f,
size_t size);
typedef struct QEMUFileHooks {
- QEMURamHookFunc *after_ram_iterate;
QEMURamHookFunc *hook_ram_load;
QEMURamSaveFunc *save_page;
} QEMUFileHooks;
@@ -126,7 +125,6 @@ void qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
-void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
/* Whenever this is found in the data stream, the flags
diff --git a/migration/rdma.h b/migration/rdma.h
index 670c67a8cb..c13b94c782 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -25,8 +25,11 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp);
#ifdef CONFIG_RDMA
int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
+int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
#else
static inline
int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
+static inline
+int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
#endif
#endif
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 5e2d73fd68..e7dba2a849 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -298,18 +298,6 @@ void qemu_fflush(QEMUFile *f)
f->iovcnt = 0;
}
-void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
-{
- int ret = 0;
-
- if (f->hooks && f->hooks->after_ram_iterate) {
- ret = f->hooks->after_ram_iterate(f, flags, NULL);
- if (ret < 0) {
- qemu_file_set_error(f, ret);
- }
- }
-}
-
void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
{
if (f->hooks && f->hooks->hook_ram_load) {
diff --git a/migration/ram.c b/migration/ram.c
index ab590a983f..15bd4ad697 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3067,7 +3067,11 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
if (ret < 0) {
qemu_file_set_error(f, ret);
}
- ram_control_after_iterate(f, RAM_CONTROL_SETUP);
+
+ ret = qemu_rdma_registration_stop(f, RAM_CONTROL_SETUP);
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
migration_ops = g_malloc0(sizeof(MigrationOps));
migration_ops->ram_save_target_page = ram_save_target_page_legacy;
@@ -3186,7 +3190,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
* Must occur before EOS (or any QEMUFile operation)
* because of RDMA protocol.
*/
- ram_control_after_iterate(f, RAM_CONTROL_ROUND);
+ ret = qemu_rdma_registration_stop(f, RAM_CONTROL_ROUND);
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
out:
if (ret >= 0
@@ -3259,7 +3266,11 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
qemu_mutex_unlock(&rs->bitmap_mutex);
ram_flush_compressed_data(rs);
- ram_control_after_iterate(f, RAM_CONTROL_FINISH);
+
+ int ret = qemu_rdma_registration_stop(f, RAM_CONTROL_FINISH);
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
}
if (ret < 0) {
diff --git a/migration/rdma.c b/migration/rdma.c
index a8bfc052c4..99c0914a23 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3878,20 +3878,20 @@ int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
* Inform dest that dynamic registrations are done for now.
* First, flush writes, if any.
*/
-static int qemu_rdma_registration_stop(QEMUFile *f,
- uint64_t flags, void *data)
+int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags)
{
- QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
+ QIOChannelRDMA *rioc;
Error *err = NULL;
RDMAContext *rdma;
RDMAControlHeader head = { .len = 0, .repeat = 1 };
int ret;
- if (migration_in_postcopy()) {
+ if (!migrate_rdma() || migration_in_postcopy()) {
return 0;
}
RCU_READ_LOCK_GUARD();
+ rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
rdma = qatomic_rcu_read(&rioc->rdmaout);
if (!rdma) {
return -1;
@@ -3999,7 +3999,6 @@ static const QEMUFileHooks rdma_read_hooks = {
};
static const QEMUFileHooks rdma_write_hooks = {
- .after_ram_iterate = qemu_rdma_registration_stop,
.save_page = qemu_rdma_save_page,
};
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 03/13] migration/rdma: Unfold ram_control_after_iterate()
2023-10-11 20:35 ` [PATCH v3 03/13] migration/rdma: Unfold ram_control_after_iterate() Juan Quintela
@ 2023-10-13 7:59 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 7:59 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Once there:
> - Remove unused data parameter
> - unfold it in its callers
> - change all callers to call qemu_rdma_registration_stop()
> - We need to call QIO_CHANNEL_RDMA() after we check for migrate_rdma()
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
>
> ---
>
> This function has goto's. So I don't change the place where we
> declare variables, althought I think that it is correct, just don't
> start that discussion.
> ---
> migration/qemu-file.h | 2 --
> migration/rdma.h | 3 +++
> migration/qemu-file.c | 12 ------------
> migration/ram.c | 17 ++++++++++++++---
> migration/rdma.c | 9 ++++-----
> 5 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index d6a370c569..35e671a01e 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -55,7 +55,6 @@ typedef int (QEMURamSaveFunc)(QEMUFile *f,
> size_t size);
>
> typedef struct QEMUFileHooks {
> - QEMURamHookFunc *after_ram_iterate;
> QEMURamHookFunc *hook_ram_load;
> QEMURamSaveFunc *save_page;
> } QEMUFileHooks;
> @@ -126,7 +125,6 @@ void qemu_fflush(QEMUFile *f);
> void qemu_file_set_blocking(QEMUFile *f, bool block);
> int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
>
> -void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
> void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
>
> /* Whenever this is found in the data stream, the flags
> diff --git a/migration/rdma.h b/migration/rdma.h
> index 670c67a8cb..c13b94c782 100644
> --- a/migration/rdma.h
> +++ b/migration/rdma.h
> @@ -25,8 +25,11 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp);
>
> #ifdef CONFIG_RDMA
> int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
> +int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
> #else
> static inline
> int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
> +static inline
> +int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
> #endif
> #endif
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 5e2d73fd68..e7dba2a849 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -298,18 +298,6 @@ void qemu_fflush(QEMUFile *f)
> f->iovcnt = 0;
> }
>
> -void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
> -{
> - int ret = 0;
> -
> - if (f->hooks && f->hooks->after_ram_iterate) {
> - ret = f->hooks->after_ram_iterate(f, flags, NULL);
> - if (ret < 0) {
> - qemu_file_set_error(f, ret);
> - }
> - }
> -}
> -
> void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
> {
> if (f->hooks && f->hooks->hook_ram_load) {
> diff --git a/migration/ram.c b/migration/ram.c
> index ab590a983f..15bd4ad697 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3067,7 +3067,11 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> - ram_control_after_iterate(f, RAM_CONTROL_SETUP);
> +
> + ret = qemu_rdma_registration_stop(f, RAM_CONTROL_SETUP);
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
>
> migration_ops = g_malloc0(sizeof(MigrationOps));
> migration_ops->ram_save_target_page = ram_save_target_page_legacy;
> @@ -3186,7 +3190,10 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> * Must occur before EOS (or any QEMUFile operation)
> * because of RDMA protocol.
> */
> - ram_control_after_iterate(f, RAM_CONTROL_ROUND);
> + ret = qemu_rdma_registration_stop(f, RAM_CONTROL_ROUND);
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
>
> out:
> if (ret >= 0
> @@ -3259,7 +3266,11 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
> qemu_mutex_unlock(&rs->bitmap_mutex);
>
> ram_flush_compressed_data(rs);
> - ram_control_after_iterate(f, RAM_CONTROL_FINISH);
> +
> + int ret = qemu_rdma_registration_stop(f, RAM_CONTROL_FINISH);
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
> }
>
> if (ret < 0) {
> diff --git a/migration/rdma.c b/migration/rdma.c
> index a8bfc052c4..99c0914a23 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3878,20 +3878,20 @@ int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
> * Inform dest that dynamic registrations are done for now.
> * First, flush writes, if any.
> */
> -static int qemu_rdma_registration_stop(QEMUFile *f,
> - uint64_t flags, void *data)
> +int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags)
> {
> - QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> + QIOChannelRDMA *rioc;
> Error *err = NULL;
> RDMAContext *rdma;
> RDMAControlHeader head = { .len = 0, .repeat = 1 };
> int ret;
>
> - if (migration_in_postcopy()) {
> + if (!migrate_rdma() || migration_in_postcopy()) {
> return 0;
> }
>
> RCU_READ_LOCK_GUARD();
> + rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> rdma = qatomic_rcu_read(&rioc->rdmaout);
> if (!rdma) {
> return -1;
> @@ -3999,7 +3999,6 @@ static const QEMUFileHooks rdma_read_hooks = {
> };
>
> static const QEMUFileHooks rdma_write_hooks = {
> - .after_ram_iterate = qemu_rdma_registration_stop,
> .save_page = qemu_rdma_save_page,
> };
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 04/13] migration/rdma: Remove all uses of RAM_CONTROL_HOOK
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (2 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 03/13] migration/rdma: Unfold ram_control_after_iterate() Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 8:00 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 05/13] migration/rdma: Unfold hook_ram_load() Juan Quintela
` (8 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Instead of going trhough ram_control_load_hook(), call
qemu_rdma_registration_handle() directly.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.h | 1 -
migration/rdma.h | 3 +++
migration/ram.c | 5 ++++-
migration/rdma.c | 12 +++++++-----
4 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 35e671a01e..14ff0d9cc4 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -41,7 +41,6 @@ typedef int (QEMURamHookFunc)(QEMUFile *f, uint64_t flags, void *data);
*/
#define RAM_CONTROL_SETUP 0
#define RAM_CONTROL_ROUND 1
-#define RAM_CONTROL_HOOK 2
#define RAM_CONTROL_FINISH 3
#define RAM_CONTROL_BLOCK_REG 4
diff --git a/migration/rdma.h b/migration/rdma.h
index c13b94c782..8bd277efb9 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -24,10 +24,13 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp);
#ifdef CONFIG_RDMA
+int qemu_rdma_registration_handle(QEMUFile *f);
int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
#else
static inline
+int qemu_rdma_registration_handle(QEMUFile *f) { return 0; }
+static inline
int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
static inline
int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
diff --git a/migration/ram.c b/migration/ram.c
index 15bd4ad697..ee8bdcdc82 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4072,7 +4072,10 @@ static int ram_load_precopy(QEMUFile *f)
}
break;
case RAM_SAVE_FLAG_HOOK:
- ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
+ ret = qemu_rdma_registration_handle(f);
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
break;
default:
error_report("Unknown combination of migration flags: 0x%x", flags);
diff --git a/migration/rdma.c b/migration/rdma.c
index 99c0914a23..e533814599 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3522,7 +3522,7 @@ static int dest_ram_sort_func(const void *a, const void *b)
*
* Keep doing this until the source tells us to stop.
*/
-static int qemu_rdma_registration_handle(QEMUFile *f)
+int qemu_rdma_registration_handle(QEMUFile *f)
{
RDMAControlHeader reg_resp = { .len = sizeof(RDMARegisterResult),
.type = RDMA_CONTROL_REGISTER_RESULT,
@@ -3534,7 +3534,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
};
RDMAControlHeader blocks = { .type = RDMA_CONTROL_RAM_BLOCKS_RESULT,
.repeat = 1 };
- QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
+ QIOChannelRDMA *rioc;
Error *err = NULL;
RDMAContext *rdma;
RDMALocalBlocks *local;
@@ -3550,7 +3550,12 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
int count = 0;
int i = 0;
+ if (!migrate_rdma()) {
+ return 0;
+ }
+
RCU_READ_LOCK_GUARD();
+ rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
rdma = qatomic_rcu_read(&rioc->rdmain);
if (!rdma) {
@@ -3841,9 +3846,6 @@ static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
case RAM_CONTROL_BLOCK_REG:
return rdma_block_notification_handle(f, data);
- case RAM_CONTROL_HOOK:
- return qemu_rdma_registration_handle(f);
-
default:
/* Shouldn't be called with any other values */
abort();
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 04/13] migration/rdma: Remove all uses of RAM_CONTROL_HOOK
2023-10-11 20:35 ` [PATCH v3 04/13] migration/rdma: Remove all uses of RAM_CONTROL_HOOK Juan Quintela
@ 2023-10-13 8:00 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:00 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Instead of going trhough ram_control_load_hook(), call
> qemu_rdma_registration_handle() directly.
>
s/trhough/through
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> migration/qemu-file.h | 1 -
> migration/rdma.h | 3 +++
> migration/ram.c | 5 ++++-
> migration/rdma.c | 12 +++++++-----
> 4 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 35e671a01e..14ff0d9cc4 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -41,7 +41,6 @@ typedef int (QEMURamHookFunc)(QEMUFile *f, uint64_t flags, void *data);
> */
> #define RAM_CONTROL_SETUP 0
> #define RAM_CONTROL_ROUND 1
> -#define RAM_CONTROL_HOOK 2
> #define RAM_CONTROL_FINISH 3
> #define RAM_CONTROL_BLOCK_REG 4
>
> diff --git a/migration/rdma.h b/migration/rdma.h
> index c13b94c782..8bd277efb9 100644
> --- a/migration/rdma.h
> +++ b/migration/rdma.h
> @@ -24,10 +24,13 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp);
>
>
> #ifdef CONFIG_RDMA
> +int qemu_rdma_registration_handle(QEMUFile *f);
> int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
> int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
> #else
> static inline
> +int qemu_rdma_registration_handle(QEMUFile *f) { return 0; }
> +static inline
> int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
> static inline
> int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
> diff --git a/migration/ram.c b/migration/ram.c
> index 15bd4ad697..ee8bdcdc82 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4072,7 +4072,10 @@ static int ram_load_precopy(QEMUFile *f)
> }
> break;
> case RAM_SAVE_FLAG_HOOK:
> - ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
> + ret = qemu_rdma_registration_handle(f);
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
> break;
> default:
> error_report("Unknown combination of migration flags: 0x%x", flags);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 99c0914a23..e533814599 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3522,7 +3522,7 @@ static int dest_ram_sort_func(const void *a, const void *b)
> *
> * Keep doing this until the source tells us to stop.
> */
> -static int qemu_rdma_registration_handle(QEMUFile *f)
> +int qemu_rdma_registration_handle(QEMUFile *f)
> {
> RDMAControlHeader reg_resp = { .len = sizeof(RDMARegisterResult),
> .type = RDMA_CONTROL_REGISTER_RESULT,
> @@ -3534,7 +3534,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
> };
> RDMAControlHeader blocks = { .type = RDMA_CONTROL_RAM_BLOCKS_RESULT,
> .repeat = 1 };
> - QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> + QIOChannelRDMA *rioc;
> Error *err = NULL;
> RDMAContext *rdma;
> RDMALocalBlocks *local;
> @@ -3550,7 +3550,12 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
> int count = 0;
> int i = 0;
>
> + if (!migrate_rdma()) {
> + return 0;
> + }
> +
> RCU_READ_LOCK_GUARD();
> + rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> rdma = qatomic_rcu_read(&rioc->rdmain);
>
> if (!rdma) {
> @@ -3841,9 +3846,6 @@ static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
> case RAM_CONTROL_BLOCK_REG:
> return rdma_block_notification_handle(f, data);
>
> - case RAM_CONTROL_HOOK:
> - return qemu_rdma_registration_handle(f);
> -
> default:
> /* Shouldn't be called with any other values */
> abort();
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 05/13] migration/rdma: Unfold hook_ram_load()
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (3 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 04/13] migration/rdma: Remove all uses of RAM_CONTROL_HOOK Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 8:02 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 06/13] migration/rdma: Create rdma_control_save_page() Juan Quintela
` (7 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
There is only one flag called with: RAM_CONTROL_BLOCK_REG.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.h | 11 -----------
migration/rdma.h | 3 +++
migration/qemu-file.c | 10 ----------
migration/ram.c | 6 ++++--
migration/rdma.c | 34 +++++++++++-----------------------
5 files changed, 18 insertions(+), 46 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 14ff0d9cc4..80c30631dc 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -29,20 +29,12 @@
#include "exec/cpu-common.h"
#include "io/channel.h"
-/*
- * This function provides hooks around different
- * stages of RAM migration.
- * 'data' is call specific data associated with the 'flags' value
- */
-typedef int (QEMURamHookFunc)(QEMUFile *f, uint64_t flags, void *data);
-
/*
* Constants used by ram_control_* hooks
*/
#define RAM_CONTROL_SETUP 0
#define RAM_CONTROL_ROUND 1
#define RAM_CONTROL_FINISH 3
-#define RAM_CONTROL_BLOCK_REG 4
/*
* This function allows override of where the RAM page
@@ -54,7 +46,6 @@ typedef int (QEMURamSaveFunc)(QEMUFile *f,
size_t size);
typedef struct QEMUFileHooks {
- QEMURamHookFunc *hook_ram_load;
QEMURamSaveFunc *save_page;
} QEMUFileHooks;
@@ -124,8 +115,6 @@ void qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
-void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
-
/* Whenever this is found in the data stream, the flags
* will be passed to ram_control_load_hook in the incoming-migration
* side. This lets before_ram_iterate/after_ram_iterate add
diff --git a/migration/rdma.h b/migration/rdma.h
index 8bd277efb9..8df8b4089a 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -27,6 +27,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp);
int qemu_rdma_registration_handle(QEMUFile *f);
int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
+int rdma_block_notification_handle(QEMUFile *f, const char *name);
#else
static inline
int qemu_rdma_registration_handle(QEMUFile *f) { return 0; }
@@ -34,5 +35,7 @@ static inline
int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
static inline
int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
+static inline
+int rdma_block_notification_handle(QEMUFile *f, const char *name) { return 0; }
#endif
#endif
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index e7dba2a849..4a414b8976 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -298,16 +298,6 @@ void qemu_fflush(QEMUFile *f)
f->iovcnt = 0;
}
-void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
-{
- if (f->hooks && f->hooks->hook_ram_load) {
- int ret = f->hooks->hook_ram_load(f, flags, data);
- if (ret < 0) {
- qemu_file_set_error(f, ret);
- }
- }
-}
-
int ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
ram_addr_t offset, size_t size)
{
diff --git a/migration/ram.c b/migration/ram.c
index ee8bdcdc82..d6a9f90b94 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4023,8 +4023,10 @@ static int ram_load_precopy(QEMUFile *f)
ret = -EINVAL;
}
}
- ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG,
- block->idstr);
+ ret = rdma_block_notification_handle(f, block->idstr);
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
} else {
error_report("Unknown ramblock \"%s\", cannot "
"accept migration", id);
diff --git a/migration/rdma.c b/migration/rdma.c
index e533814599..6bca9e8f19 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3799,22 +3799,23 @@ err:
}
/* Destination:
- * Called via a ram_control_load_hook during the initial RAM load section which
- * lists the RAMBlocks by name. This lets us know the order of the RAMBlocks
- * on the source.
- * We've already built our local RAMBlock list, but not yet sent the list to
- * the source.
+ * Called during the initial RAM load section which lists the
+ * RAMBlocks by name. This lets us know the order of the RAMBlocks on
+ * the source. We've already built our local RAMBlock list, but not
+ * yet sent the list to the source.
*/
-static int
-rdma_block_notification_handle(QEMUFile *f, const char *name)
+int rdma_block_notification_handle(QEMUFile *f, const char *name)
{
- RDMAContext *rdma;
- QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
int curr;
int found = -1;
+ if (!migrate_rdma()) {
+ return 0;
+ }
+
RCU_READ_LOCK_GUARD();
- rdma = qatomic_rcu_read(&rioc->rdmain);
+ QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
+ RDMAContext *rdma = qatomic_rcu_read(&rioc->rdmain);
if (!rdma) {
return -1;
@@ -3840,18 +3841,6 @@ rdma_block_notification_handle(QEMUFile *f, const char *name)
return 0;
}
-static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
-{
- switch (flags) {
- case RAM_CONTROL_BLOCK_REG:
- return rdma_block_notification_handle(f, data);
-
- default:
- /* Shouldn't be called with any other values */
- abort();
- }
-}
-
int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
{
if (!migrate_rdma () || migration_in_postcopy()) {
@@ -3997,7 +3986,6 @@ err:
}
static const QEMUFileHooks rdma_read_hooks = {
- .hook_ram_load = rdma_load_hook,
};
static const QEMUFileHooks rdma_write_hooks = {
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 05/13] migration/rdma: Unfold hook_ram_load()
2023-10-11 20:35 ` [PATCH v3 05/13] migration/rdma: Unfold hook_ram_load() Juan Quintela
@ 2023-10-13 8:02 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:02 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> There is only one flag called with: RAM_CONTROL_BLOCK_REG.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> migration/qemu-file.h | 11 -----------
> migration/rdma.h | 3 +++
> migration/qemu-file.c | 10 ----------
> migration/ram.c | 6 ++++--
> migration/rdma.c | 34 +++++++++++-----------------------
> 5 files changed, 18 insertions(+), 46 deletions(-)
>
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 14ff0d9cc4..80c30631dc 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -29,20 +29,12 @@
> #include "exec/cpu-common.h"
> #include "io/channel.h"
>
> -/*
> - * This function provides hooks around different
> - * stages of RAM migration.
> - * 'data' is call specific data associated with the 'flags' value
> - */
> -typedef int (QEMURamHookFunc)(QEMUFile *f, uint64_t flags, void *data);
> -
> /*
> * Constants used by ram_control_* hooks
> */
> #define RAM_CONTROL_SETUP 0
> #define RAM_CONTROL_ROUND 1
> #define RAM_CONTROL_FINISH 3
> -#define RAM_CONTROL_BLOCK_REG 4
>
> /*
> * This function allows override of where the RAM page
> @@ -54,7 +46,6 @@ typedef int (QEMURamSaveFunc)(QEMUFile *f,
> size_t size);
>
> typedef struct QEMUFileHooks {
> - QEMURamHookFunc *hook_ram_load;
> QEMURamSaveFunc *save_page;
> } QEMUFileHooks;
>
> @@ -124,8 +115,6 @@ void qemu_fflush(QEMUFile *f);
> void qemu_file_set_blocking(QEMUFile *f, bool block);
> int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
>
> -void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
> -
> /* Whenever this is found in the data stream, the flags
> * will be passed to ram_control_load_hook in the incoming-migration
> * side. This lets before_ram_iterate/after_ram_iterate add
> diff --git a/migration/rdma.h b/migration/rdma.h
> index 8bd277efb9..8df8b4089a 100644
> --- a/migration/rdma.h
> +++ b/migration/rdma.h
> @@ -27,6 +27,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp);
> int qemu_rdma_registration_handle(QEMUFile *f);
> int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
> int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
> +int rdma_block_notification_handle(QEMUFile *f, const char *name);
> #else
> static inline
> int qemu_rdma_registration_handle(QEMUFile *f) { return 0; }
> @@ -34,5 +35,7 @@ static inline
> int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
> static inline
> int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
> +static inline
> +int rdma_block_notification_handle(QEMUFile *f, const char *name) { return 0; }
> #endif
> #endif
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index e7dba2a849..4a414b8976 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -298,16 +298,6 @@ void qemu_fflush(QEMUFile *f)
> f->iovcnt = 0;
> }
>
> -void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
> -{
> - if (f->hooks && f->hooks->hook_ram_load) {
> - int ret = f->hooks->hook_ram_load(f, flags, data);
> - if (ret < 0) {
> - qemu_file_set_error(f, ret);
> - }
> - }
> -}
> -
> int ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
> ram_addr_t offset, size_t size)
> {
> diff --git a/migration/ram.c b/migration/ram.c
> index ee8bdcdc82..d6a9f90b94 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4023,8 +4023,10 @@ static int ram_load_precopy(QEMUFile *f)
> ret = -EINVAL;
> }
> }
> - ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG,
> - block->idstr);
> + ret = rdma_block_notification_handle(f, block->idstr);
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
> } else {
> error_report("Unknown ramblock \"%s\", cannot "
> "accept migration", id);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index e533814599..6bca9e8f19 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3799,22 +3799,23 @@ err:
> }
>
> /* Destination:
> - * Called via a ram_control_load_hook during the initial RAM load section which
> - * lists the RAMBlocks by name. This lets us know the order of the RAMBlocks
> - * on the source.
> - * We've already built our local RAMBlock list, but not yet sent the list to
> - * the source.
> + * Called during the initial RAM load section which lists the
> + * RAMBlocks by name. This lets us know the order of the RAMBlocks on
> + * the source. We've already built our local RAMBlock list, but not
> + * yet sent the list to the source.
> */
> -static int
> -rdma_block_notification_handle(QEMUFile *f, const char *name)
> +int rdma_block_notification_handle(QEMUFile *f, const char *name)
> {
> - RDMAContext *rdma;
> - QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> int curr;
> int found = -1;
>
> + if (!migrate_rdma()) {
> + return 0;
> + }
> +
> RCU_READ_LOCK_GUARD();
> - rdma = qatomic_rcu_read(&rioc->rdmain);
> + QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> + RDMAContext *rdma = qatomic_rcu_read(&rioc->rdmain);
>
> if (!rdma) {
> return -1;
> @@ -3840,18 +3841,6 @@ rdma_block_notification_handle(QEMUFile *f, const char *name)
> return 0;
> }
>
> -static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
> -{
> - switch (flags) {
> - case RAM_CONTROL_BLOCK_REG:
> - return rdma_block_notification_handle(f, data);
> -
> - default:
> - /* Shouldn't be called with any other values */
> - abort();
> - }
> -}
> -
> int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
> {
> if (!migrate_rdma () || migration_in_postcopy()) {
> @@ -3997,7 +3986,6 @@ err:
> }
>
> static const QEMUFileHooks rdma_read_hooks = {
> - .hook_ram_load = rdma_load_hook,
> };
>
> static const QEMUFileHooks rdma_write_hooks = {
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 06/13] migration/rdma: Create rdma_control_save_page()
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (4 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 05/13] migration/rdma: Unfold hook_ram_load() Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 8:05 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 07/13] qemu-file: Remove QEMUFileHooks Juan Quintela
` (6 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
The only user of ram_control_save_page() and save_page() hook was
rdma. Just move the function to rdma.c, rename it to
rdma_control_save_page().
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.h | 12 ------------
migration/rdma.h | 10 ++++++++++
migration/qemu-file.c | 20 --------------------
migration/ram.c | 4 ++--
migration/rdma.c | 20 +++++++++++++++++++-
5 files changed, 31 insertions(+), 35 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 80c30631dc..60510a2819 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -36,17 +36,7 @@
#define RAM_CONTROL_ROUND 1
#define RAM_CONTROL_FINISH 3
-/*
- * This function allows override of where the RAM page
- * is saved (such as RDMA, for example.)
- */
-typedef int (QEMURamSaveFunc)(QEMUFile *f,
- ram_addr_t block_offset,
- ram_addr_t offset,
- size_t size);
-
typedef struct QEMUFileHooks {
- QEMURamSaveFunc *save_page;
} QEMUFileHooks;
QEMUFile *qemu_file_new_input(QIOChannel *ioc);
@@ -125,8 +115,6 @@ int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
#define RAM_SAVE_CONTROL_NOT_SUPP -1000
#define RAM_SAVE_CONTROL_DELAYED -2000
-int ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
- ram_addr_t offset, size_t size);
QIOChannel *qemu_file_get_ioc(QEMUFile *file);
#endif
diff --git a/migration/rdma.h b/migration/rdma.h
index 8df8b4089a..09a16c1e3c 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -17,6 +17,8 @@
#ifndef QEMU_MIGRATION_RDMA_H
#define QEMU_MIGRATION_RDMA_H
+#include "exec/memory.h"
+
void rdma_start_outgoing_migration(void *opaque, const char *host_port,
Error **errp);
@@ -28,6 +30,8 @@ int qemu_rdma_registration_handle(QEMUFile *f);
int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
int rdma_block_notification_handle(QEMUFile *f, const char *name);
+int rdma_control_save_page(QEMUFile *f, ram_addr_t block_offset,
+ ram_addr_t offset, size_t size);
#else
static inline
int qemu_rdma_registration_handle(QEMUFile *f) { return 0; }
@@ -37,5 +41,11 @@ static inline
int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
static inline
int rdma_block_notification_handle(QEMUFile *f, const char *name) { return 0; }
+static inline
+int rdma_control_save_page(QEMUFile *f, ram_addr_t block_offset,
+ ram_addr_t offset, size_t size)
+{
+ return RAM_SAVE_CONTROL_NOT_SUPP;
+}
#endif
#endif
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 4a414b8976..745eaf7a5b 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -298,26 +298,6 @@ void qemu_fflush(QEMUFile *f)
f->iovcnt = 0;
}
-int ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
- ram_addr_t offset, size_t size)
-{
- if (f->hooks && f->hooks->save_page) {
- int ret = f->hooks->save_page(f, block_offset, offset, size);
- /*
- * RAM_SAVE_CONTROL_* are negative values
- */
- if (ret != RAM_SAVE_CONTROL_DELAYED &&
- ret != RAM_SAVE_CONTROL_NOT_SUPP) {
- if (ret < 0) {
- qemu_file_set_error(f, ret);
- }
- }
- return ret;
- }
-
- return RAM_SAVE_CONTROL_NOT_SUPP;
-}
-
/*
* Attempt to fill the buffer from the underlying file
* Returns the number of bytes read, or negative value for an error.
diff --git a/migration/ram.c b/migration/ram.c
index d6a9f90b94..fd5c61c739 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1197,8 +1197,8 @@ static bool control_save_page(PageSearchStatus *pss, RAMBlock *block,
{
int ret;
- ret = ram_control_save_page(pss->pss_channel, block->offset, offset,
- TARGET_PAGE_SIZE);
+ ret = rdma_control_save_page(pss->pss_channel, block->offset, offset,
+ TARGET_PAGE_SIZE);
if (ret == RAM_SAVE_CONTROL_NOT_SUPP) {
return false;
}
diff --git a/migration/rdma.c b/migration/rdma.c
index 6bca9e8f19..bf16990f91 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3314,6 +3314,25 @@ err:
return -1;
}
+int rdma_control_save_page(QEMUFile *f, ram_addr_t block_offset,
+ ram_addr_t offset, size_t size)
+{
+ if (!migrate_rdma()) {
+ return RAM_SAVE_CONTROL_NOT_SUPP;
+ }
+
+ int ret = qemu_rdma_save_page(f, block_offset, offset, size);
+
+ if (ret != RAM_SAVE_CONTROL_DELAYED &&
+ ret != RAM_SAVE_CONTROL_NOT_SUPP) {
+ if (ret < 0) {
+ qemu_file_set_error(f, ret);
+ }
+ }
+ return ret;
+}
+
+
static void rdma_accept_incoming_migration(void *opaque);
static void rdma_cm_poll_handler(void *opaque)
@@ -3989,7 +4008,6 @@ static const QEMUFileHooks rdma_read_hooks = {
};
static const QEMUFileHooks rdma_write_hooks = {
- .save_page = qemu_rdma_save_page,
};
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 06/13] migration/rdma: Create rdma_control_save_page()
2023-10-11 20:35 ` [PATCH v3 06/13] migration/rdma: Create rdma_control_save_page() Juan Quintela
@ 2023-10-13 8:05 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:05 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> The only user of ram_control_save_page() and save_page() hook was
> rdma. Just move the function to rdma.c, rename it to
> rdma_control_save_page().
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
[...]
>
> +int rdma_control_save_page(QEMUFile *f, ram_addr_t block_offset,
> + ram_addr_t offset, size_t size)
> +{
> + if (!migrate_rdma()) {
> + return RAM_SAVE_CONTROL_NOT_SUPP;
> + }
> +
> + int ret = qemu_rdma_save_page(f, block_offset, offset, size);
> +
> + if (ret != RAM_SAVE_CONTROL_DELAYED &&
> + ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> + if (ret < 0) {
> + qemu_file_set_error(f, ret);
> + }
> + }
> + return ret;
> +}
> +
> +
Redundant new line？
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> static void rdma_accept_incoming_migration(void *opaque);
>
> static void rdma_cm_poll_handler(void *opaque)
> @@ -3989,7 +4008,6 @@ static const QEMUFileHooks rdma_read_hooks = {
> };
>
> static const QEMUFileHooks rdma_write_hooks = {
> - .save_page = qemu_rdma_save_page,
> };
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 07/13] qemu-file: Remove QEMUFileHooks
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (5 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 06/13] migration/rdma: Create rdma_control_save_page() Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 8:07 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 08/13] migration/rdma: Move rdma constants from qemu-file.h to rdma.h Juan Quintela
` (5 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
The only user was rdma, and its use is gone.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.h | 4 ----
migration/qemu-file.c | 6 ------
migration/rdma.c | 9 ---------
3 files changed, 19 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 60510a2819..0b22d8335f 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -36,12 +36,8 @@
#define RAM_CONTROL_ROUND 1
#define RAM_CONTROL_FINISH 3
-typedef struct QEMUFileHooks {
-} QEMUFileHooks;
-
QEMUFile *qemu_file_new_input(QIOChannel *ioc);
QEMUFile *qemu_file_new_output(QIOChannel *ioc);
-void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
int qemu_fclose(QEMUFile *f);
/*
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 745eaf7a5b..3fb25148d1 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -38,7 +38,6 @@
#define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
struct QEMUFile {
- const QEMUFileHooks *hooks;
QIOChannel *ioc;
bool is_writable;
@@ -133,11 +132,6 @@ QEMUFile *qemu_file_new_input(QIOChannel *ioc)
return qemu_file_new_impl(ioc, false);
}
-void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
-{
- f->hooks = hooks;
-}
-
/*
* Get last error for stream f with optional Error*
*
diff --git a/migration/rdma.c b/migration/rdma.c
index bf16990f91..ca2a15be99 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4004,13 +4004,6 @@ err:
return -1;
}
-static const QEMUFileHooks rdma_read_hooks = {
-};
-
-static const QEMUFileHooks rdma_write_hooks = {
-};
-
-
static void qio_channel_rdma_finalize(Object *obj)
{
QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(obj);
@@ -4062,7 +4055,6 @@ static QEMUFile *rdma_new_input(RDMAContext *rdma)
rioc->file = qemu_file_new_input(QIO_CHANNEL(rioc));
rioc->rdmain = rdma;
rioc->rdmaout = rdma->return_path;
- qemu_file_set_hooks(rioc->file, &rdma_read_hooks);
return rioc->file;
}
@@ -4074,7 +4066,6 @@ static QEMUFile *rdma_new_output(RDMAContext *rdma)
rioc->file = qemu_file_new_output(QIO_CHANNEL(rioc));
rioc->rdmaout = rdma;
rioc->rdmain = rdma->return_path;
- qemu_file_set_hooks(rioc->file, &rdma_write_hooks);
return rioc->file;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 07/13] qemu-file: Remove QEMUFileHooks
2023-10-11 20:35 ` [PATCH v3 07/13] qemu-file: Remove QEMUFileHooks Juan Quintela
@ 2023-10-13 8:07 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:07 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> The only user was rdma, and its use is gone.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> migration/qemu-file.h | 4 ----
> migration/qemu-file.c | 6 ------
> migration/rdma.c | 9 ---------
> 3 files changed, 19 deletions(-)
>
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 60510a2819..0b22d8335f 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -36,12 +36,8 @@
> #define RAM_CONTROL_ROUND 1
> #define RAM_CONTROL_FINISH 3
>
> -typedef struct QEMUFileHooks {
> -} QEMUFileHooks;
> -
> QEMUFile *qemu_file_new_input(QIOChannel *ioc);
> QEMUFile *qemu_file_new_output(QIOChannel *ioc);
> -void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
> int qemu_fclose(QEMUFile *f);
>
> /*
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 745eaf7a5b..3fb25148d1 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -38,7 +38,6 @@
> #define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
>
> struct QEMUFile {
> - const QEMUFileHooks *hooks;
> QIOChannel *ioc;
> bool is_writable;
>
> @@ -133,11 +132,6 @@ QEMUFile *qemu_file_new_input(QIOChannel *ioc)
> return qemu_file_new_impl(ioc, false);
> }
>
> -void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
> -{
> - f->hooks = hooks;
> -}
> -
> /*
> * Get last error for stream f with optional Error*
> *
> diff --git a/migration/rdma.c b/migration/rdma.c
> index bf16990f91..ca2a15be99 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -4004,13 +4004,6 @@ err:
> return -1;
> }
>
> -static const QEMUFileHooks rdma_read_hooks = {
> -};
> -
> -static const QEMUFileHooks rdma_write_hooks = {
> -};
> -
> -
> static void qio_channel_rdma_finalize(Object *obj)
> {
> QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(obj);
> @@ -4062,7 +4055,6 @@ static QEMUFile *rdma_new_input(RDMAContext *rdma)
> rioc->file = qemu_file_new_input(QIO_CHANNEL(rioc));
> rioc->rdmain = rdma;
> rioc->rdmaout = rdma->return_path;
> - qemu_file_set_hooks(rioc->file, &rdma_read_hooks);
>
> return rioc->file;
> }
> @@ -4074,7 +4066,6 @@ static QEMUFile *rdma_new_output(RDMAContext *rdma)
> rioc->file = qemu_file_new_output(QIO_CHANNEL(rioc));
> rioc->rdmaout = rdma;
> rioc->rdmain = rdma->return_path;
> - qemu_file_set_hooks(rioc->file, &rdma_write_hooks);
>
> return rioc->file;
> }
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 08/13] migration/rdma: Move rdma constants from qemu-file.h to rdma.h
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (6 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 07/13] qemu-file: Remove QEMUFileHooks Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 8:08 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 09/13] migration/rdma: Remove qemu_ prefix from exported functions Juan Quintela
` (4 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.h | 17 -----------------
migration/rdma.h | 16 ++++++++++++++++
migration/ram.c | 2 +-
3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 0b22d8335f..a29c37b0d0 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -29,13 +29,6 @@
#include "exec/cpu-common.h"
#include "io/channel.h"
-/*
- * Constants used by ram_control_* hooks
- */
-#define RAM_CONTROL_SETUP 0
-#define RAM_CONTROL_ROUND 1
-#define RAM_CONTROL_FINISH 3
-
QEMUFile *qemu_file_new_input(QIOChannel *ioc);
QEMUFile *qemu_file_new_output(QIOChannel *ioc);
int qemu_fclose(QEMUFile *f);
@@ -101,16 +94,6 @@ void qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
-/* Whenever this is found in the data stream, the flags
- * will be passed to ram_control_load_hook in the incoming-migration
- * side. This lets before_ram_iterate/after_ram_iterate add
- * transport-specific sections to the RAM migration data.
- */
-#define RAM_SAVE_FLAG_HOOK 0x80
-
-#define RAM_SAVE_CONTROL_NOT_SUPP -1000
-#define RAM_SAVE_CONTROL_DELAYED -2000
-
QIOChannel *qemu_file_get_ioc(QEMUFile *file);
#endif
diff --git a/migration/rdma.h b/migration/rdma.h
index 09a16c1e3c..1ff3718a76 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -24,6 +24,22 @@ void rdma_start_outgoing_migration(void *opaque, const char *host_port,
void rdma_start_incoming_migration(const char *host_port, Error **errp);
+/*
+ * Constants used by rdma return codes
+ */
+#define RAM_CONTROL_SETUP 0
+#define RAM_CONTROL_ROUND 1
+#define RAM_CONTROL_FINISH 3
+
+/*
+ * Whenever this is found in the data stream, the flags
+ * will be passed to rdma functions in the incoming-migration
+ * side.
+ */
+#define RAM_SAVE_FLAG_HOOK 0x80
+
+#define RAM_SAVE_CONTROL_NOT_SUPP -1000
+#define RAM_SAVE_CONTROL_DELAYED -2000
#ifdef CONFIG_RDMA
int qemu_rdma_registration_handle(QEMUFile *f);
diff --git a/migration/ram.c b/migration/ram.c
index fd5c61c739..f9bbd17028 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -89,7 +89,7 @@
#define RAM_SAVE_FLAG_EOS 0x10
#define RAM_SAVE_FLAG_CONTINUE 0x20
#define RAM_SAVE_FLAG_XBZRLE 0x40
-/* 0x80 is reserved in qemu-file.h for RAM_SAVE_FLAG_HOOK */
+/* 0x80 is reserved in rdma.h for RAM_SAVE_FLAG_HOOK */
#define RAM_SAVE_FLAG_COMPRESS_PAGE 0x100
#define RAM_SAVE_FLAG_MULTIFD_FLUSH 0x200
/* We can't use any flag that is bigger than 0x200 */
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 08/13] migration/rdma: Move rdma constants from qemu-file.h to rdma.h
2023-10-11 20:35 ` [PATCH v3 08/13] migration/rdma: Move rdma constants from qemu-file.h to rdma.h Juan Quintela
@ 2023-10-13 8:08 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:08 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> migration/qemu-file.h | 17 -----------------
> migration/rdma.h | 16 ++++++++++++++++
> migration/ram.c | 2 +-
> 3 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 0b22d8335f..a29c37b0d0 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -29,13 +29,6 @@
> #include "exec/cpu-common.h"
> #include "io/channel.h"
>
> -/*
> - * Constants used by ram_control_* hooks
> - */
> -#define RAM_CONTROL_SETUP 0
> -#define RAM_CONTROL_ROUND 1
> -#define RAM_CONTROL_FINISH 3
> -
> QEMUFile *qemu_file_new_input(QIOChannel *ioc);
> QEMUFile *qemu_file_new_output(QIOChannel *ioc);
> int qemu_fclose(QEMUFile *f);
> @@ -101,16 +94,6 @@ void qemu_fflush(QEMUFile *f);
> void qemu_file_set_blocking(QEMUFile *f, bool block);
> int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
>
> -/* Whenever this is found in the data stream, the flags
> - * will be passed to ram_control_load_hook in the incoming-migration
> - * side. This lets before_ram_iterate/after_ram_iterate add
> - * transport-specific sections to the RAM migration data.
> - */
> -#define RAM_SAVE_FLAG_HOOK 0x80
> -
> -#define RAM_SAVE_CONTROL_NOT_SUPP -1000
> -#define RAM_SAVE_CONTROL_DELAYED -2000
> -
> QIOChannel *qemu_file_get_ioc(QEMUFile *file);
>
> #endif
> diff --git a/migration/rdma.h b/migration/rdma.h
> index 09a16c1e3c..1ff3718a76 100644
> --- a/migration/rdma.h
> +++ b/migration/rdma.h
> @@ -24,6 +24,22 @@ void rdma_start_outgoing_migration(void *opaque, const char *host_port,
>
> void rdma_start_incoming_migration(const char *host_port, Error **errp);
>
> +/*
> + * Constants used by rdma return codes
> + */
> +#define RAM_CONTROL_SETUP 0
> +#define RAM_CONTROL_ROUND 1
> +#define RAM_CONTROL_FINISH 3
> +
> +/*
> + * Whenever this is found in the data stream, the flags
> + * will be passed to rdma functions in the incoming-migration
> + * side.
> + */
> +#define RAM_SAVE_FLAG_HOOK 0x80
> +
> +#define RAM_SAVE_CONTROL_NOT_SUPP -1000
> +#define RAM_SAVE_CONTROL_DELAYED -2000
>
> #ifdef CONFIG_RDMA
> int qemu_rdma_registration_handle(QEMUFile *f);
> diff --git a/migration/ram.c b/migration/ram.c
> index fd5c61c739..f9bbd17028 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -89,7 +89,7 @@
> #define RAM_SAVE_FLAG_EOS 0x10
> #define RAM_SAVE_FLAG_CONTINUE 0x20
> #define RAM_SAVE_FLAG_XBZRLE 0x40
> -/* 0x80 is reserved in qemu-file.h for RAM_SAVE_FLAG_HOOK */
> +/* 0x80 is reserved in rdma.h for RAM_SAVE_FLAG_HOOK */
> #define RAM_SAVE_FLAG_COMPRESS_PAGE 0x100
> #define RAM_SAVE_FLAG_MULTIFD_FLUSH 0x200
> /* We can't use any flag that is bigger than 0x200 */
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 09/13] migration/rdma: Remove qemu_ prefix from exported functions
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (7 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 08/13] migration/rdma: Move rdma constants from qemu-file.h to rdma.h Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 8:09 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 10/13] migration/rdma: Check sooner if we are in postcopy for save_page() Juan Quintela
` (3 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Functions are long enough even without this.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/rdma.h | 12 ++++++------
migration/ram.c | 14 +++++++-------
migration/rdma.c | 40 +++++++++++++++++++---------------------
migration/trace-events | 28 ++++++++++++++--------------
4 files changed, 46 insertions(+), 48 deletions(-)
diff --git a/migration/rdma.h b/migration/rdma.h
index 1ff3718a76..30b15b4466 100644
--- a/migration/rdma.h
+++ b/migration/rdma.h
@@ -42,19 +42,19 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp);
#define RAM_SAVE_CONTROL_DELAYED -2000
#ifdef CONFIG_RDMA
-int qemu_rdma_registration_handle(QEMUFile *f);
-int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
-int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
+int rdma_registration_handle(QEMUFile *f);
+int rdma_registration_start(QEMUFile *f, uint64_t flags);
+int rdma_registration_stop(QEMUFile *f, uint64_t flags);
int rdma_block_notification_handle(QEMUFile *f, const char *name);
int rdma_control_save_page(QEMUFile *f, ram_addr_t block_offset,
ram_addr_t offset, size_t size);
#else
static inline
-int qemu_rdma_registration_handle(QEMUFile *f) { return 0; }
+int rdma_registration_handle(QEMUFile *f) { return 0; }
static inline
-int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
+int rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
static inline
-int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
+int rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
static inline
int rdma_block_notification_handle(QEMUFile *f, const char *name) { return 0; }
static inline
diff --git a/migration/ram.c b/migration/ram.c
index f9bbd17028..2f65535d05 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3063,12 +3063,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
}
}
- ret = qemu_rdma_registration_start(f, RAM_CONTROL_SETUP);
+ ret = rdma_registration_start(f, RAM_CONTROL_SETUP);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
- ret = qemu_rdma_registration_stop(f, RAM_CONTROL_SETUP);
+ ret = rdma_registration_stop(f, RAM_CONTROL_SETUP);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -3130,7 +3130,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
/* Read version before ram_list.blocks */
smp_rmb();
- ret = qemu_rdma_registration_start(f, RAM_CONTROL_ROUND);
+ ret = rdma_registration_start(f, RAM_CONTROL_ROUND);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -3190,7 +3190,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
* Must occur before EOS (or any QEMUFile operation)
* because of RDMA protocol.
*/
- ret = qemu_rdma_registration_stop(f, RAM_CONTROL_ROUND);
+ ret = rdma_registration_stop(f, RAM_CONTROL_ROUND);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -3241,7 +3241,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
migration_bitmap_sync_precopy(rs, true);
}
- ret = qemu_rdma_registration_start(f, RAM_CONTROL_FINISH);
+ ret = rdma_registration_start(f, RAM_CONTROL_FINISH);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -3267,7 +3267,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
ram_flush_compressed_data(rs);
- int ret = qemu_rdma_registration_stop(f, RAM_CONTROL_FINISH);
+ int ret = rdma_registration_stop(f, RAM_CONTROL_FINISH);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -4074,7 +4074,7 @@ static int ram_load_precopy(QEMUFile *f)
}
break;
case RAM_SAVE_FLAG_HOOK:
- ret = qemu_rdma_registration_handle(f);
+ ret = rdma_registration_handle(f);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
diff --git a/migration/rdma.c b/migration/rdma.c
index ca2a15be99..d3bba05262 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3541,7 +3541,7 @@ static int dest_ram_sort_func(const void *a, const void *b)
*
* Keep doing this until the source tells us to stop.
*/
-int qemu_rdma_registration_handle(QEMUFile *f)
+int rdma_registration_handle(QEMUFile *f)
{
RDMAControlHeader reg_resp = { .len = sizeof(RDMARegisterResult),
.type = RDMA_CONTROL_REGISTER_RESULT,
@@ -3587,7 +3587,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
local = &rdma->local_ram_blocks;
do {
- trace_qemu_rdma_registration_handle_wait();
+ trace_rdma_registration_handle_wait();
ret = qemu_rdma_exchange_recv(rdma, &head, RDMA_CONTROL_NONE, &err);
@@ -3607,9 +3607,9 @@ int qemu_rdma_registration_handle(QEMUFile *f)
comp = (RDMACompress *) rdma->wr_data[idx].control_curr;
network_to_compress(comp);
- trace_qemu_rdma_registration_handle_compress(comp->length,
- comp->block_idx,
- comp->offset);
+ trace_rdma_registration_handle_compress(comp->length,
+ comp->block_idx,
+ comp->offset);
if (comp->block_idx >= rdma->local_ram_blocks.nb_blocks) {
error_report("rdma: 'compress' bad block index %u (vs %d)",
(unsigned int)comp->block_idx,
@@ -3625,11 +3625,11 @@ int qemu_rdma_registration_handle(QEMUFile *f)
break;
case RDMA_CONTROL_REGISTER_FINISHED:
- trace_qemu_rdma_registration_handle_finished();
+ trace_rdma_registration_handle_finished();
return 0;
case RDMA_CONTROL_RAM_BLOCKS_REQUEST:
- trace_qemu_rdma_registration_handle_ram_blocks();
+ trace_rdma_registration_handle_ram_blocks();
/* Sort our local RAM Block list so it's the same as the source,
* we can do this since we've filled in a src_index in the list
@@ -3668,7 +3668,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
rdma->dest_blocks[i].length = local->block[i].length;
dest_block_to_network(&rdma->dest_blocks[i]);
- trace_qemu_rdma_registration_handle_ram_blocks_loop(
+ trace_rdma_registration_handle_ram_blocks_loop(
local->block[i].block_name,
local->block[i].offset,
local->block[i].length,
@@ -3691,7 +3691,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
break;
case RDMA_CONTROL_REGISTER_REQUEST:
- trace_qemu_rdma_registration_handle_register(head.repeat);
+ trace_rdma_registration_handle_register(head.repeat);
reg_resp.repeat = head.repeat;
registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
@@ -3705,7 +3705,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
reg_result = &results[count];
- trace_qemu_rdma_registration_handle_register_loop(count,
+ trace_rdma_registration_handle_register_loop(count,
reg->current_index, reg->key.current_addr, reg->chunks);
if (reg->current_index >= rdma->local_ram_blocks.nb_blocks) {
@@ -3753,8 +3753,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
reg_result->host_addr = (uintptr_t)block->local_host_addr;
- trace_qemu_rdma_registration_handle_register_rkey(
- reg_result->rkey);
+ trace_rdma_registration_handle_register_rkey(reg_result->rkey);
result_to_network(reg_result);
}
@@ -3768,7 +3767,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
}
break;
case RDMA_CONTROL_UNREGISTER_REQUEST:
- trace_qemu_rdma_registration_handle_unregister(head.repeat);
+ trace_rdma_registration_handle_unregister(head.repeat);
unreg_resp.repeat = head.repeat;
registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
@@ -3776,7 +3775,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
reg = ®isters[count];
network_to_register(reg);
- trace_qemu_rdma_registration_handle_unregister_loop(count,
+ trace_rdma_registration_handle_unregister_loop(count,
reg->current_index, reg->key.chunk);
block = &(rdma->local_ram_blocks.block[reg->current_index]);
@@ -3792,8 +3791,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
rdma->total_registrations--;
- trace_qemu_rdma_registration_handle_unregister_success(
- reg->key.chunk);
+ trace_rdma_registration_handle_unregister_success(reg->key.chunk);
}
ret = qemu_rdma_post_send_control(rdma, NULL, &unreg_resp, &err);
@@ -3860,7 +3858,7 @@ int rdma_block_notification_handle(QEMUFile *f, const char *name)
return 0;
}
-int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
+int rdma_registration_start(QEMUFile *f, uint64_t flags)
{
if (!migrate_rdma () || migration_in_postcopy()) {
return 0;
@@ -3877,7 +3875,7 @@ int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
return -1;
}
- trace_qemu_rdma_registration_start(flags);
+ trace_rdma_registration_start(flags);
qemu_put_be64(f, RAM_SAVE_FLAG_HOOK);
qemu_fflush(f);
@@ -3888,7 +3886,7 @@ int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
* Inform dest that dynamic registrations are done for now.
* First, flush writes, if any.
*/
-int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags)
+int rdma_registration_stop(QEMUFile *f, uint64_t flags)
{
QIOChannelRDMA *rioc;
Error *err = NULL;
@@ -3924,7 +3922,7 @@ int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags)
int reg_result_idx, i, nb_dest_blocks;
head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
- trace_qemu_rdma_registration_stop_ram();
+ trace_rdma_registration_stop_ram();
/*
* Make sure that we parallelize the pinning on both sides.
@@ -3988,7 +3986,7 @@ int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags)
}
}
- trace_qemu_rdma_registration_stop(flags);
+ trace_rdma_registration_stop(flags);
head.type = RDMA_CONTROL_REGISTER_FINISHED;
ret = qemu_rdma_exchange_send(rdma, &head, NULL, NULL, NULL, NULL, &err);
diff --git a/migration/trace-events b/migration/trace-events
index ee9c8f4d63..b56442d7dd 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -231,20 +231,6 @@ qemu_rdma_post_send_control(const char *desc) "CONTROL: sending %s.."
qemu_rdma_register_and_get_keys(uint64_t len, void *start) "Registering %" PRIu64 " bytes @ %p"
qemu_rdma_register_odp_mr(const char *name) "Try to register On-Demand Paging memory region: %s"
qemu_rdma_advise_mr(const char *name, uint32_t len, uint64_t addr, const char *res) "Try to advise block %s prefetch at %" PRIu32 "@0x%" PRIx64 ": %s"
-qemu_rdma_registration_handle_compress(int64_t length, int index, int64_t offset) "Zapping zero chunk: %" PRId64 " bytes, index %d, offset %" PRId64
-qemu_rdma_registration_handle_finished(void) ""
-qemu_rdma_registration_handle_ram_blocks(void) ""
-qemu_rdma_registration_handle_ram_blocks_loop(const char *name, uint64_t offset, uint64_t length, void *local_host_addr, unsigned int src_index) "%s: @0x%" PRIx64 "/%" PRIu64 " host:@%p src_index: %u"
-qemu_rdma_registration_handle_register(int requests) "%d requests"
-qemu_rdma_registration_handle_register_loop(int req, int index, uint64_t addr, uint64_t chunks) "Registration request (%d): index %d, current_addr %" PRIu64 " chunks: %" PRIu64
-qemu_rdma_registration_handle_register_rkey(int rkey) "0x%x"
-qemu_rdma_registration_handle_unregister(int requests) "%d requests"
-qemu_rdma_registration_handle_unregister_loop(int count, int index, uint64_t chunk) "Unregistration request (%d): index %d, chunk %" PRIu64
-qemu_rdma_registration_handle_unregister_success(uint64_t chunk) "%" PRIu64
-qemu_rdma_registration_handle_wait(void) ""
-qemu_rdma_registration_start(uint64_t flags) "%" PRIu64
-qemu_rdma_registration_stop(uint64_t flags) "%" PRIu64
-qemu_rdma_registration_stop_ram(void) ""
qemu_rdma_resolve_host_trying(const char *host, const char *ip) "Trying %s => %s"
qemu_rdma_signal_unregister_append(uint64_t chunk, int pos) "Appending unregister chunk %" PRIu64 " at position %d"
qemu_rdma_signal_unregister_already(uint64_t chunk) "Unregister chunk %" PRIu64 " already in queue"
@@ -263,6 +249,20 @@ qemu_rdma_write_one_zero(uint64_t chunk, int len, int index, int64_t offset) "En
rdma_add_block(const char *block_name, int block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Added Block: '%s':%d, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d"
rdma_block_notification_handle(const char *name, int index) "%s at %d"
rdma_delete_block(void *block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Deleted Block: %p, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d"
+rdma_registration_handle_compress(int64_t length, int index, int64_t offset) "Zapping zero chunk: %" PRId64 " bytes, index %d, offset %" PRId64
+rdma_registration_handle_finished(void) ""
+rdma_registration_handle_ram_blocks(void) ""
+rdma_registration_handle_ram_blocks_loop(const char *name, uint64_t offset, uint64_t length, void *local_host_addr, unsigned int src_index) "%s: @0x%" PRIx64 "/%" PRIu64 " host:@%p src_index: %u"
+rdma_registration_handle_register(int requests) "%d requests"
+rdma_registration_handle_register_loop(int req, int index, uint64_t addr, uint64_t chunks) "Registration request (%d): index %d, current_addr %" PRIu64 " chunks: %" PRIu64
+rdma_registration_handle_register_rkey(int rkey) "0x%x"
+rdma_registration_handle_unregister(int requests) "%d requests"
+rdma_registration_handle_unregister_loop(int count, int index, uint64_t chunk) "Unregistration request (%d): index %d, chunk %" PRIu64
+rdma_registration_handle_unregister_success(uint64_t chunk) "%" PRIu64
+rdma_registration_handle_wait(void) ""
+rdma_registration_start(uint64_t flags) "%" PRIu64
+rdma_registration_stop(uint64_t flags) "%" PRIu64
+rdma_registration_stop_ram(void) ""
rdma_start_incoming_migration(void) ""
rdma_start_incoming_migration_after_dest_init(void) ""
rdma_start_incoming_migration_after_rdma_listen(void) ""
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 09/13] migration/rdma: Remove qemu_ prefix from exported functions
2023-10-11 20:35 ` [PATCH v3 09/13] migration/rdma: Remove qemu_ prefix from exported functions Juan Quintela
@ 2023-10-13 8:09 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:09 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Functions are long enough even without this.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> migration/rdma.h | 12 ++++++------
> migration/ram.c | 14 +++++++-------
> migration/rdma.c | 40 +++++++++++++++++++---------------------
> migration/trace-events | 28 ++++++++++++++--------------
> 4 files changed, 46 insertions(+), 48 deletions(-)
>
> diff --git a/migration/rdma.h b/migration/rdma.h
> index 1ff3718a76..30b15b4466 100644
> --- a/migration/rdma.h
> +++ b/migration/rdma.h
> @@ -42,19 +42,19 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp);
> #define RAM_SAVE_CONTROL_DELAYED -2000
>
> #ifdef CONFIG_RDMA
> -int qemu_rdma_registration_handle(QEMUFile *f);
> -int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags);
> -int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags);
> +int rdma_registration_handle(QEMUFile *f);
> +int rdma_registration_start(QEMUFile *f, uint64_t flags);
> +int rdma_registration_stop(QEMUFile *f, uint64_t flags);
> int rdma_block_notification_handle(QEMUFile *f, const char *name);
> int rdma_control_save_page(QEMUFile *f, ram_addr_t block_offset,
> ram_addr_t offset, size_t size);
> #else
> static inline
> -int qemu_rdma_registration_handle(QEMUFile *f) { return 0; }
> +int rdma_registration_handle(QEMUFile *f) { return 0; }
> static inline
> -int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
> +int rdma_registration_start(QEMUFile *f, uint64_t flags) { return 0; }
> static inline
> -int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
> +int rdma_registration_stop(QEMUFile *f, uint64_t flags) { return 0; }
> static inline
> int rdma_block_notification_handle(QEMUFile *f, const char *name) { return 0; }
> static inline
> diff --git a/migration/ram.c b/migration/ram.c
> index f9bbd17028..2f65535d05 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3063,12 +3063,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> }
> }
>
> - ret = qemu_rdma_registration_start(f, RAM_CONTROL_SETUP);
> + ret = rdma_registration_start(f, RAM_CONTROL_SETUP);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
>
> - ret = qemu_rdma_registration_stop(f, RAM_CONTROL_SETUP);
> + ret = rdma_registration_stop(f, RAM_CONTROL_SETUP);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -3130,7 +3130,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> /* Read version before ram_list.blocks */
> smp_rmb();
>
> - ret = qemu_rdma_registration_start(f, RAM_CONTROL_ROUND);
> + ret = rdma_registration_start(f, RAM_CONTROL_ROUND);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -3190,7 +3190,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> * Must occur before EOS (or any QEMUFile operation)
> * because of RDMA protocol.
> */
> - ret = qemu_rdma_registration_stop(f, RAM_CONTROL_ROUND);
> + ret = rdma_registration_stop(f, RAM_CONTROL_ROUND);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -3241,7 +3241,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
> migration_bitmap_sync_precopy(rs, true);
> }
>
> - ret = qemu_rdma_registration_start(f, RAM_CONTROL_FINISH);
> + ret = rdma_registration_start(f, RAM_CONTROL_FINISH);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -3267,7 +3267,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>
> ram_flush_compressed_data(rs);
>
> - int ret = qemu_rdma_registration_stop(f, RAM_CONTROL_FINISH);
> + int ret = rdma_registration_stop(f, RAM_CONTROL_FINISH);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -4074,7 +4074,7 @@ static int ram_load_precopy(QEMUFile *f)
> }
> break;
> case RAM_SAVE_FLAG_HOOK:
> - ret = qemu_rdma_registration_handle(f);
> + ret = rdma_registration_handle(f);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> diff --git a/migration/rdma.c b/migration/rdma.c
> index ca2a15be99..d3bba05262 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3541,7 +3541,7 @@ static int dest_ram_sort_func(const void *a, const void *b)
> *
> * Keep doing this until the source tells us to stop.
> */
> -int qemu_rdma_registration_handle(QEMUFile *f)
> +int rdma_registration_handle(QEMUFile *f)
> {
> RDMAControlHeader reg_resp = { .len = sizeof(RDMARegisterResult),
> .type = RDMA_CONTROL_REGISTER_RESULT,
> @@ -3587,7 +3587,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
>
> local = &rdma->local_ram_blocks;
> do {
> - trace_qemu_rdma_registration_handle_wait();
> + trace_rdma_registration_handle_wait();
>
> ret = qemu_rdma_exchange_recv(rdma, &head, RDMA_CONTROL_NONE, &err);
>
> @@ -3607,9 +3607,9 @@ int qemu_rdma_registration_handle(QEMUFile *f)
> comp = (RDMACompress *) rdma->wr_data[idx].control_curr;
> network_to_compress(comp);
>
> - trace_qemu_rdma_registration_handle_compress(comp->length,
> - comp->block_idx,
> - comp->offset);
> + trace_rdma_registration_handle_compress(comp->length,
> + comp->block_idx,
> + comp->offset);
> if (comp->block_idx >= rdma->local_ram_blocks.nb_blocks) {
> error_report("rdma: 'compress' bad block index %u (vs %d)",
> (unsigned int)comp->block_idx,
> @@ -3625,11 +3625,11 @@ int qemu_rdma_registration_handle(QEMUFile *f)
> break;
>
> case RDMA_CONTROL_REGISTER_FINISHED:
> - trace_qemu_rdma_registration_handle_finished();
> + trace_rdma_registration_handle_finished();
> return 0;
>
> case RDMA_CONTROL_RAM_BLOCKS_REQUEST:
> - trace_qemu_rdma_registration_handle_ram_blocks();
> + trace_rdma_registration_handle_ram_blocks();
>
> /* Sort our local RAM Block list so it's the same as the source,
> * we can do this since we've filled in a src_index in the list
> @@ -3668,7 +3668,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
> rdma->dest_blocks[i].length = local->block[i].length;
>
> dest_block_to_network(&rdma->dest_blocks[i]);
> - trace_qemu_rdma_registration_handle_ram_blocks_loop(
> + trace_rdma_registration_handle_ram_blocks_loop(
> local->block[i].block_name,
> local->block[i].offset,
> local->block[i].length,
> @@ -3691,7 +3691,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
>
> break;
> case RDMA_CONTROL_REGISTER_REQUEST:
> - trace_qemu_rdma_registration_handle_register(head.repeat);
> + trace_rdma_registration_handle_register(head.repeat);
>
> reg_resp.repeat = head.repeat;
> registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
> @@ -3705,7 +3705,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
>
> reg_result = &results[count];
>
> - trace_qemu_rdma_registration_handle_register_loop(count,
> + trace_rdma_registration_handle_register_loop(count,
> reg->current_index, reg->key.current_addr, reg->chunks);
>
> if (reg->current_index >= rdma->local_ram_blocks.nb_blocks) {
> @@ -3753,8 +3753,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
>
> reg_result->host_addr = (uintptr_t)block->local_host_addr;
>
> - trace_qemu_rdma_registration_handle_register_rkey(
> - reg_result->rkey);
> + trace_rdma_registration_handle_register_rkey(reg_result->rkey);
>
> result_to_network(reg_result);
> }
> @@ -3768,7 +3767,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
> }
> break;
> case RDMA_CONTROL_UNREGISTER_REQUEST:
> - trace_qemu_rdma_registration_handle_unregister(head.repeat);
> + trace_rdma_registration_handle_unregister(head.repeat);
> unreg_resp.repeat = head.repeat;
> registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
>
> @@ -3776,7 +3775,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
> reg = ®isters[count];
> network_to_register(reg);
>
> - trace_qemu_rdma_registration_handle_unregister_loop(count,
> + trace_rdma_registration_handle_unregister_loop(count,
> reg->current_index, reg->key.chunk);
>
> block = &(rdma->local_ram_blocks.block[reg->current_index]);
> @@ -3792,8 +3791,7 @@ int qemu_rdma_registration_handle(QEMUFile *f)
>
> rdma->total_registrations--;
>
> - trace_qemu_rdma_registration_handle_unregister_success(
> - reg->key.chunk);
> + trace_rdma_registration_handle_unregister_success(reg->key.chunk);
> }
>
> ret = qemu_rdma_post_send_control(rdma, NULL, &unreg_resp, &err);
> @@ -3860,7 +3858,7 @@ int rdma_block_notification_handle(QEMUFile *f, const char *name)
> return 0;
> }
>
> -int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
> +int rdma_registration_start(QEMUFile *f, uint64_t flags)
> {
> if (!migrate_rdma () || migration_in_postcopy()) {
> return 0;
> @@ -3877,7 +3875,7 @@ int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
> return -1;
> }
>
> - trace_qemu_rdma_registration_start(flags);
> + trace_rdma_registration_start(flags);
> qemu_put_be64(f, RAM_SAVE_FLAG_HOOK);
> qemu_fflush(f);
>
> @@ -3888,7 +3886,7 @@ int qemu_rdma_registration_start(QEMUFile *f, uint64_t flags)
> * Inform dest that dynamic registrations are done for now.
> * First, flush writes, if any.
> */
> -int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags)
> +int rdma_registration_stop(QEMUFile *f, uint64_t flags)
> {
> QIOChannelRDMA *rioc;
> Error *err = NULL;
> @@ -3924,7 +3922,7 @@ int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags)
> int reg_result_idx, i, nb_dest_blocks;
>
> head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
> - trace_qemu_rdma_registration_stop_ram();
> + trace_rdma_registration_stop_ram();
>
> /*
> * Make sure that we parallelize the pinning on both sides.
> @@ -3988,7 +3986,7 @@ int qemu_rdma_registration_stop(QEMUFile *f, uint64_t flags)
> }
> }
>
> - trace_qemu_rdma_registration_stop(flags);
> + trace_rdma_registration_stop(flags);
>
> head.type = RDMA_CONTROL_REGISTER_FINISHED;
> ret = qemu_rdma_exchange_send(rdma, &head, NULL, NULL, NULL, NULL, &err);
> diff --git a/migration/trace-events b/migration/trace-events
> index ee9c8f4d63..b56442d7dd 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -231,20 +231,6 @@ qemu_rdma_post_send_control(const char *desc) "CONTROL: sending %s.."
> qemu_rdma_register_and_get_keys(uint64_t len, void *start) "Registering %" PRIu64 " bytes @ %p"
> qemu_rdma_register_odp_mr(const char *name) "Try to register On-Demand Paging memory region: %s"
> qemu_rdma_advise_mr(const char *name, uint32_t len, uint64_t addr, const char *res) "Try to advise block %s prefetch at %" PRIu32 "@0x%" PRIx64 ": %s"
> -qemu_rdma_registration_handle_compress(int64_t length, int index, int64_t offset) "Zapping zero chunk: %" PRId64 " bytes, index %d, offset %" PRId64
> -qemu_rdma_registration_handle_finished(void) ""
> -qemu_rdma_registration_handle_ram_blocks(void) ""
> -qemu_rdma_registration_handle_ram_blocks_loop(const char *name, uint64_t offset, uint64_t length, void *local_host_addr, unsigned int src_index) "%s: @0x%" PRIx64 "/%" PRIu64 " host:@%p src_index: %u"
> -qemu_rdma_registration_handle_register(int requests) "%d requests"
> -qemu_rdma_registration_handle_register_loop(int req, int index, uint64_t addr, uint64_t chunks) "Registration request (%d): index %d, current_addr %" PRIu64 " chunks: %" PRIu64
> -qemu_rdma_registration_handle_register_rkey(int rkey) "0x%x"
> -qemu_rdma_registration_handle_unregister(int requests) "%d requests"
> -qemu_rdma_registration_handle_unregister_loop(int count, int index, uint64_t chunk) "Unregistration request (%d): index %d, chunk %" PRIu64
> -qemu_rdma_registration_handle_unregister_success(uint64_t chunk) "%" PRIu64
> -qemu_rdma_registration_handle_wait(void) ""
> -qemu_rdma_registration_start(uint64_t flags) "%" PRIu64
> -qemu_rdma_registration_stop(uint64_t flags) "%" PRIu64
> -qemu_rdma_registration_stop_ram(void) ""
> qemu_rdma_resolve_host_trying(const char *host, const char *ip) "Trying %s => %s"
> qemu_rdma_signal_unregister_append(uint64_t chunk, int pos) "Appending unregister chunk %" PRIu64 " at position %d"
> qemu_rdma_signal_unregister_already(uint64_t chunk) "Unregister chunk %" PRIu64 " already in queue"
> @@ -263,6 +249,20 @@ qemu_rdma_write_one_zero(uint64_t chunk, int len, int index, int64_t offset) "En
> rdma_add_block(const char *block_name, int block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Added Block: '%s':%d, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d"
> rdma_block_notification_handle(const char *name, int index) "%s at %d"
> rdma_delete_block(void *block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Deleted Block: %p, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d"
> +rdma_registration_handle_compress(int64_t length, int index, int64_t offset) "Zapping zero chunk: %" PRId64 " bytes, index %d, offset %" PRId64
> +rdma_registration_handle_finished(void) ""
> +rdma_registration_handle_ram_blocks(void) ""
> +rdma_registration_handle_ram_blocks_loop(const char *name, uint64_t offset, uint64_t length, void *local_host_addr, unsigned int src_index) "%s: @0x%" PRIx64 "/%" PRIu64 " host:@%p src_index: %u"
> +rdma_registration_handle_register(int requests) "%d requests"
> +rdma_registration_handle_register_loop(int req, int index, uint64_t addr, uint64_t chunks) "Registration request (%d): index %d, current_addr %" PRIu64 " chunks: %" PRIu64
> +rdma_registration_handle_register_rkey(int rkey) "0x%x"
> +rdma_registration_handle_unregister(int requests) "%d requests"
> +rdma_registration_handle_unregister_loop(int count, int index, uint64_t chunk) "Unregistration request (%d): index %d, chunk %" PRIu64
> +rdma_registration_handle_unregister_success(uint64_t chunk) "%" PRIu64
> +rdma_registration_handle_wait(void) ""
> +rdma_registration_start(uint64_t flags) "%" PRIu64
> +rdma_registration_stop(uint64_t flags) "%" PRIu64
> +rdma_registration_stop_ram(void) ""
> rdma_start_incoming_migration(void) ""
> rdma_start_incoming_migration_after_dest_init(void) ""
> rdma_start_incoming_migration_after_rdma_listen(void) ""
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 10/13] migration/rdma: Check sooner if we are in postcopy for save_page()
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (8 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 09/13] migration/rdma: Remove qemu_ prefix from exported functions Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-13 8:11 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 11/13] migration/rdma: Use i as for index instead of idx Juan Quintela
` (2 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/rdma.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index d3bba05262..932d4eda9b 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3240,10 +3240,6 @@ static int qemu_rdma_save_page(QEMUFile *f, ram_addr_t block_offset,
RDMAContext *rdma;
int ret;
- if (migration_in_postcopy()) {
- return RAM_SAVE_CONTROL_NOT_SUPP;
- }
-
RCU_READ_LOCK_GUARD();
rdma = qatomic_rcu_read(&rioc->rdmaout);
@@ -3317,7 +3313,7 @@ err:
int rdma_control_save_page(QEMUFile *f, ram_addr_t block_offset,
ram_addr_t offset, size_t size)
{
- if (!migrate_rdma()) {
+ if (!migrate_rdma() || migration_in_postcopy()) {
return RAM_SAVE_CONTROL_NOT_SUPP;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 10/13] migration/rdma: Check sooner if we are in postcopy for save_page()
2023-10-11 20:35 ` [PATCH v3 10/13] migration/rdma: Check sooner if we are in postcopy for save_page() Juan Quintela
@ 2023-10-13 8:11 ` Zhijian Li (Fujitsu)
0 siblings, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:11 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> migration/rdma.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index d3bba05262..932d4eda9b 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3240,10 +3240,6 @@ static int qemu_rdma_save_page(QEMUFile *f, ram_addr_t block_offset,
> RDMAContext *rdma;
> int ret;
>
> - if (migration_in_postcopy()) {
> - return RAM_SAVE_CONTROL_NOT_SUPP;
> - }
> -
> RCU_READ_LOCK_GUARD();
> rdma = qatomic_rcu_read(&rioc->rdmaout);
>
> @@ -3317,7 +3313,7 @@ err:
> int rdma_control_save_page(QEMUFile *f, ram_addr_t block_offset,
> ram_addr_t offset, size_t size)
> {
> - if (!migrate_rdma()) {
> + if (!migrate_rdma() || migration_in_postcopy()) {
> return RAM_SAVE_CONTROL_NOT_SUPP;
> }
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 11/13] migration/rdma: Use i as for index instead of idx
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (9 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 10/13] migration/rdma: Check sooner if we are in postcopy for save_page() Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-12 14:30 ` Fabiano Rosas
2023-10-13 8:13 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 12/13] migration/rdma: Declare for index variables local Juan Quintela
2023-10-11 20:35 ` [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once Juan Quintela
12 siblings, 2 replies; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Once there, all the uses are local to the for, so declare the variable
inside the for statement.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/rdma.c | 49 ++++++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index 932d4eda9b..e29e5551d1 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2354,7 +2354,6 @@ static int qemu_rdma_write(RDMAContext *rdma,
static void qemu_rdma_cleanup(RDMAContext *rdma)
{
Error *err = NULL;
- int idx;
if (rdma->cm_id && rdma->connected) {
if ((rdma->errored ||
@@ -2381,12 +2380,12 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
g_free(rdma->dest_blocks);
rdma->dest_blocks = NULL;
- for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
- if (rdma->wr_data[idx].control_mr) {
+ for (int i = 0; i < RDMA_WRID_MAX; i++) {
+ if (rdma->wr_data[i].control_mr) {
rdma->total_registrations--;
- ibv_dereg_mr(rdma->wr_data[idx].control_mr);
+ ibv_dereg_mr(rdma->wr_data[i].control_mr);
}
- rdma->wr_data[idx].control_mr = NULL;
+ rdma->wr_data[i].control_mr = NULL;
}
if (rdma->local_ram_blocks.block) {
@@ -2452,7 +2451,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
static int qemu_rdma_source_init(RDMAContext *rdma, bool pin_all, Error **errp)
{
- int ret, idx;
+ int ret;
/*
* Will be validated against destination's actual capabilities
@@ -2480,18 +2479,17 @@ static int qemu_rdma_source_init(RDMAContext *rdma, bool pin_all, Error **errp)
/* Build the hash that maps from offset to RAMBlock */
rdma->blockmap = g_hash_table_new(g_direct_hash, g_direct_equal);
- for (idx = 0; idx < rdma->local_ram_blocks.nb_blocks; idx++) {
+ for (int i = 0; i < rdma->local_ram_blocks.nb_blocks; i++) {
g_hash_table_insert(rdma->blockmap,
- (void *)(uintptr_t)rdma->local_ram_blocks.block[idx].offset,
- &rdma->local_ram_blocks.block[idx]);
+ (void *)(uintptr_t)rdma->local_ram_blocks.block[i].offset,
+ &rdma->local_ram_blocks.block[i]);
}
- for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
- ret = qemu_rdma_reg_control(rdma, idx);
+ for (int i = 0; i < RDMA_WRID_MAX; i++) {
+ ret = qemu_rdma_reg_control(rdma, i);
if (ret < 0) {
- error_setg(errp,
- "RDMA ERROR: rdma migration: error registering %d control!",
- idx);
+ error_setg(errp, "RDMA ERROR: rdma migration: error "
+ "registering %d control!", i);
goto err_rdma_source_init;
}
}
@@ -2625,16 +2623,16 @@ err_rdma_source_connect:
static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
{
Error *err = NULL;
- int ret, idx;
+ int ret;
struct rdma_cm_id *listen_id;
char ip[40] = "unknown";
struct rdma_addrinfo *res, *e;
char port_str[16];
int reuse = 1;
- for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
- rdma->wr_data[idx].control_len = 0;
- rdma->wr_data[idx].control_curr = NULL;
+ for (int i = 0; i < RDMA_WRID_MAX; i++) {
+ rdma->wr_data[i].control_len = 0;
+ rdma->wr_data[i].control_curr = NULL;
}
if (!rdma->host || !rdma->host[0]) {
@@ -2723,11 +2721,9 @@ err_dest_init_create_listen_id:
static void qemu_rdma_return_path_dest_init(RDMAContext *rdma_return_path,
RDMAContext *rdma)
{
- int idx;
-
- for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
- rdma_return_path->wr_data[idx].control_len = 0;
- rdma_return_path->wr_data[idx].control_curr = NULL;
+ for (int i = 0; i < RDMA_WRID_MAX; i++) {
+ rdma_return_path->wr_data[i].control_len = 0;
+ rdma_return_path->wr_data[i].control_curr = NULL;
}
/*the CM channel and CM id is shared*/
@@ -3377,7 +3373,6 @@ static int qemu_rdma_accept(RDMAContext *rdma)
struct rdma_cm_event *cm_event;
struct ibv_context *verbs;
int ret;
- int idx;
ret = rdma_get_cm_event(rdma->channel, &cm_event);
if (ret < 0) {
@@ -3463,10 +3458,10 @@ static int qemu_rdma_accept(RDMAContext *rdma)
qemu_rdma_init_ram_blocks(rdma);
- for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
- ret = qemu_rdma_reg_control(rdma, idx);
+ for (int i = 0; i < RDMA_WRID_MAX; i++) {
+ ret = qemu_rdma_reg_control(rdma, i);
if (ret < 0) {
- error_report("rdma: error registering %d control", idx);
+ error_report("rdma: error registering %d control", i);
goto err_rdma_dest_wait;
}
}
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 11/13] migration/rdma: Use i as for index instead of idx
2023-10-11 20:35 ` [PATCH v3 11/13] migration/rdma: Use i as for index instead of idx Juan Quintela
@ 2023-10-12 14:30 ` Fabiano Rosas
2023-10-13 8:13 ` Zhijian Li (Fujitsu)
1 sibling, 0 replies; 32+ messages in thread
From: Fabiano Rosas @ 2023-10-12 14:30 UTC (permalink / raw)
To: Juan Quintela, qemu-devel
Cc: Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Juan Quintela <quintela@redhat.com> writes:
> Once there, all the uses are local to the for, so declare the variable
> inside the for statement.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 11/13] migration/rdma: Use i as for index instead of idx
2023-10-11 20:35 ` [PATCH v3 11/13] migration/rdma: Use i as for index instead of idx Juan Quintela
2023-10-12 14:30 ` Fabiano Rosas
@ 2023-10-13 8:13 ` Zhijian Li (Fujitsu)
1 sibling, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:13 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Once there, all the uses are local to the for, so declare the variable
> inside the for statement.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> migration/rdma.c | 49 ++++++++++++++++++++++--------------------------
> 1 file changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 932d4eda9b..e29e5551d1 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2354,7 +2354,6 @@ static int qemu_rdma_write(RDMAContext *rdma,
> static void qemu_rdma_cleanup(RDMAContext *rdma)
> {
> Error *err = NULL;
> - int idx;
>
> if (rdma->cm_id && rdma->connected) {
> if ((rdma->errored ||
> @@ -2381,12 +2380,12 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
> g_free(rdma->dest_blocks);
> rdma->dest_blocks = NULL;
>
> - for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> - if (rdma->wr_data[idx].control_mr) {
> + for (int i = 0; i < RDMA_WRID_MAX; i++) {
> + if (rdma->wr_data[i].control_mr) {
> rdma->total_registrations--;
> - ibv_dereg_mr(rdma->wr_data[idx].control_mr);
> + ibv_dereg_mr(rdma->wr_data[i].control_mr);
> }
> - rdma->wr_data[idx].control_mr = NULL;
> + rdma->wr_data[i].control_mr = NULL;
> }
>
> if (rdma->local_ram_blocks.block) {
> @@ -2452,7 +2451,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
>
> static int qemu_rdma_source_init(RDMAContext *rdma, bool pin_all, Error **errp)
> {
> - int ret, idx;
> + int ret;
>
> /*
> * Will be validated against destination's actual capabilities
> @@ -2480,18 +2479,17 @@ static int qemu_rdma_source_init(RDMAContext *rdma, bool pin_all, Error **errp)
>
> /* Build the hash that maps from offset to RAMBlock */
> rdma->blockmap = g_hash_table_new(g_direct_hash, g_direct_equal);
> - for (idx = 0; idx < rdma->local_ram_blocks.nb_blocks; idx++) {
> + for (int i = 0; i < rdma->local_ram_blocks.nb_blocks; i++) {
> g_hash_table_insert(rdma->blockmap,
> - (void *)(uintptr_t)rdma->local_ram_blocks.block[idx].offset,
> - &rdma->local_ram_blocks.block[idx]);
> + (void *)(uintptr_t)rdma->local_ram_blocks.block[i].offset,
> + &rdma->local_ram_blocks.block[i]);
> }
>
> - for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> - ret = qemu_rdma_reg_control(rdma, idx);
> + for (int i = 0; i < RDMA_WRID_MAX; i++) {
> + ret = qemu_rdma_reg_control(rdma, i);
> if (ret < 0) {
> - error_setg(errp,
> - "RDMA ERROR: rdma migration: error registering %d control!",
> - idx);
> + error_setg(errp, "RDMA ERROR: rdma migration: error "
> + "registering %d control!", i);
> goto err_rdma_source_init;
> }
> }
> @@ -2625,16 +2623,16 @@ err_rdma_source_connect:
> static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
> {
> Error *err = NULL;
> - int ret, idx;
> + int ret;
> struct rdma_cm_id *listen_id;
> char ip[40] = "unknown";
> struct rdma_addrinfo *res, *e;
> char port_str[16];
> int reuse = 1;
>
> - for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> - rdma->wr_data[idx].control_len = 0;
> - rdma->wr_data[idx].control_curr = NULL;
> + for (int i = 0; i < RDMA_WRID_MAX; i++) {
> + rdma->wr_data[i].control_len = 0;
> + rdma->wr_data[i].control_curr = NULL;
> }
>
> if (!rdma->host || !rdma->host[0]) {
> @@ -2723,11 +2721,9 @@ err_dest_init_create_listen_id:
> static void qemu_rdma_return_path_dest_init(RDMAContext *rdma_return_path,
> RDMAContext *rdma)
> {
> - int idx;
> -
> - for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> - rdma_return_path->wr_data[idx].control_len = 0;
> - rdma_return_path->wr_data[idx].control_curr = NULL;
> + for (int i = 0; i < RDMA_WRID_MAX; i++) {
> + rdma_return_path->wr_data[i].control_len = 0;
> + rdma_return_path->wr_data[i].control_curr = NULL;
> }
>
> /*the CM channel and CM id is shared*/
> @@ -3377,7 +3373,6 @@ static int qemu_rdma_accept(RDMAContext *rdma)
> struct rdma_cm_event *cm_event;
> struct ibv_context *verbs;
> int ret;
> - int idx;
>
> ret = rdma_get_cm_event(rdma->channel, &cm_event);
> if (ret < 0) {
> @@ -3463,10 +3458,10 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>
> qemu_rdma_init_ram_blocks(rdma);
>
> - for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> - ret = qemu_rdma_reg_control(rdma, idx);
> + for (int i = 0; i < RDMA_WRID_MAX; i++) {
> + ret = qemu_rdma_reg_control(rdma, i);
> if (ret < 0) {
> - error_report("rdma: error registering %d control", idx);
> + error_report("rdma: error registering %d control", i);
> goto err_rdma_dest_wait;
> }
> }
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 12/13] migration/rdma: Declare for index variables local
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (10 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 11/13] migration/rdma: Use i as for index instead of idx Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-12 14:33 ` Fabiano Rosas
2023-10-13 8:17 ` Zhijian Li (Fujitsu)
2023-10-11 20:35 ` [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once Juan Quintela
12 siblings, 2 replies; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Declare all variables that are only used inside a for loop inside the
for statement.
This makes clear that they are not used outside of the for loop.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/rdma.c | 44 ++++++++++++++++++--------------------------
1 file changed, 18 insertions(+), 26 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index e29e5551d1..a43527a83c 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -559,10 +559,8 @@ static void rdma_add_block(RDMAContext *rdma, const char *block_name,
local->block = g_new0(RDMALocalBlock, local->nb_blocks + 1);
if (local->nb_blocks) {
- int x;
-
if (rdma->blockmap) {
- for (x = 0; x < local->nb_blocks; x++) {
+ for (int x = 0; x < local->nb_blocks; x++) {
g_hash_table_remove(rdma->blockmap,
(void *)(uintptr_t)old[x].offset);
g_hash_table_insert(rdma->blockmap,
@@ -649,15 +647,12 @@ static void rdma_delete_block(RDMAContext *rdma, RDMALocalBlock *block)
{
RDMALocalBlocks *local = &rdma->local_ram_blocks;
RDMALocalBlock *old = local->block;
- int x;
if (rdma->blockmap) {
g_hash_table_remove(rdma->blockmap, (void *)(uintptr_t)block->offset);
}
if (block->pmr) {
- int j;
-
- for (j = 0; j < block->nb_chunks; j++) {
+ for (int j = 0; j < block->nb_chunks; j++) {
if (!block->pmr[j]) {
continue;
}
@@ -687,7 +682,7 @@ static void rdma_delete_block(RDMAContext *rdma, RDMALocalBlock *block)
block->block_name = NULL;
if (rdma->blockmap) {
- for (x = 0; x < local->nb_blocks; x++) {
+ for (int x = 0; x < local->nb_blocks; x++) {
g_hash_table_remove(rdma->blockmap,
(void *)(uintptr_t)old[x].offset);
}
@@ -705,7 +700,7 @@ static void rdma_delete_block(RDMAContext *rdma, RDMALocalBlock *block)
memcpy(local->block + block->index, old + (block->index + 1),
sizeof(RDMALocalBlock) *
(local->nb_blocks - (block->index + 1)));
- for (x = block->index; x < local->nb_blocks - 1; x++) {
+ for (int x = block->index; x < local->nb_blocks - 1; x++) {
local->block[x].index--;
}
}
@@ -725,7 +720,7 @@ static void rdma_delete_block(RDMAContext *rdma, RDMALocalBlock *block)
local->nb_blocks--;
if (local->nb_blocks && rdma->blockmap) {
- for (x = 0; x < local->nb_blocks; x++) {
+ for (int x = 0; x < local->nb_blocks; x++) {
g_hash_table_insert(rdma->blockmap,
(void *)(uintptr_t)local->block[x].offset,
&local->block[x]);
@@ -828,12 +823,12 @@ static int qemu_rdma_broken_ipv6_kernel(struct ibv_context *verbs, Error **errp)
* Otherwise, there are no guarantees until the bug is fixed in linux.
*/
if (!verbs) {
- int num_devices, x;
+ int num_devices;
struct ibv_device **dev_list = ibv_get_device_list(&num_devices);
bool roce_found = false;
bool ib_found = false;
- for (x = 0; x < num_devices; x++) {
+ for (int x = 0; x < num_devices; x++) {
verbs = ibv_open_device(dev_list[x]);
/*
* ibv_open_device() is not documented to set errno. If
@@ -925,7 +920,6 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
char port_str[16];
struct rdma_cm_event *cm_event;
char ip[40] = "unknown";
- struct rdma_addrinfo *e;
if (rdma->host == NULL || !strcmp(rdma->host, "")) {
error_setg(errp, "RDMA ERROR: RDMA hostname has not been set");
@@ -957,7 +951,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
}
/* Try all addresses, saving the first error in @err */
- for (e = res; e != NULL; e = e->ai_next) {
+ for (struct rdma_addrinfo *e = res; e != NULL; e = e->ai_next) {
Error **local_errp = err ? NULL : &err;
inet_ntop(e->ai_family,
@@ -2777,7 +2771,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
RDMAContext *rdma;
int ret;
ssize_t done = 0;
- size_t i, len;
+ size_t len;
RCU_READ_LOCK_GUARD();
rdma = qatomic_rcu_read(&rioc->rdmaout);
@@ -2803,7 +2797,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
return -1;
}
- for (i = 0; i < niov; i++) {
+ for (int i = 0; i < niov; i++) {
size_t remaining = iov[i].iov_len;
uint8_t * data = (void *)iov[i].iov_base;
while (remaining) {
@@ -2866,7 +2860,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
RDMAControlHeader head;
int ret;
ssize_t done = 0;
- size_t i, len;
+ size_t len;
RCU_READ_LOCK_GUARD();
rdma = qatomic_rcu_read(&rioc->rdmain);
@@ -2882,7 +2876,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
return -1;
}
- for (i = 0; i < niov; i++) {
+ for (int i = 0; i < niov; i++) {
size_t want = iov[i].iov_len;
uint8_t *data = (void *)iov[i].iov_base;
@@ -3557,8 +3551,6 @@ int rdma_registration_handle(QEMUFile *f)
void *host_addr;
int ret;
int idx = 0;
- int count = 0;
- int i = 0;
if (!migrate_rdma()) {
return 0;
@@ -3629,7 +3621,7 @@ int rdma_registration_handle(QEMUFile *f)
qsort(rdma->local_ram_blocks.block,
rdma->local_ram_blocks.nb_blocks,
sizeof(RDMALocalBlock), dest_ram_sort_func);
- for (i = 0; i < local->nb_blocks; i++) {
+ for (int i = 0; i < local->nb_blocks; i++) {
local->block[i].index = i;
}
@@ -3647,7 +3639,7 @@ int rdma_registration_handle(QEMUFile *f)
* Both sides use the "remote" structure to communicate and update
* their "local" descriptions with what was sent.
*/
- for (i = 0; i < local->nb_blocks; i++) {
+ for (int i = 0; i < local->nb_blocks; i++) {
rdma->dest_blocks[i].remote_host_addr =
(uintptr_t)(local->block[i].local_host_addr);
@@ -3687,7 +3679,7 @@ int rdma_registration_handle(QEMUFile *f)
reg_resp.repeat = head.repeat;
registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
- for (count = 0; count < head.repeat; count++) {
+ for (int count = 0; count < head.repeat; count++) {
uint64_t chunk;
uint8_t *chunk_start, *chunk_end;
@@ -3762,7 +3754,7 @@ int rdma_registration_handle(QEMUFile *f)
unreg_resp.repeat = head.repeat;
registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
- for (count = 0; count < head.repeat; count++) {
+ for (int count = 0; count < head.repeat; count++) {
reg = ®isters[count];
network_to_register(reg);
@@ -3910,7 +3902,7 @@ int rdma_registration_stop(QEMUFile *f, uint64_t flags)
if (flags == RAM_CONTROL_SETUP) {
RDMAControlHeader resp = {.type = RDMA_CONTROL_RAM_BLOCKS_RESULT };
RDMALocalBlocks *local = &rdma->local_ram_blocks;
- int reg_result_idx, i, nb_dest_blocks;
+ int reg_result_idx, nb_dest_blocks;
head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
trace_rdma_registration_stop_ram();
@@ -3958,7 +3950,7 @@ int rdma_registration_stop(QEMUFile *f, uint64_t flags)
qemu_rdma_move_header(rdma, reg_result_idx, &resp);
memcpy(rdma->dest_blocks,
rdma->wr_data[reg_result_idx].control_curr, resp.len);
- for (i = 0; i < nb_dest_blocks; i++) {
+ for (int i = 0; i < nb_dest_blocks; i++) {
network_to_dest_block(&rdma->dest_blocks[i]);
/* We require that the blocks are in the same order */
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 12/13] migration/rdma: Declare for index variables local
2023-10-11 20:35 ` [PATCH v3 12/13] migration/rdma: Declare for index variables local Juan Quintela
@ 2023-10-12 14:33 ` Fabiano Rosas
2023-10-13 8:17 ` Zhijian Li (Fujitsu)
1 sibling, 0 replies; 32+ messages in thread
From: Fabiano Rosas @ 2023-10-12 14:33 UTC (permalink / raw)
To: Juan Quintela, qemu-devel
Cc: Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Juan Quintela <quintela@redhat.com> writes:
> Declare all variables that are only used inside a for loop inside the
> for statement.
>
> This makes clear that they are not used outside of the for loop.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 12/13] migration/rdma: Declare for index variables local
2023-10-11 20:35 ` [PATCH v3 12/13] migration/rdma: Declare for index variables local Juan Quintela
2023-10-12 14:33 ` Fabiano Rosas
@ 2023-10-13 8:17 ` Zhijian Li (Fujitsu)
1 sibling, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:17 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Declare all variables that are only used inside a for loop inside the
> for statement.
>
> This makes clear that they are not used outside of the for loop.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once
2023-10-11 20:35 [PATCH v3 00/13] Removal of QEMUFileHooks Juan Quintela
` (11 preceding siblings ...)
2023-10-11 20:35 ` [PATCH v3 12/13] migration/rdma: Declare for index variables local Juan Quintela
@ 2023-10-11 20:35 ` Juan Quintela
2023-10-12 14:44 ` Fabiano Rosas
2023-10-13 8:19 ` Zhijian Li (Fujitsu)
12 siblings, 2 replies; 32+ messages in thread
From: Juan Quintela @ 2023-10-11 20:35 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Change code that is:
int ret;
...
ret = foo();
if (ret[ < 0]?) {
to:
if (foo()[ < 0]) {
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/rdma.c | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index a43527a83c..c382588b26 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1107,7 +1107,6 @@ err_alloc_pd_cq:
static int qemu_rdma_alloc_qp(RDMAContext *rdma)
{
struct ibv_qp_init_attr attr = { 0 };
- int ret;
attr.cap.max_send_wr = RDMA_SIGNALED_SEND_MAX;
attr.cap.max_recv_wr = 3;
@@ -1117,8 +1116,7 @@ static int qemu_rdma_alloc_qp(RDMAContext *rdma)
attr.recv_cq = rdma->recv_cq;
attr.qp_type = IBV_QPT_RC;
- ret = rdma_create_qp(rdma->cm_id, rdma->pd, &attr);
- if (ret < 0) {
+ if (rdma_create_qp(rdma->cm_id, rdma->pd, &attr) < 0) {
return -1;
}
@@ -1130,8 +1128,8 @@ static int qemu_rdma_alloc_qp(RDMAContext *rdma)
static bool rdma_support_odp(struct ibv_context *dev)
{
struct ibv_device_attr_ex attr = {0};
- int ret = ibv_query_device_ex(dev, NULL, &attr);
- if (ret) {
+
+ if (ibv_query_device_ex(dev, NULL, &attr)) {
return false;
}
@@ -1508,7 +1506,6 @@ static int qemu_rdma_wait_comp_channel(RDMAContext *rdma,
struct ibv_comp_channel *comp_channel)
{
struct rdma_cm_event *cm_event;
- int ret;
/*
* Coroutine doesn't start until migration_fd_process_incoming()
@@ -1544,8 +1541,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext *rdma,
}
if (pfds[1].revents) {
- ret = rdma_get_cm_event(rdma->channel, &cm_event);
- if (ret < 0) {
+ if (rdma_get_cm_event(rdma->channel, &cm_event) < 0) {
return -1;
}
@@ -2317,12 +2313,10 @@ static int qemu_rdma_write(RDMAContext *rdma,
uint64_t current_addr = block_offset + offset;
uint64_t index = rdma->current_index;
uint64_t chunk = rdma->current_chunk;
- int ret;
/* If we cannot merge it, we flush the current buffer first. */
if (!qemu_rdma_buffer_mergeable(rdma, current_addr, len)) {
- ret = qemu_rdma_write_flush(rdma, errp);
- if (ret < 0) {
+ if (qemu_rdma_write_flush(rdma, errp) < 0) {
return -1;
}
rdma->current_length = 0;
@@ -2936,7 +2930,6 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
static int qemu_rdma_drain_cq(RDMAContext *rdma)
{
Error *err = NULL;
- int ret;
if (qemu_rdma_write_flush(rdma, &err) < 0) {
error_report_err(err);
@@ -2944,8 +2937,7 @@ static int qemu_rdma_drain_cq(RDMAContext *rdma)
}
while (rdma->nb_sent) {
- ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
- if (ret < 0) {
+ if (qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL) < 0) {
error_report("rdma migration: complete polling error!");
return -1;
}
@@ -3324,12 +3316,10 @@ static void rdma_accept_incoming_migration(void *opaque);
static void rdma_cm_poll_handler(void *opaque)
{
RDMAContext *rdma = opaque;
- int ret;
struct rdma_cm_event *cm_event;
MigrationIncomingState *mis = migration_incoming_get_current();
- ret = rdma_get_cm_event(rdma->channel, &cm_event);
- if (ret < 0) {
+ if (rdma_get_cm_event(rdma->channel, &cm_event) < 0) {
error_report("get_cm_event failed %d", errno);
return;
}
@@ -4054,14 +4044,11 @@ static QEMUFile *rdma_new_output(RDMAContext *rdma)
static void rdma_accept_incoming_migration(void *opaque)
{
RDMAContext *rdma = opaque;
- int ret;
QEMUFile *f;
Error *local_err = NULL;
trace_qemu_rdma_accept_incoming_migration();
- ret = qemu_rdma_accept(rdma);
-
- if (ret < 0) {
+ if (qemu_rdma_accept(rdma) < 0) {
error_report("RDMA ERROR: Migration initialization failed");
return;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once
2023-10-11 20:35 ` [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once Juan Quintela
@ 2023-10-12 14:44 ` Fabiano Rosas
2023-10-16 6:36 ` Juan Quintela
2023-10-13 8:19 ` Zhijian Li (Fujitsu)
1 sibling, 1 reply; 32+ messages in thread
From: Fabiano Rosas @ 2023-10-12 14:44 UTC (permalink / raw)
To: Juan Quintela, qemu-devel
Cc: Peter Xu, Juan Quintela, Leonardo Bras, Li Zhijian
Juan Quintela <quintela@redhat.com> writes:
> Change code that is:
>
> int ret;
> ...
>
> ret = foo();
> if (ret[ < 0]?) {
>
> to:
>
> if (foo()[ < 0]) {
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> migration/rdma.c | 29 ++++++++---------------------
> 1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index a43527a83c..c382588b26 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1107,7 +1107,6 @@ err_alloc_pd_cq:
> static int qemu_rdma_alloc_qp(RDMAContext *rdma)
> {
> struct ibv_qp_init_attr attr = { 0 };
> - int ret;
>
> attr.cap.max_send_wr = RDMA_SIGNALED_SEND_MAX;
> attr.cap.max_recv_wr = 3;
> @@ -1117,8 +1116,7 @@ static int qemu_rdma_alloc_qp(RDMAContext *rdma)
> attr.recv_cq = rdma->recv_cq;
> attr.qp_type = IBV_QPT_RC;
>
> - ret = rdma_create_qp(rdma->cm_id, rdma->pd, &attr);
> - if (ret < 0) {
> + if (rdma_create_qp(rdma->cm_id, rdma->pd, &attr) < 0) {
This particular pattern hurts readability IMO. See how the < 0 got
pushed all the way to the end of the line. The longer the list of
arguments, the larger the chance of missing the < 0 when glancing over
the code.
Anyway:
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once
2023-10-12 14:44 ` Fabiano Rosas
@ 2023-10-16 6:36 ` Juan Quintela
0 siblings, 0 replies; 32+ messages in thread
From: Juan Quintela @ 2023-10-16 6:36 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Peter Xu, Leonardo Bras, Li Zhijian
Fabiano Rosas <farosas@suse.de> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> Change code that is:
>>
>> int ret;
>> ...
>>
>> ret = foo();
>> if (ret[ < 0]?) {
>>
>> to:
>>
>> if (foo()[ < 0]) {
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> migration/rdma.c | 29 ++++++++---------------------
>> 1 file changed, 8 insertions(+), 21 deletions(-)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index a43527a83c..c382588b26 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -1107,7 +1107,6 @@ err_alloc_pd_cq:
>> static int qemu_rdma_alloc_qp(RDMAContext *rdma)
>> {
>> struct ibv_qp_init_attr attr = { 0 };
>> - int ret;
>>
>> attr.cap.max_send_wr = RDMA_SIGNALED_SEND_MAX;
>> attr.cap.max_recv_wr = 3;
>> @@ -1117,8 +1116,7 @@ static int qemu_rdma_alloc_qp(RDMAContext *rdma)
>> attr.recv_cq = rdma->recv_cq;
>> attr.qp_type = IBV_QPT_RC;
>>
>> - ret = rdma_create_qp(rdma->cm_id, rdma->pd, &attr);
>> - if (ret < 0) {
>> + if (rdma_create_qp(rdma->cm_id, rdma->pd, &attr) < 0) {
>
> This particular pattern hurts readability IMO. See how the < 0 got
> pushed all the way to the end of the line. The longer the list of
> arguments, the larger the chance of missing the < 0 when glancing over
> the code.
You can't have both:
* No "ret" variable that is used only once.
* make the condition line small
I choosed the 1st one. For "ret" I always asume that you set it in one
place and you use it in multiple places. Otherwise you don't need it.
But I can understand the other point of view.
There is a reason that this patch was last O:-)
> Anyway:
>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
Thanks.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once
2023-10-11 20:35 ` [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once Juan Quintela
2023-10-12 14:44 ` Fabiano Rosas
@ 2023-10-13 8:19 ` Zhijian Li (Fujitsu)
1 sibling, 0 replies; 32+ messages in thread
From: Zhijian Li (Fujitsu) @ 2023-10-13 8:19 UTC (permalink / raw)
To: Juan Quintela, qemu-devel@nongnu.org
Cc: Fabiano Rosas, Peter Xu, Leonardo Bras
On 12/10/2023 04:35, Juan Quintela wrote:
> Change code that is:
>
> int ret;
> ...
>
> ret = foo();
> if (ret[ < 0]?) {
>
> to:
>
> if (foo()[ < 0]) {
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
^ permalink raw reply [flat|nested] 32+ messages in thread