On Tue, Oct 22, 2024 at 10:41:29AM +0200, Markus Armbruster wrote:
> 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.
They could, but in practice I don't believe anything is doing this, so
the warning message is a practical way to alert people to the usage.
>
> > +
> > 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>
>
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 :|
