Re: [Qemu-devel] [PATCH v4] scripts/qmp: python3 support for qmp.py

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2715 bytes --]

On Sun, Apr 02, 2017 at 03:09:16PM +0300, Joannah Nanjekye wrote:

This patch causes connect() to hang during negotiation.  Please run the
code before sending patches.

>      def __json_read(self, only_event=False):
>          while True:
> -            data = self.__sockfile.readline()
> -            if not data:
> -                return
> -            resp = json.loads(data)
> -            if 'event' in resp:
> -                if self._debug:
> -                    print >>sys.stderr, "QMP:<<< %s" % resp
> -                self.__events.append(resp)
> -                if not only_event:
> -                    continue
> -            return resp
> -
> +            data =[]
> +            for byte_read in self.__sockfile.read(1):

I think the intention was to read 1 byte at a time but this loop reads
at most 1 byte and no more because read(1) returns 'X' where X is the
byte or '' for end-of-file.

The following code reads more than one byte:

  while True:
      byte_read = self.__sockfile.read(1)
      if not byte_read: # end of file
          break
      ...

> +                data.append(byte_read)
> +                joined_data = ''.join(str(v) for v in data)

In Python 2 joined_data is a string containing the raw UTF-8 bytes.
That works.

In Python 3 joined_data is a string representation of the individual
bytes in the data array.  For example, if the file contains the 'ü'
character encoded as 0xc3 0xbc in UTF-8, then the result is:

  joined_data = "b'\\xc3'b'\\xbc'"

This is unexpected.  Instead we need to decode the array of bytes:

  joined_data = b''.join(a).decode('utf-8')

This produces joined_data = 'ü' in both Python 2 and Python 3.

The b'' byte literal notation is safe to use in QEMU since it was added
in Python 2.6 (the minimum version for QEMU).

> +                if byte_read == '\n':

In Python 3:

  >>> b'\n' == '\n'
  False

Since byte_read is a byte (not a str) in Python 3, this line must use
byte literal notation:

  if byte_read == b'\n':

(This also works in Python 2 where byte is just str.)

> +                    resp = json.loads(json.dumps(joined_data))

Did you mean json.loads(joind_data)?  There is no need to call dumps().

> +                    if 'event' in resp:
> +                        if self._debug:
> +                            print("QMP:<<< %s" % resp, file=sys.stderr) 
> +                        self.__events.append(resp)
> +                        if not only_event:
> +                            continue

The previous line must be discarded before parsing the next one:

  if not only_event:
      data = []
      continue

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]