Re: [PATCH] migration: Deprecate query-migrationthreads command

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Oct 21, 2024 at 05:52:20PM -0400, Peter Xu wrote:
> 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-peterx@redhat.com
> [2] https://lore.kernel.org/r/20241011153417.516715-1-peterx@redhat.com
> 
> Signed-off-by: Peter Xu <peterx@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)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''

The title underline should be truncated at the ')'

> +
> +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.
> +
>  Incorrectly typed ``device_add`` arguments (since 6.2)
>  ''''''''''''''''''''''''''''''''''''''''''''''''''''''

>  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");

This needs to be warn_report_once, since it is in a codepath that can be
called repeated at runtime.

> +
>      QLIST_FOREACH(thread, &migration_threads, node) {
>          MigrationThreadInfo *info = g_new0(MigrationThreadInfo, 1);
>          info->name = g_strdup(thread->name);

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|