diff mbox

Hack qmp.py to support reading a JSON multi-line response

Message ID 1345471297-21459-1-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé Aug. 20, 2012, 2:01 p.m. UTC
From: "Daniel P. Berrange" <berrange@redhat.com>

The qmp-shell code assumes the JSON response is only on a single
line. If the QEMU monitor is configured in "pretty print" mode
the JSON response can be multi-line. The basic Python JSON APIs
do not appear to support a streaming mode, so the simple hack
here is to try parsing a line, and if it fails, then read another
line, append it, and try parsing again. Keep reading lines until
we can successfully parse

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 QMP/qmp.py | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Luiz Capitulino Aug. 20, 2012, 6:03 p.m. UTC | #1
On Mon, 20 Aug 2012 15:01:37 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> The qmp-shell code assumes the JSON response is only on a single
> line. If the QEMU monitor is configured in "pretty print" mode
> the JSON response can be multi-line. The basic Python JSON APIs
> do not appear to support a streaming mode, so the simple hack
> here is to try parsing a line, and if it fails, then read another
> line, append it, and try parsing again. Keep reading lines until
> we can successfully parse
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  QMP/qmp.py | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/QMP/qmp.py b/QMP/qmp.py
> index 36ecc1d..464a01a 100644
> --- a/QMP/qmp.py
> +++ b/QMP/qmp.py
> @@ -61,10 +61,18 @@ class QEMUMonitorProtocol:
>  
>      def __json_read(self, only_event=False):
>          while True:
> -            data = self.__sockfile.readline()
> -            if not data:
> -                return
> -            resp = json.loads(data)
> +            data = ""
> +            while True:
> +                moredata = self.__sockfile.readline()
> +                if not moredata:
> +                    return
> +                data = data + moredata
> +                try:
> +                    resp = json.loads(data)
> +                    break
> +                except ValueError:
> +                    pass
> +

I'm reluctant about this, because it makes it impossible to detect bad
json from qemu (not that qmp-shell handles this gracefully today).

At the same time I can't think of anything simpler than your hack. What
about calling json.loads() when closing } matches the number of opening ones?
QMP responses are always dictionaries.

>              if 'event' in resp:
>                  self.__events.append(resp)
>                  if not only_event:
Daniel P. Berrangé Aug. 20, 2012, 6:25 p.m. UTC | #2
On Mon, Aug 20, 2012 at 03:03:11PM -0300, Luiz Capitulino wrote:
> On Mon, 20 Aug 2012 15:01:37 +0100
> "Daniel P. Berrange" <berrange@redhat.com> wrote:
> 
> > From: "Daniel P. Berrange" <berrange@redhat.com>
> > 
> > The qmp-shell code assumes the JSON response is only on a single
> > line. If the QEMU monitor is configured in "pretty print" mode
> > the JSON response can be multi-line. The basic Python JSON APIs
> > do not appear to support a streaming mode, so the simple hack
> > here is to try parsing a line, and if it fails, then read another
> > line, append it, and try parsing again. Keep reading lines until
> > we can successfully parse
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  QMP/qmp.py | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/QMP/qmp.py b/QMP/qmp.py
> > index 36ecc1d..464a01a 100644
> > --- a/QMP/qmp.py
> > +++ b/QMP/qmp.py
> > @@ -61,10 +61,18 @@ class QEMUMonitorProtocol:
> >  
> >      def __json_read(self, only_event=False):
> >          while True:
> > -            data = self.__sockfile.readline()
> > -            if not data:
> > -                return
> > -            resp = json.loads(data)
> > +            data = ""
> > +            while True:
> > +                moredata = self.__sockfile.readline()
> > +                if not moredata:
> > +                    return
> > +                data = data + moredata
> > +                try:
> > +                    resp = json.loads(data)
> > +                    break
> > +                except ValueError:
> > +                    pass
> > +
> 
> I'm reluctant about this, because it makes it impossible to detect bad
> json from qemu (not that qmp-shell handles this gracefully today).

Yeah, it is slightly unpleasant.

> At the same time I can't think of anything simpler than your hack. What
> about calling json.loads() when closing } matches the number of opening ones?
> QMP responses are always dictionaries.

How about making multi-line processing an optional feature, by adding
a '-m' flag to qmp-shell. That way we still get  error checking by
default


Daniel
Luiz Capitulino Aug. 20, 2012, 6:28 p.m. UTC | #3
On Mon, 20 Aug 2012 19:25:02 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Mon, Aug 20, 2012 at 03:03:11PM -0300, Luiz Capitulino wrote:
> > On Mon, 20 Aug 2012 15:01:37 +0100
> > "Daniel P. Berrange" <berrange@redhat.com> wrote:
> > 
> > > From: "Daniel P. Berrange" <berrange@redhat.com>
> > > 
> > > The qmp-shell code assumes the JSON response is only on a single
> > > line. If the QEMU monitor is configured in "pretty print" mode
> > > the JSON response can be multi-line. The basic Python JSON APIs
> > > do not appear to support a streaming mode, so the simple hack
> > > here is to try parsing a line, and if it fails, then read another
> > > line, append it, and try parsing again. Keep reading lines until
> > > we can successfully parse
> > > 
> > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > > ---
> > >  QMP/qmp.py | 16 ++++++++++++----
> > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/QMP/qmp.py b/QMP/qmp.py
> > > index 36ecc1d..464a01a 100644
> > > --- a/QMP/qmp.py
> > > +++ b/QMP/qmp.py
> > > @@ -61,10 +61,18 @@ class QEMUMonitorProtocol:
> > >  
> > >      def __json_read(self, only_event=False):
> > >          while True:
> > > -            data = self.__sockfile.readline()
> > > -            if not data:
> > > -                return
> > > -            resp = json.loads(data)
> > > +            data = ""
> > > +            while True:
> > > +                moredata = self.__sockfile.readline()
> > > +                if not moredata:
> > > +                    return
> > > +                data = data + moredata
> > > +                try:
> > > +                    resp = json.loads(data)
> > > +                    break
> > > +                except ValueError:
> > > +                    pass
> > > +
> > 
> > I'm reluctant about this, because it makes it impossible to detect bad
> > json from qemu (not that qmp-shell handles this gracefully today).
> 
> Yeah, it is slightly unpleasant.
> 
> > At the same time I can't think of anything simpler than your hack. What
> > about calling json.loads() when closing } matches the number of opening ones?
> > QMP responses are always dictionaries.
> 
> How about making multi-line processing an optional feature, by adding
> a '-m' flag to qmp-shell. That way we still get  error checking by
> default

Good idea.

> 
> 
> Daniel
diff mbox

Patch

diff --git a/QMP/qmp.py b/QMP/qmp.py
index 36ecc1d..464a01a 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -61,10 +61,18 @@  class QEMUMonitorProtocol:
 
     def __json_read(self, only_event=False):
         while True:
-            data = self.__sockfile.readline()
-            if not data:
-                return
-            resp = json.loads(data)
+            data = ""
+            while True:
+                moredata = self.__sockfile.readline()
+                if not moredata:
+                    return
+                data = data + moredata
+                try:
+                    resp = json.loads(data)
+                    break
+                except ValueError:
+                    pass
+
             if 'event' in resp:
                 self.__events.append(resp)
                 if not only_event: