qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: mdroth <mdroth@linux.vnet.ibm.com>
To: Stefan Priebe <s.priebe@profihost.ag>
Cc: aliguori@us.ibm.com, Luiz Capitulino <lcapitulino@redhat.com>,
	"qemu-stable@nongnu.org" <qemu-stable@nongnu.org>,
	Dietmar Maurer <dietmar@proxmox.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [Qemu-stable]  qmp commands get rejected
Date: Sun, 26 May 2013 10:36:48 -0500	[thread overview]
Message-ID: <20130526153648.GB4599@vm> (raw)
In-Reply-To: <51A226A0.2080708@profihost.ag>

On Sun, May 26, 2013 at 05:13:36PM +0200, Stefan Priebe wrote:
> Am 26.05.2013 03:23, schrieb mdroth:
> >On Sat, May 25, 2013 at 01:09:50PM +0200, Stefan Priebe wrote:
> >>Am 25.05.2013 00:32, schrieb mdroth:
> >>>On Sat, May 25, 2013 at 12:12:22AM +0200, Stefan Priebe wrote:
> >>>>Am 25.05.2013 00:09, schrieb mdroth:
> >>>>>>>>>I would try to create a small example script.
> >>>>>>>>
> >>>>>>>>I use qmp-shell and other little scripts very often.
> >>>>>>>>
> >>>>>>>>>Am this be due to the fact that I don't wait for the welcome banner
> >>>>>>>>>right now?
> >>>>>>>>
> >>>>>>>>If you're not reading from the socket, then you'll get the banner back
> >>>>>>>>when
> >>>>>>>>you read your first response. But qom-set shouldn't fail because of that.
> >>>>>>
> >>>>>>I can workaround it by adding this patch:
> >>>>>>diff --git a/monitor.c b/monitor.c
> >>>>>>index 62aaebe..9997520 100644
> >>>>>>--- a/monitor.c
> >>>>>>+++ b/monitor.c
> >>>>>>@@ -4239,7 +4239,8 @@ static int monitor_can_read(void *opaque)
> >>>>>>  static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
> >>>>>>  {
> >>>>>>      int is_cap = compare_cmd(cmd_name, "qmp_capabilities");
> >>>>>>-    return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
> >>>>>>+//    return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
> >>>>>>+    return ((is_cap > 0) ? 0 : (qmp_cmd_mode(mon) ? is_cap : !is_cap));
> >>>>>>  }
> >>>>>
> >>>>>I think this is unrelated to your original issue. If you issue
> >>>>>'qmp_capabilities' command more than once you will get CommandNotFound,
> >>>>>and that behavior seems to be present even with v1.3.0. This patch seems
> >>>>>to be masking the problem you're having (which seems to be state from
> >>>>>previous monitor sessions/connections leaking into subsequent ones).
> >>>>
> >>>>That sounds reasonable. I'm using proxmox / PVE which does a lot of
> >>>>qmp queries in the background. So i might see situations where X
> >>>>connections in parallel do qmp queries.
> >>>>
> >>>>>It's possible the GSource-based mechanism for handling I/O for chardev
> >>>>>backends is causing a difference in behavior. Still not sure exactly
> >>>>>what's going on though.
> >>>>Can i revert some patches to test?
> >>>
> >>>I think somewhere prior to this one should be enough to test:
> >>>
> >>>2ea5a7af7bfa576a5936400ccca4144caca9640b
> >>
> >>YES! I used 2ea5a7af7bfa576a5936400ccca4144caca9640b~1 for my tests
> >>and this works absoluty fine.
> >
> >Turns out the real culprit was a few commits later:
> >
> >9f939df955a4152aad69a19a77e0898631bb2c18
> >
> >I've sent a workaround this fixes things for QMP, but we may need a more
> >general fix. Please give it a shot and see if it fixes your issues:
> >
> >http://article.gmane.org/gmane.comp.emulators.qemu/213106
> 
> no i got again:
> The command qom-set has not been found  JSON Reply: {"id":
> "21677:2", "error": {"class": "CommandNotFound", "desc": "The
> command qom-set has not been found"}} JSON Query: {"execute":"qom-set","id":"21677:2","arguments":{"value":2,"path":"machine/peripheral/balloon0","property":"guest-stats-polling-interval"}}
> at /usr/share/perl5/PVE/QMPClient.pm line 101.

Darn, I noticed another potential race after I sent the patch. Hadn't
been able to trigger it on my end, but it might be what you're hitting.

Just sent a v2, can you give that a shot?

http://article.gmane.org/gmane.comp.emulators.qemu/213147

> 
> Stefan
> 

  reply	other threads:[~2013-05-26 15:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-24  5:50 [Qemu-devel] qmp commands get rejected Stefan Priebe
2013-05-24 13:23 ` Luiz Capitulino
2013-05-24 13:57   ` Stefan Priebe - Profihost AG
2013-05-24 14:02     ` Luiz Capitulino
2013-05-24 14:36       ` Stefan Priebe - Profihost AG
2013-05-24 15:21         ` Luiz Capitulino
2013-05-24 20:12           ` Stefan Priebe
2013-05-24 21:37             ` Stefan Priebe
2013-05-24 22:03               ` Stefan Priebe
2013-05-24 22:04               ` Stefan Priebe
2013-05-24 22:09               ` [Qemu-devel] [Qemu-stable] " mdroth
2013-05-24 22:12                 ` Stefan Priebe
2013-05-24 22:32                   ` mdroth
2013-05-25 11:09                     ` Stefan Priebe
2013-05-26  1:23                       ` mdroth
2013-05-26 15:13                         ` Stefan Priebe
2013-05-26 15:36                           ` mdroth [this message]
2013-05-26 20:52                             ` Stefan Priebe

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=20130526153648.GB4599@vm \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=aliguori@us.ibm.com \
    --cc=dietmar@proxmox.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=s.priebe@profihost.ag \
    /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).