[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event |
Date: |
Wed, 19 Sep 2018 16:05:13 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
* Markus Armbruster (address@hidden) wrote:
> "Dr. David Alan Gilbert" <address@hidden> writes:
>
> > * Markus Armbruster (address@hidden) wrote:
> >> Daniel P. Berrangé <address@hidden> writes:
> >>
> >> > On Tue, Sep 04, 2018 at 08:39:27AM +0200, Markus Armbruster wrote:
> >> >> Daniel P. Berrangé <address@hidden> 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 it could add to version,package, capabilities a 'values' array?
> 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?
Oh OK, that's a lot less scary; I suggest appending a 'Each QMP channel
has it's own independent queue.'
Dave
> >
> > 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 / address@hidden / Manchester, UK
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, (continued)
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Eric Blake, 2018/09/03
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Daniel P . Berrangé, 2018/09/03
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Peter Xu, 2018/09/04
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Markus Armbruster, 2018/09/04
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Peter Xu, 2018/09/04
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Markus Armbruster, 2018/09/04
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Daniel P . Berrangé, 2018/09/04
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Markus Armbruster, 2018/09/04
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Dr. David Alan Gilbert, 2018/09/05
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event, Markus Armbruster, 2018/09/19
- Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event,
Dr. David Alan Gilbert <=
[Qemu-devel] [PATCH v7 5/7] monitor: remove "x-oob", turn oob on by default, Peter Xu, 2018/09/03
[Qemu-devel] [PATCH v7 6/7] Revert "tests: Add parameter to qtest_init_without_qmp_handshake", Peter Xu, 2018/09/03
[Qemu-devel] [PATCH v7 7/7] tests: add oob functional test for test-qmp-cmds, Peter Xu, 2018/09/03
Re: [Qemu-devel] [PATCH v7 0/7] monitor: enable OOB by default, Markus Armbruster, 2018/09/03