All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Eric Blake" <eblake@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Peter Xu" <peterx@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event
Date: Tue, 04 Sep 2018 08:39:27 +0200	[thread overview]
Message-ID: <87y3chydeo.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20180903144116.GH14377@redhat.com> ("Daniel P. =?utf-8?Q?Ber?= =?utf-8?Q?rang=C3=A9=22's?= message of "Mon, 3 Sep 2018 15:41:16 +0100")

Daniel P. Berrangé <berrange@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:
>> 
>> > Example:
>> > 
>> >      client sends in-band command #1
>> >      QEMU reads and queues
>> >      QEMU dequeues in-band command #1
>> >      in-band command #1 starts executing, but it's slooow
>> >      client sends in-band command #2
>> >      QEMU reads and queues
>> >      ...
>> >      client sends in-band command #8
>> >      QEMU reads, queues and suspends the monitor
>> >      client sends out-of-band command
>> > --> time passes...
>> >      in-band command #1 completes, QEMU sends reply
>> >      QEMU dequeues in-band command #2, resumes the monitor
>> >      in-band command #2 starts executing
>> >      QEMU reads and executes out-of-band command
>> >      out-of-band command completes, QEMU sends reply
>> >      in-band command #2 completes, QEMU sends reply
>> >      ... same for remaining in-band commands ...
>> > 
>> > The out-of-band command gets stuck behind the in-band commands.
>> > 
>> > The client can avoid this by managing the flow of in-band commands: have
>> > no more than 7 in flight, so the monitor never gets suspended.
>> > 
>> > This is a potentially useful thing to do for clients, isn't it?
>> > 
>> > Eric, Daniel, is it something libvirt would do?
>> 
>> Right now, libvirt serializes commands - it never sends a QMP command until
>> the previous command's response has been processed. But that may not help
>> much, since libvirt does not send OOB commands.
>
> Note that is not merely due to the QMP monitor restriction either.
>
> Libvirt serializes all its public APIs that can change state of a running
> domain.  It usually aims to allow read-only APIs to be run in parallel with
> APIs that change state.

Makes sense.  State changes are complex enough without concurrency.
Even permitting just concurrent queries can add non-trivial complexity.

However, pipelineing != concurrency.  "Serializing" as I understand it
implies no concurrency, it doesn't imply no pipelining.

Mind, I'm not telling you to pipeline, I'm just trying to understand the
constraints.

> The exception to the rule right now are some of the migration APIs which
> we allow to be invoked to manage the migration process. 

Can you give an example?

>> 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?

> In terms of what libvirt would want to parallelize, I think it is reasonable
> to consider any of the query-XXXX commands desirable. Other stuff is likely
> to remain serialized from libvirt's side.

QEMU doesn't care whether it queues in-band query commands or other
in-band commands.


[*] With a queue length of 1, the monitor suspends on receipt of an
in-band command, and resumes after it completes.  This renders
out-of-band commands useless: there is no way an out-of-band command can
overtake an in-band command.

  parent reply	other threads:[~2018-09-04  6:39 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-03  4:31 [Qemu-devel] [PATCH v7 0/7] monitor: enable OOB by default Peter Xu
2018-09-03  4:31 ` [Qemu-devel] [PATCH v7 1/7] qapi: Fix build_params() for empty parameter list Peter Xu
2018-09-03  4:31 ` [Qemu-devel] [PATCH v7 2/7] qapi: Drop qapi_event_send_FOO()'s Error ** argument Peter Xu
2018-09-03  4:31 ` [Qemu-devel] [PATCH v7 3/7] monitor: suspend monitor instead of send CMD_DROP Peter Xu
2018-09-03  7:38   ` Markus Armbruster
2018-09-03  7:56     ` Markus Armbruster
2018-09-03  9:06     ` Peter Xu
2018-09-03 13:16       ` Markus Armbruster
2018-09-04  3:33         ` Peter Xu
2018-09-04  6:17           ` Markus Armbruster
2018-09-04  7:01             ` Peter Xu
2018-09-03  4:31 ` [Qemu-devel] [PATCH v7 4/7] qapi: remove COMMAND_DROPPED event Peter Xu
2018-09-03  7:49   ` Markus Armbruster
2018-09-03 10:16     ` Peter Xu
2018-09-03 13:31       ` Markus Armbruster
2018-09-03 14:30         ` Eric Blake
2018-09-03 14:41           ` Daniel P. Berrangé
2018-09-04  5:30             ` Peter Xu
2018-09-04  8:04               ` Markus Armbruster
2018-09-05  3:53                 ` Peter Xu
2018-09-04  6:39             ` Markus Armbruster [this message]
2018-09-04  8:23               ` Daniel P. Berrangé
2018-09-04 11:46                 ` Markus Armbruster
2018-09-05 11:45                   ` Dr. David Alan Gilbert
2018-09-03  4:31 ` [Qemu-devel] [PATCH v7 5/7] monitor: remove "x-oob", turn oob on by default Peter Xu
2018-09-03  4:31 ` [Qemu-devel] [PATCH v7 6/7] Revert "tests: Add parameter to qtest_init_without_qmp_handshake" Peter Xu
2018-09-03  4:31 ` [Qemu-devel] [PATCH v7 7/7] tests: add oob functional test for test-qmp-cmds Peter Xu
2018-09-03  5:36 ` [Qemu-devel] [PATCH v7 0/7] monitor: enable OOB by default Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y3chydeo.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.