The code in threadinfo.c is only used for the QMP command
query-migrationthreads. Make it explicit that this is something
related to QMP.

The current names are also too generic for a piece of code that
doesn't affect the migration directly in any way.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/migration.c  | 4 ++--
 migration/multifd.c    | 4 ++--
 migration/threadinfo.c | 4 ++--
 migration/threadinfo.h | 5 ++---
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index dc05c6f6ea..e731fc98a1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2922,7 +2922,7 @@ static void *migration_thread(void *opaque)
     MigThrError thr_error;
     bool urgent = false;
 
-    thread = MigrationThreadAdd("live_migration", qemu_get_thread_id());
+    thread = qmp_migration_threads_add("live_migration", qemu_get_thread_id());
 
     rcu_register_thread();
 
@@ -3000,7 +3000,7 @@ static void *migration_thread(void *opaque)
     migration_iteration_finish(s);
     object_unref(OBJECT(s));
     rcu_unregister_thread();
-    MigrationThreadDel(thread);
+    qmp_migration_threads_remove(thread);
     return NULL;
 }
 
diff --git a/migration/multifd.c b/migration/multifd.c
index 0bf5958a9c..5ec1ac5c64 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -651,7 +651,7 @@ static void *multifd_send_thread(void *opaque)
     int ret = 0;
     bool use_zero_copy_send = migrate_zero_copy_send();
 
-    thread = MigrationThreadAdd(p->name, qemu_get_thread_id());
+    thread = qmp_migration_threads_add(p->name, qemu_get_thread_id());
 
     trace_multifd_send_thread_start(p->id);
     rcu_register_thread();
@@ -767,7 +767,7 @@ out:
     qemu_mutex_unlock(&p->mutex);
 
     rcu_unregister_thread();
-    MigrationThreadDel(thread);
+    qmp_migration_threads_remove(thread);
     trace_multifd_send_thread_end(p->id, p->num_packets, p->total_normal_pages);
 
     return NULL;
diff --git a/migration/threadinfo.c b/migration/threadinfo.c
index 1de8b31855..c3e85c33e8 100644
--- a/migration/threadinfo.c
+++ b/migration/threadinfo.c
@@ -14,7 +14,7 @@
 
 static QLIST_HEAD(, MigrationThread) migration_threads;
 
-MigrationThread *MigrationThreadAdd(const char *name, int thread_id)
+MigrationThread *qmp_migration_threads_add(const char *name, int thread_id)
 {
     MigrationThread *thread =  g_new0(MigrationThread, 1);
     thread->name = name;
@@ -25,7 +25,7 @@ MigrationThread *MigrationThreadAdd(const char *name, int thread_id)
     return thread;
 }
 
