Peter Xu <pet...@redhat.com> writes:
> Per previous discussion [1,2], this patch deprecates query-migrationthreads
> command.
>
> To summarize, the major reason of the deprecation is due to no sensible way
> to consume the API properly:
>
> (1) The reported list of threads are incomplete (ignoring destination
> threads and non-multifd threads).
>
> (2) For CPU pinning, there's no way to properly pin the threads with
> the API if the threads will start running right away after migration
> threads can be queried, so the threads will always run on the default
> cores for a short window.
>
> (3) For VM debugging, one can use "-name $VM,debug-threads=on" instead,
> which will provide proper names for all migration threads.
>
> [1] https://lore.kernel.org/r/20240930195837.825728-1-pet...@redhat.com
> [2] https://lore.kernel.org/r/20241011153417.516715-1-pet...@redhat.com
>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> docs/about/deprecated.rst | 8 ++++++++
> qapi/migration.json | 6 +++++-
> migration/threadinfo.c | 4 ++++
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index ce38a3d0cf..ffb147e896 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -147,6 +147,14 @@ options are removed in favor of using explicit
> ``blockdev-create`` and
> ``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for
> details.
>
> +``query-migrationthreads`` (since 9.2)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +To be removed with no replacement, as it reports only a limited set of
> +threads (for example, it only reports source side of multifd threads,
> +without reporting any destination threads, or non-multifd source threads).
> +For debugging purpose, please use ``-name $VM,debug-threads=on`` instead.
This covers commit message items (1) and (3), but not (2). Observation,
not objection.
> +
> Incorrectly typed ``device_add`` arguments (since 6.2)
> ''''''''''''''''''''''''''''''''''''''''''''''''''''''
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 3af6aa1740..a71a9f0cd3 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -2284,12 +2284,16 @@
> #
> # Returns information of migration threads
> #
> +# Features:
Blank line, please.
> +# @deprecated: This command is deprecated with no replacement yet.
> +#
> # Returns: @MigrationThreadInfo
> #
> # Since: 7.2
> ##
> { 'command': 'query-migrationthreads',
> - 'returns': ['MigrationThreadInfo'] }
> + 'returns': ['MigrationThreadInfo'],
> + 'features': ['deprecated'] }
>
> ##
> # @snapshot-save:
> diff --git a/migration/threadinfo.c b/migration/threadinfo.c
> index 262990dd75..2867413420 100644
> --- a/migration/threadinfo.c
> +++ b/migration/threadinfo.c
> @@ -13,6 +13,7 @@
> #include "qemu/osdep.h"
> #include "qemu/queue.h"
> #include "qemu/lockable.h"
> +#include "qemu/error-report.h"
> #include "threadinfo.h"
>
> QemuMutex migration_threads_lock;
> @@ -52,6 +53,9 @@ MigrationThreadInfoList *qmp_query_migrationthreads(Error
> **errp)
> MigrationThread *thread = NULL;
>
> QEMU_LOCK_GUARD(&migration_threads_lock);
> +
> + warn_report("Command 'query-migrationthreads' is deprecated");
We don't normally do this for QMP commands.
Management applications can use -compat deprecated-input=reject to check
they're not sending deprecated commands or arguments.
Suggest to drop.
> +
> QLIST_FOREACH(thread, &migration_threads, node) {
> MigrationThreadInfo *info = g_new0(MigrationThreadInfo, 1);
> info->name = g_strdup(thread->name);
Acked-by: Markus Armbruster <arm...@redhat.com>
