Re: [PATCH] migration: Remove interface query-migrationthreads

[Fabiano Rosas <farosas@suse.de>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Fri, Oct 11, 2024 at 11:34:17AM -0400, Peter Xu wrote:
>> This reverts two commits:
>> 
>> 671326201dac8fe91222ba0045709f04a8ec3af4
>> 1b1f4ab69c41279a45ccd0d3178e83471e6e4ec1
>> 
>> Meanwhile it adds an entry to removed-features.rst for the
>> query-migrationthreads QMP command.
>> 
>> This patch originates from another patchset [1] that wanted to cleanup the
>> interface and add corresponding HMP command, as lots of things are missing
>> in the query report; so far it only reports the main thread and multifd
>> sender threads; all the rest migration threads are not reported, including
>> multifd recv threads.
>> 
>> As pointed out by Dan in the follow up discussions [1], the API is designed
>> in an awkward way where CPU pinning may not cover the whole lifecycle of
>> even the thread being reported.  When asked, we also didn't get chance to
>> hear from the developer who introduced this feature to explain how this API
>> can be properly used.
>> 
>> OTOH, this feature from debugging POV isn't very helpful either, as all
>> these information can be easily obtained by GDB.  Esepcially, if with
>> "-name $VM,debug-threads=on" we do already have names for each migration
>> threads (which covers more than multifd sender threads).
>> 
>> So it looks like the API isn't helpful in any form as of now, besides it
>> only adds maintenance burden to migration code, even if not much.
>> 
>> Considering that so far there's totally no justification on how to use this
>> interface correctly, let's remove this interface instead of cleaning it up.
>> 
>> In this special case, we even go beyond normal deprecation procedure,
>> because a deprecation process would only make sense when there are existing
>> users. In this specific case, we expect zero serious users with this API.
>
> We have no way of knowing whether there are existing users of this, or
> any other feature in QEMU. This is why we have a formal deprecation
> period, rather than immediately deleting existing features.
>
> Yes, there are plenty of reasons why this feature is sub-optimal, but
> it is not broken to the extent that it is *impossible* for people to
> be using it.
>
> IOW, I don't see that there's anything special here to justify bypassing
> our deprecation process here.

I have no dog in this race, but as a data point, I see that this was
submitted to libvirt as a new migrationpin command:

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/FVNAUEVIMLG6F2VCRKHZDUEOLBJCXQHO/#BVEGJVZMMLQMXE263GO5BSIWUDIYIFZU

>
>
> With regards,
> Daniel