-void MigrationThreadDel(MigrationThread *thread)
+void qmp_migration_threads_remove(MigrationThread *thread)
 {
     if (thread) {
         QLIST_REMOVE(thread, node);
diff --git a/migration/threadinfo.h b/migration/threadinfo.h
index 4d69423c0a..61b990f5e3 100644
--- a/migration/threadinfo.h
+++ b/migration/threadinfo.h
@@ -23,6 +23,5 @@ struct MigrationThread {
     QLIST_ENTRY(MigrationThread) node;
 };
 
-MigrationThread *MigrationThreadAdd(const char *name, int thread_id);
-
-void MigrationThreadDel(MigrationThread *info);
+MigrationThread *qmp_migration_threads_add(const char *name, int thread_id);
+void qmp_migration_threads_remove(MigrationThread *info);
-- 
2.35.3

On 6/6/23 16:45, Fabiano Rosas wrote:
> The code in threadinfo.c is only used for the QMP command
> query-migrationthreads. Make it explicit that this is something
> related to QMP.
> 
> The current names are also too generic for a piece of code that
> doesn't affect the migration directly in any way.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   migration/migration.c  | 4 ++--
>   migration/multifd.c    | 4 ++--
>   migration/threadinfo.c | 4 ++--
>   migration/threadinfo.h | 5 ++---
>   4 files changed, 8 insertions(+), 9 deletions(-)


> -MigrationThread *MigrationThreadAdd(const char *name, int thread_id);
> -
> -void MigrationThreadDel(MigrationThread *info);
> +MigrationThread *qmp_migration_threads_add(const char *name, int thread_id);
> +void qmp_migration_threads_remove(MigrationThread *info);

Dropping 'qmp_' prefix:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Fabiano Rosas <farosas@suse.de> wrote:
> The code in threadinfo.c is only used for the QMP command
> query-migrationthreads. Make it explicit that this is something
> related to QMP.
>
> The current names are also too generic for a piece of code that
> doesn't affect the migration directly in any way.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Don't like the caml case.  And don't really care with/without the qmp_
preffix.  You got it eitherwise.

On Tue, Jun 06, 2023 at 11:45:49AM -0300, Fabiano Rosas wrote:
> The code in threadinfo.c is only used for the QMP command
> query-migrationthreads. Make it explicit that this is something
> related to QMP.
> 
> The current names are also too generic for a piece of code that
> doesn't affect the migration directly in any way.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Looks good here, but shall we reserve the qmp_* prefix to mostly qmp stuff
only?  Dropping "qmp_" in the new names would look better to me..

> ---
>  migration/migration.c  | 4 ++--
>  migration/multifd.c    | 4 ++--
>  migration/threadinfo.c | 4 ++--
>  migration/threadinfo.h | 5 ++---
>  4 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index dc05c6f6ea..e731fc98a1 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2922,7 +2922,7 @@ static void *migration_thread(void *opaque)
>      MigThrError thr_error;
>      bool urgent = false;
>  
> -    thread = MigrationThreadAdd("live_migration", qemu_get_thread_id());
> +    thread = qmp_migration_threads_add("live_migration", qemu_get_thread_id());
>  
>      rcu_register_thread();
>  
> @@ -3000,7 +3000,7 @@ static void *migration_thread(void *opaque)
>      migration_iteration_finish(s);
>      object_unref(OBJECT(s));
>      rcu_unregister_thread();
> -    MigrationThreadDel(thread);
> +    qmp_migration_threads_remove(thread);
>      return NULL;
>  }
>  
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 0bf5958a9c..5ec1ac5c64 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -651,7 +651,7 @@ static void *multifd_send_thread(void *opaque)
>      int ret = 0;
>      bool use_zero_copy_send = migrate_zero_copy_send();
>  
> -    thread = MigrationThreadAdd(p->name, qemu_get_thread_id());
> +    thread = qmp_migration_threads_add(p->name, qemu_get_thread_id());
>  
>      trace_multifd_send_thread_start(p->id);
>      rcu_register_thread();
> @@ -767,7 +767,7 @@ out:
>      qemu_mutex_unlock(&p->mutex);
>  
>      rcu_unregister_thread();
> -    MigrationThreadDel(thread);
> +    qmp_migration_threads_remove(thread);
>      trace_multifd_send_thread_end(p->id, p->num_packets, p->total_normal_pages);
>  
>      return NULL;
> diff --git a/migration/threadinfo.c b/migration/threadinfo.c
> index 1de8b31855..c3e85c33e8 100644
> --- a/migration/threadinfo.c
> +++ b/migration/threadinfo.c
> @@ -14,7 +14,7 @@
>  
>  static QLIST_HEAD(, MigrationThread) migration_threads;
>  
> -MigrationThread *MigrationThreadAdd(const char *name, int thread_id)
> +MigrationThread *qmp_migration_threads_add(const char *name, int thread_id)
>  {
>      MigrationThread *thread =  g_new0(MigrationThread, 1);
>      thread->name = name;
> @@ -25,7 +25,7 @@ MigrationThread *MigrationThreadAdd(const char *name, int thread_id)
>      return thread;
>  }
>  
> -void MigrationThreadDel(MigrationThread *thread)
> +void qmp_migration_threads_remove(MigrationThread *thread)
>  {
>      if (thread) {
>          QLIST_REMOVE(thread, node);
> diff --git a/migration/threadinfo.h b/migration/threadinfo.h
> index 4d69423c0a..61b990f5e3 100644
> --- a/migration/threadinfo.h
> +++ b/migration/threadinfo.h
> @@ -23,6 +23,5 @@ struct MigrationThread {
>      QLIST_ENTRY(MigrationThread) node;
>  };
>  
> -MigrationThread *MigrationThreadAdd(const char *name, int thread_id);
> -
> -void MigrationThreadDel(MigrationThread *info);
> +MigrationThread *qmp_migration_threads_add(const char *name, int thread_id);
> +void qmp_migration_threads_remove(MigrationThread *info);
> -- 
> 2.35.3
> 

-- 
Peter Xu

Peter Xu <peterx@redhat.com> writes:

> On Tue, Jun 06, 2023 at 11:45:49AM -0300, Fabiano Rosas wrote:
>> The code in threadinfo.c is only used for the QMP command
>> query-migrationthreads. Make it explicit that this is something
>> related to QMP.
>> 
>> The current names are also too generic for a piece of code that
>> doesn't affect the migration directly in any way.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Looks good here, but shall we reserve the qmp_* prefix to mostly qmp stuff
> only?  Dropping "qmp_" in the new names would look better to me..
>

Well, we're just putting the thread name and id on a list so that QMP
can use them later. It is nothing "important" enough to have a generic
name like migration_thread.

Perhaps:

thread_info_add
thread_info_remove
thread_info_init
thread_info_cleanup

Anyway, as long as we drop that camel case I'm ok with just removing the
qmp =)

On Tue, Jun 06, 2023 at 04:34:31PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Tue, Jun 06, 2023 at 11:45:49AM -0300, Fabiano Rosas wrote:
> >> The code in threadinfo.c is only used for the QMP command
> >> query-migrationthreads. Make it explicit that this is something
> >> related to QMP.
> >> 
> >> The current names are also too generic for a piece of code that
> >> doesn't affect the migration directly in any way.
> >> 
> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >
> > Looks good here, but shall we reserve the qmp_* prefix to mostly qmp stuff
> > only?  Dropping "qmp_" in the new names would look better to me..
> >
> 
> Well, we're just putting the thread name and id on a list so that QMP
> can use them later. It is nothing "important" enough to have a generic
> name like migration_thread.
> 
> Perhaps:
> 
> thread_info_add
> thread_info_remove
> thread_info_init
> thread_info_cleanup
> 
> Anyway, as long as we drop that camel case I'm ok with just removing the
> qmp =)

Thanks.  To me OTOH it's good as long as "qmp_" dropped. :)

I don't worry on using "migration_thread_" as prefix, that's exactly what
the api does to me.  Or, migration_thread_info_*(), migration_thr_mgr_*(),
etc.

-- 
Peter Xu