Our RPC calls can be divided into two groups: regular and high
priority. The latter can be then processed by so called high
priority worker threads. This is our way of defeating a
'deadlock' and allowing some RPCs to be processed even when all
(regular) worker threads are stuck. For instance: if all regular
worker threads get stuck when talking to QEMU on monitor, the
virDomainDestroy() can be processed by a high priority worker
thread(s) and thus unstuck those threads.

Now, this is all fine, except if users want to use virsh
non interactively:

  virsh destroy $dom

This does a bit more - it needs to open a connection. And that
consists of multiple RPC calls: AUTH_LIST,
CONNECT_SUPPORTS_FEATURE, CONNECT_OPEN, and finally
CONNECT_REGISTER_CLOSE_CALLBACK. All of them are marked as high
priority except the last one. Therefore, virsh just sits there
with a partially open connection.

There's one requirement for high priority calls though: they can
not get stuck. Hopefully, the reason is obvious by now. And
looking into the server side implementation the
CONNECT_REGISTER_CLOSE_CALLBACK processing can't ever get stuck.
The only driver that implements the callback for public API is
Parallels (vz). And that can't block really.

And for virConnectUnregisterCloseCallback() it's the same story.

Therefore, both can be marked as high priority.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2143840
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/remote/remote_protocol.x | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 79ffc63f03..7dfb4548f4 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -6416,12 +6416,14 @@ enum remote_procedure {
 
     /**
      * @generate: none
+     * @priority: high
      * @acl: connect:getattr
      */
     REMOTE_PROC_CONNECT_REGISTER_CLOSE_CALLBACK = 360,
 
     /**
      * @generate: none
+     * @priority: high
      * @acl: connect:getattr
      */
     REMOTE_PROC_CONNECT_UNREGISTER_CLOSE_CALLBACK = 361,
-- 
2.37.4

On Wed, Nov 23, 2022 at 10:35:59AM +0100, Michal Privoznik wrote:
> Our RPC calls can be divided into two groups: regular and high
> priority. The latter can be then processed by so called high
> priority worker threads. This is our way of defeating a
> 'deadlock' and allowing some RPCs to be processed even when all
> (regular) worker threads are stuck. For instance: if all regular
> worker threads get stuck when talking to QEMU on monitor, the
> virDomainDestroy() can be processed by a high priority worker
> thread(s) and thus unstuck those threads.
> 
> Now, this is all fine, except if users want to use virsh
> non interactively:
> 
>   virsh destroy $dom
> 
> This does a bit more - it needs to open a connection. And that
> consists of multiple RPC calls: AUTH_LIST,
> CONNECT_SUPPORTS_FEATURE, CONNECT_OPEN, and finally
> CONNECT_REGISTER_CLOSE_CALLBACK. All of them are marked as high
> priority except the last one. Therefore, virsh just sits there
> with a partially open connection.
> 
> There's one requirement for high priority calls though: they can
> not get stuck. Hopefully, the reason is obvious by now. And
> looking into the server side implementation the
> CONNECT_REGISTER_CLOSE_CALLBACK processing can't ever get stuck.
> The only driver that implements the callback for public API is
> Parallels (vz). And that can't block really.
> 
> And for virConnectUnregisterCloseCallback() it's the same story.
> 
> Therefore, both can be marked as high priority.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2143840
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/remote/remote_protocol.x | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@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 :|