All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 4/6] monitor: check if chardev can switch gcontext for OOB
Date: Wed, 10 Oct 2018 12:37:00 +0800	[thread overview]
Message-ID: <20181010043700.GO18728@xz-x1> (raw)
In-Reply-To: <20181009131251.721-5-marcandre.lureau@redhat.com>

On Tue, Oct 09, 2018 at 05:12:49PM +0400, Marc-André Lureau wrote:
> Note: this patch will conflict with Peter "[PATCH v9 3/6] monitor:
> remove "x-oob", turn oob on by default", but can be trivially updated.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  monitor.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index a25514490a..c175cf6f0d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4550,9 +4550,10 @@ void monitor_init(Chardev *chr, int flags)
>      bool use_oob = flags & MONITOR_USE_OOB;
>  
>      if (use_oob) {
> -        if (CHARDEV_IS_MUX(chr)) {
> +        if (!qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)) {
>              error_report("Monitor out-of-band is not supported with "
> -                         "MUX typed chardev backend");
> +                         "%s typed chardev backend",
> +                         object_get_typename(OBJECT(chr)));

This seems a bit confusing to me at the first glance since we forbid
mux because not all frontends are ready to run outside main loop (and
now we have mon_iothread so it'll be odd too to run anything
non-monitor on that too...), rather than whether the backend can
dynamically switch its context.

I'm not sure, but do you mean you want to disable oob for backends
like spice or braille?  I just noticed that it seems even legal if we
pipe a qmp monitor with a windows mouse...

I believe in all cases the commit message can be enhanced on
explaining "why" of this patch. :)

Regards,

-- 
Peter Xu

  reply	other threads:[~2018-10-10  4:37 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-09 13:12 [Qemu-devel] [PATCH 0/6] monitor: misc fixes Marc-André Lureau
2018-10-09 13:12 ` [Qemu-devel] [PATCH 1/6] monitor: inline ambiguous helper functions Marc-André Lureau
2018-10-09 13:12 ` [Qemu-devel] [PATCH 2/6] monitor: accept chardev input from iothread Marc-André Lureau
2018-10-10  3:45   ` Peter Xu
2018-10-29 11:34     ` Marc-André Lureau
2018-10-09 13:12 ` [Qemu-devel] [PATCH 3/6] char: add a QEMU_CHAR_FEATURE_GCONTEXT flag Marc-André Lureau
2018-10-10  3:54   ` Peter Xu
2018-10-29 11:45     ` Marc-André Lureau
2018-10-09 13:12 ` [Qemu-devel] [PATCH 4/6] monitor: check if chardev can switch gcontext for OOB Marc-André Lureau
2018-10-10  4:37   ` Peter Xu [this message]
2018-10-29 12:06     ` Marc-André Lureau
2018-10-09 13:12 ` [Qemu-devel] [PATCH 5/6] monitor: prevent inserting new monitors after cleanup Marc-André Lureau
2018-10-09 13:12 ` [Qemu-devel] [PATCH 6/6] monitor: avoid potential dead-lock when cleaning up Marc-André Lureau

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=20181010043700.GO18728@xz-x1 \
    --to=peterx@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@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.