qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: stefanha@redhat.com, marcandre.lureau@gmail.com,
	qemu-devel@nongnu.org, qemu-block@nongnu.org, armbru@redhat.com
Subject: Re: [PATCH v2 0/4] qmp: Optionally run handlers in coroutines
Date: Tue, 14 Jan 2020 18:45:08 +0000	[thread overview]
Message-ID: <20200114184508.GT4071034@redhat.com> (raw)
In-Reply-To: <20200114182735.5553-1-kwolf@redhat.com>

On Tue, Jan 14, 2020 at 07:27:31PM +0100, Kevin Wolf wrote:
> Some QMP command handlers can block the main loop for a relatively long
> time, for example because they perform some I/O. This is quite nasty.
> Allowing such handlers to run in a coroutine where they can yield (and
> therefore release the BQL) while waiting for an event such as I/O
> completion solves the problem.

Am I right that with this approach, there's no functional difference
to QMP from a mgmt app POV ? ie this is purely focused on avoiding
stalls in the main event loop which impact the guest OS and other
parts of QEMU ?

IOW, the response to the QMP command being executed will get sent
back to the mgmt app as normal when the command completes, and the
QMP monitor still serializes execution of commands ?

> This series adds the infrastructure to allow this and switches
> block_resize to run in a coroutine as a first example.
> 
> This is an alternative solution to Marc-André's "monitor: add
> asynchronous command type" series.

Where as this is an actual functional improvement to QMP from
the mgmt app POV in allowing background commands & thus
concurrent execution ?

If this is correct, then the two proposals are somewhat
complementary 

> 
> v2:
> - Fix typo in a commit message [Eric]
> - Use hyphen instead of underscore for the test command [Eric]
> - Mark qmp_block_resize() as coroutine_fn [Stefan]
> 
> 
> Kevin Wolf (4):
>   qapi: Add a 'coroutine' flag for commands
>   vl: Initialise main loop earlier
>   qmp: Move dispatcher to a coroutine
>   block: Mark 'block_resize' as coroutine
> 
>  qapi/block-core.json                    |  3 +-
>  tests/qapi-schema/qapi-schema-test.json |  1 +
>  docs/devel/qapi-code-gen.txt            |  4 ++
>  include/qapi/qmp/dispatch.h             |  3 +
>  monitor/monitor-internal.h              |  5 +-
>  blockdev.c                              |  6 +-
>  monitor/monitor.c                       | 24 ++++---
>  monitor/qmp.c                           | 83 ++++++++++++++++---------
>  qapi/qmp-dispatch.c                     | 38 ++++++++++-
>  tests/test-qmp-cmds.c                   |  4 ++
>  vl.c                                    | 10 +--
>  scripts/qapi/commands.py                | 17 +++--
>  scripts/qapi/doc.py                     |  2 +-
>  scripts/qapi/expr.py                    |  4 +-
>  scripts/qapi/introspect.py              |  2 +-
>  scripts/qapi/schema.py                  |  9 ++-
>  tests/qapi-schema/qapi-schema-test.out  |  2 +
>  tests/qapi-schema/test-qapi.py          |  7 ++-
>  18 files changed, 158 insertions(+), 66 deletions(-)
> 
> -- 
> 2.20.1
> 
> 

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



  parent reply	other threads:[~2020-01-14 18:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-14 18:27 [PATCH v2 0/4] qmp: Optionally run handlers in coroutines Kevin Wolf
2020-01-14 18:27 ` [PATCH v2 1/4] qapi: Add a 'coroutine' flag for commands Kevin Wolf
2020-01-14 18:27 ` [PATCH v2 2/4] vl: Initialise main loop earlier Kevin Wolf
2020-01-14 18:27 ` [PATCH v2 3/4] qmp: Move dispatcher to a coroutine Kevin Wolf
2020-01-14 18:27 ` [PATCH v2 4/4] block: Mark 'block_resize' as coroutine Kevin Wolf
2020-01-14 18:45 ` Daniel P. Berrangé [this message]
2020-01-14 18:58   ` [PATCH v2 0/4] qmp: Optionally run handlers in coroutines Kevin Wolf
2020-01-14 20:41 ` no-reply
2020-01-15 12:24   ` Kevin Wolf
2020-01-15 13:11 ` Stefan Hajnoczi

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=20200114184508.GT4071034@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).