"Dr. David Alan Gilbert" <dgilb...@redhat.com> writes: > * Markus Armbruster (arm...@redhat.com) wrote: >> Daniel P. Berrangé <berra...@redhat.com> writes: >> >> > On Tue, Sep 04, 2018 at 08:39:27AM +0200, Markus Armbruster wrote: >> >> Daniel P. Berrangé <berra...@redhat.com> writes: >> >> >> >> > On Mon, Sep 03, 2018 at 09:30:52AM -0500, Eric Blake wrote: >> >> >> On 09/03/2018 08:31 AM, Markus Armbruster wrote: > >> >> >> I guess when we are designing what libvirt should do, and deciding >> >> >> WHEN it >> >> >> should send OOB commands, we have the luxury of designing libvirt to >> >> >> enforce >> >> >> how many in-flight in-band commands it will ever have pending at once >> >> >> (whether the current 'at most 1', or even if we make it more parallel >> >> >> to 'at >> >> >> most 7'), so that we can still be ensured that the OOB command will be >> >> >> processed without being stuck in the queue of suspended in-band >> >> >> commands. >> >> >> If we never send more than one in-band at a time, then it's not a >> >> >> concern >> >> >> how deep the qemu queue is; but if we do want libvirt to start parallel >> >> >> in-band commands, then you are right that having a way to learn the >> >> >> qemu >> >> >> queue depth is programmatically more precise than just guessing the >> >> >> maximum >> >> >> depth. But it's also hard to argue we need that complexity if we >> >> >> don't have >> >> >> an immediate use envisioned for it. >> >> >> >> True. >> >> >> >> Options for the initial interface: >> >> >> >> (1) Provide means for the client to determine the queue length limit >> >> (introspection or configuration). Clients that need the monitory to >> >> remain available for out-of-band commands can keep limit - 1 in-band >> >> commands in flight. >> >> >> >> (2) Make the queue length limit part of the documented interface. >> >> Clients that need the monitory to remain available for out-of-band >> >> commands can keep limit - 1 in-band commands in flight. We can >> >> increase the limit later, but not decrease it. We can also switch >> >> to (1) as needed. >> >> >> >> (3) Treat the queue length limit as implementation detail (but tacitly >> >> assume its at least 2, since less makes no sense[*]). Clients that >> >> need the monitory to remain available for out-of-band commands >> >> cannot safely keep more than one in-band command in flight. We can >> >> switch to (2) or (1) as needed. >> >> >> >> Opinions? >> > >> > If you did (3), effectively apps will be behaving as if you had done >> > (2) with a documented queue limit of 2, so why not just do (2) right >> > away. >> >> Yup, that's what I thought, too. >> >> I append a proposed replacement for the patch to qmp-spec.txt. It >> assumes the current queue size 8. That value is of course open to >> debate. > > Could you return that size in the response to qmp_capabilities > at the start of the connection?
Awkward. qmp_capabilities returns version, package and capabilities. The latter fits. Except it's an array, not a struct, and therefore can only say "I can do capability 'oob'", but can't add "and by the way, 'oob' has property 'queue-size': 8". So far, we've used query commands for such stuff. >> diff --git a/docs/interop/qmp-spec.txt b/docs/interop/qmp-spec.txt >> index 67e44a8120..1fc6a507e2 100644 >> --- a/docs/interop/qmp-spec.txt >> +++ b/docs/interop/qmp-spec.txt >> @@ -130,9 +130,11 @@ to pass "id" with out-of-band commands. Passing it >> with all commands >> is recommended for clients that accept capability "oob". >> >> If the client sends in-band commands faster than the server can >> -execute them, the server will stop reading the requests from the QMP >> -channel until the request queue length is reduced to an acceptable >> -range. >> +execute them, the server's queue will fill up, and the server will >> +stop reading commands from the QMP channel until the queue has space >> +again. Clients that need the server to remain responsive to >> +out-of-band commands should avoid filling up the queue by limiting the >> +number of in-band commands in flight to seven. > > If I understand what you're saying then this is a shared limit; i.e. > if you've got two QMP connections then you have to be aware of how many > the other connection is queuing, which is tricky. No, the queue is per monitor. Care to suggest a better wording? > > Dave > >> Only a few commands support out-of-band execution. The ones that do >> have "allow-oob": true in output of query-qmp-schema. > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
