[PATCH 0/2] analyze-migration.py: require python 3

[PATCH 0/2] analyze-migration.py: require python 3

[Marc-André Lureau]

Hi,

The following 2 patches fix some error and deprecation warnings with
python 3. It drops usage of numpy and python 2 support.

Marc-André Lureau (2):
  analyze-migration.py: fix find() type error
  analyze-migration.py: replace numpy with python 3.2

 scripts/analyze-migration.py | 39 +++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 18 deletions(-)

-- 
2.24.0

[PATCH 1/2] analyze-migration.py: fix find() type error

[Marc-André Lureau]

Traceback (most recent call last):
  File "../scripts/analyze-migration.py", line 611, in <module>
    dump.read(desc_only = True)
  File "../scripts/analyze-migration.py", line 513, in read
    self.load_vmsd_json(file)
  File "../scripts/analyze-migration.py", line 556, in load_vmsd_json
    vmsd_json = file.read_migration_debug_json()
  File "../scripts/analyze-migration.py", line 89, in read_migration_debug_json
    nulpos = data.rfind("\0")
TypeError: argument should be integer or bytes-like object, not 'str'

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/analyze-migration.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
index e527eb168e..2b835d9b70 100755
--- a/scripts/analyze-migration.py
+++ b/scripts/analyze-migration.py
@@ -86,8 +86,8 @@ class MigrationFile(object):
 
         # Find the last NULL byte, then the first brace after that. This should
         # be the beginning of our JSON data.
-        nulpos = data.rfind("\0")
-        jsonpos = data.find("{", nulpos)
+        nulpos = data.rfind(b'\0')
+        jsonpos = data.find(b'{', nulpos)
 
         # Check backwards from there and see whether we guessed right
         self.file.seek(datapos + jsonpos - 5, 0)
-- 
2.24.0

[PATCH 2/2] analyze-migration.py: replace numpy with python 3.2

[Marc-André Lureau]

Use int.from_bytes() from python 3.2 instead.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/analyze-migration.py | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
index 2b835d9b70..96a31d3974 100755
--- a/scripts/analyze-migration.py
+++ b/scripts/analyze-migration.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 #
 #  Migration Stream Analyzer
 #
@@ -17,12 +17,18 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library; if not, see <http://www.gnu.org/licenses/>.
 
-from __future__ import print_function
-import numpy as np
 import json
 import os
 import argparse
 import collections
+import struct
+import sys
+
+
+MIN_PYTHON = (3, 2)
+if sys.version_info < MIN_PYTHON:
+    sys.exit("Python %s.%s or later is required.\n" % MIN_PYTHON)
+
 
 def mkdir_p(path):
     try:
@@ -30,29 +36,26 @@ def mkdir_p(path):
     except OSError:
         pass
 
+
 class MigrationFile(object):
     def __init__(self, filename):
         self.filename = filename
         self.file = open(self.filename, "rb")
 
     def read64(self):
-        return np.asscalar(np.fromfile(self.file, count=1, dtype='>i8')[0])
+        return int.from_bytes(self.file.read(8), byteorder='big', signed=True)
 
     def read32(self):
-        return np.asscalar(np.fromfile(self.file, count=1, dtype='>i4')[0])
+        return int.from_bytes(self.file.read(4), byteorder='big', signed=True)
 
     def read16(self):
-        return np.asscalar(np.fromfile(self.file, count=1, dtype='>i2')[0])
+        return int.from_bytes(self.file.read(2), byteorder='big', signed=True)
 
     def read8(self):
-        return np.asscalar(np.fromfile(self.file, count=1, dtype='>i1')[0])
+        return int.from_bytes(self.file.read(1), byteorder='big', signed=True)
 
     def readstr(self, len = None):
-        if len is None:
-            len = self.read8()
-        if len == 0:
-            return ""
-        return np.fromfile(self.file, count=1, dtype=('S%d' % len))[0]
+        return self.readvar(len).decode('utf-8')
 
     def readvar(self, size = None):
         if size is None:
@@ -275,7 +278,7 @@ class VMSDFieldGeneric(object):
         return str(self.__str__())
 
     def __str__(self):
-        return " ".join("{0:02x}".format(ord(c)) for c in self.data)
+        return " ".join("{0:02x}".format(c) for c in self.data)
 
     def getDict(self):
         return self.__str__()
@@ -307,8 +310,8 @@ class VMSDFieldInt(VMSDFieldGeneric):
 
     def read(self):
         super(VMSDFieldInt, self).read()
-        self.sdata = np.fromstring(self.data, count=1, dtype=(self.sdtype))[0]
-        self.udata = np.fromstring(self.data, count=1, dtype=(self.udtype))[0]
+        self.sdata = int.from_bytes(self.data, byteorder='big', signed=True)
+        self.udata = int.from_bytes(self.data, byteorder='big', signed=False)
         self.data = self.sdata
         return self.data
 
@@ -363,7 +366,7 @@ class VMSDFieldStruct(VMSDFieldGeneric):
             array_len = field.pop('array_len')
             field['index'] = 0
             new_fields.append(field)
-            for i in xrange(1, array_len):
+            for i in range(1, array_len):
                 c = field.copy()
                 c['index'] = i
                 new_fields.append(c)
-- 
2.24.0

Re: [PATCH 1/2] analyze-migration.py: fix find() type error

[Eduardo Habkost]

On Wed, Nov 27, 2019 at 02:10:37PM +0400, Marc-André Lureau wrote:
> Traceback (most recent call last):
>   File "../scripts/analyze-migration.py", line 611, in <module>
>     dump.read(desc_only = True)
>   File "../scripts/analyze-migration.py", line 513, in read
>     self.load_vmsd_json(file)
>   File "../scripts/analyze-migration.py", line 556, in load_vmsd_json
>     vmsd_json = file.read_migration_debug_json()
>   File "../scripts/analyze-migration.py", line 89, in read_migration_debug_json
>     nulpos = data.rfind("\0")
> TypeError: argument should be integer or bytes-like object, not 'str'
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks!  I'm queueing this, but I'm hoping we don't have a -rc4
and 4.2.0 gets released next week.

-- 
Eduardo

Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2

[Cleber Rosa]

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

On Wed, Nov 27, 2019 at 02:10:38PM +0400, Marc-André Lureau wrote:
> Use int.from_bytes() from python 3.2 instead.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/analyze-migration.py | 35 +++++++++++++++++++----------------
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> index 2b835d9b70..96a31d3974 100755
> --- a/scripts/analyze-migration.py
> +++ b/scripts/analyze-migration.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/env python
> +#!/usr/bin/env python3
>  #
>  #  Migration Stream Analyzer
>  #
> @@ -17,12 +17,18 @@
>  # You should have received a copy of the GNU Lesser General Public
>  # License along with this library; if not, see <http://www.gnu.org/licenses/>.
>  
> -from __future__ import print_function
> -import numpy as np
>  import json
>  import os
>  import argparse
>  import collections
> +import struct
> +import sys
> +
> +
> +MIN_PYTHON = (3, 2)
> +if sys.version_info < MIN_PYTHON:
> +    sys.exit("Python %s.%s or later is required.\n" % MIN_PYTHON)
> +
>
>  def mkdir_p(path):
>      try:
> @@ -30,29 +36,26 @@ def mkdir_p(path):
>      except OSError:
>          pass
>  
> +
>  class MigrationFile(object):
>      def __init__(self, filename):
>          self.filename = filename
>          self.file = open(self.filename, "rb")
>  
>      def read64(self):
> -        return np.asscalar(np.fromfile(self.file, count=1, dtype='>i8')[0])
> +        return int.from_bytes(self.file.read(8), byteorder='big', signed=True)
>
>      def read32(self):
> -        return np.asscalar(np.fromfile(self.file, count=1, dtype='>i4')[0])
> +        return int.from_bytes(self.file.read(4), byteorder='big', signed=True)
>  
>      def read16(self):
> -        return np.asscalar(np.fromfile(self.file, count=1, dtype='>i2')[0])
> +        return int.from_bytes(self.file.read(2), byteorder='big', signed=True)
>  
>      def read8(self):
> -        return np.asscalar(np.fromfile(self.file, count=1, dtype='>i1')[0])
> +        return int.from_bytes(self.file.read(1), byteorder='big', signed=True)
>
>      def readstr(self, len = None):
> -        if len is None:
> -            len = self.read8()
> -        if len == 0:
> -            return ""
> -        return np.fromfile(self.file, count=1, dtype=('S%d' % len))[0]
> +        return self.readvar(len).decode('utf-8')
>  
>      def readvar(self, size = None):
>          if size is None:
> @@ -275,7 +278,7 @@ class VMSDFieldGeneric(object):
>          return str(self.__str__())
>  
>      def __str__(self):
> -        return " ".join("{0:02x}".format(ord(c)) for c in self.data)
> +        return " ".join("{0:02x}".format(c) for c in self.data)
>  
>      def getDict(self):
>          return self.__str__()
> @@ -307,8 +310,8 @@ class VMSDFieldInt(VMSDFieldGeneric):
>  
>      def read(self):
>          super(VMSDFieldInt, self).read()
> -        self.sdata = np.fromstring(self.data, count=1, dtype=(self.sdtype))[0]
> -        self.udata = np.fromstring(self.data, count=1, dtype=(self.udtype))[0]
> +        self.sdata = int.from_bytes(self.data, byteorder='big', signed=True)
> +        self.udata = int.from_bytes(self.data, byteorder='big', signed=False)
>          self.data = self.sdata
>          return self.data
>  
> @@ -363,7 +366,7 @@ class VMSDFieldStruct(VMSDFieldGeneric):
>              array_len = field.pop('array_len')
>              field['index'] = 0
>              new_fields.append(field)
> -            for i in xrange(1, array_len):
> +            for i in range(1, array_len):
>                  c = field.copy()
>                  c['index'] = i
>                  new_fields.append(c)
> -- 
> 2.24.0
> 

Marc-André, I couldn't yet pinpoint the reason yet, but this patch
changes the parsing of bool fields.  This is a diff between the output
pre/post this patch on the same images:

$ diff -u out_x8664_pre out_x8664_post 
--- out_x8664_pre       2019-12-06 09:14:16.128943264 -0500
+++ out_x8664_post      2019-12-06 09:23:35.861378600 -0500
@@ -3039,7 +3039,7 @@
             "mac_reg[RADV]": "0x00000000",
             "mac_reg[TADV]": "0x00000000",
             "mac_reg[ITR]": "0x00000000",
-            "mit_irq_level": true
+            "mit_irq_level": false
         },
         "e1000/full_mac_state": {
             "mac_reg": [
@@ -36010,10 +36010,10 @@
             ],
             "smb_auxctl": "0x02",
             "smb_blkdata": "0x00",
-            "i2c_enable": true,
+            "i2c_enable": false,
             "op_done": true,
-            "in_i2c_block_read": true,
-            "start_transaction_on_status_read": true
+            "in_i2c_block_read": false,
+            "start_transaction_on_status_read": false
         },
         "ar.tmr.timer": "ff ff ff ff ff ff ff ff",
         "ar.tmr.overflow_time": "0x0000000000000000",

This true/false flipping is consistent across various images (tried on
images generated on a few other targets).

- Cleber.

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

Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2

[Eduardo Habkost]

On Fri, Dec 06, 2019 at 09:27:23AM -0500, Cleber Rosa wrote:
> On Wed, Nov 27, 2019 at 02:10:38PM +0400, Marc-André Lureau wrote:
> > Use int.from_bytes() from python 3.2 instead.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  scripts/analyze-migration.py | 35 +++++++++++++++++++----------------
> >  1 file changed, 19 insertions(+), 16 deletions(-)
> > 
> > diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> > index 2b835d9b70..96a31d3974 100755
> > --- a/scripts/analyze-migration.py
> > +++ b/scripts/analyze-migration.py
> > @@ -1,4 +1,4 @@
> > -#!/usr/bin/env python
> > +#!/usr/bin/env python3
[...]
> 
> Marc-André, I couldn't yet pinpoint the reason yet, but this patch
> changes the parsing of bool fields.  This is a diff between the output
> pre/post this patch on the same images:
> 
> $ diff -u out_x8664_pre out_x8664_post 
> --- out_x8664_pre       2019-12-06 09:14:16.128943264 -0500
> +++ out_x8664_post      2019-12-06 09:23:35.861378600 -0500
> @@ -3039,7 +3039,7 @@
>              "mac_reg[RADV]": "0x00000000",
>              "mac_reg[TADV]": "0x00000000",
>              "mac_reg[ITR]": "0x00000000",
> -            "mit_irq_level": true
> +            "mit_irq_level": false
>          },
>          "e1000/full_mac_state": {
>              "mac_reg": [
> @@ -36010,10 +36010,10 @@
>              ],
>              "smb_auxctl": "0x02",
>              "smb_blkdata": "0x00",
> -            "i2c_enable": true,
> +            "i2c_enable": false,
>              "op_done": true,
> -            "in_i2c_block_read": true,
> -            "start_transaction_on_status_read": true
> +            "in_i2c_block_read": false,
> +            "start_transaction_on_status_read": false
>          },
>          "ar.tmr.timer": "ff ff ff ff ff ff ff ff",
>          "ar.tmr.overflow_time": "0x0000000000000000",
> 
> This true/false flipping is consistent across various images (tried on
> images generated on a few other targets).

It looks like moving to python3 accidentally fixes a bug.

This is VMSDFieldBool.read:

    def read(self):
        super(VMSDFieldBool, self).read()
        if self.data[0] == 0:
            self.data = False
        else:
            self.data = True
        return self.data

On python2, MigrationFile.readvar() returned a string, so the
(self.data[0] == 0) check was never true.  This means all boolean
fields were always initialized to True.

On python3, MigrationFile.readvar() returns a bytearray, so the
(self.data[0] == 0) check now works as expected.

-- 
Eduardo

Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2

[Marc-André Lureau]

Hi

On Tue, Dec 10, 2019 at 6:59 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> On Fri, Dec 06, 2019 at 09:27:23AM -0500, Cleber Rosa wrote:
> > On Wed, Nov 27, 2019 at 02:10:38PM +0400, Marc-André Lureau wrote:
> > > Use int.from_bytes() from python 3.2 instead.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  scripts/analyze-migration.py | 35 +++++++++++++++++++----------------
> > >  1 file changed, 19 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> > > index 2b835d9b70..96a31d3974 100755
> > > --- a/scripts/analyze-migration.py
> > > +++ b/scripts/analyze-migration.py
> > > @@ -1,4 +1,4 @@
> > > -#!/usr/bin/env python
> > > +#!/usr/bin/env python3
> [...]
> >
> > Marc-André, I couldn't yet pinpoint the reason yet, but this patch
> > changes the parsing of bool fields.  This is a diff between the output
> > pre/post this patch on the same images:
> >
> > $ diff -u out_x8664_pre out_x8664_post
> > --- out_x8664_pre       2019-12-06 09:14:16.128943264 -0500
> > +++ out_x8664_post      2019-12-06 09:23:35.861378600 -0500
> > @@ -3039,7 +3039,7 @@
> >              "mac_reg[RADV]": "0x00000000",
> >              "mac_reg[TADV]": "0x00000000",
> >              "mac_reg[ITR]": "0x00000000",
> > -            "mit_irq_level": true
> > +            "mit_irq_level": false
> >          },
> >          "e1000/full_mac_state": {
> >              "mac_reg": [
> > @@ -36010,10 +36010,10 @@
> >              ],
> >              "smb_auxctl": "0x02",
> >              "smb_blkdata": "0x00",
> > -            "i2c_enable": true,
> > +            "i2c_enable": false,
> >              "op_done": true,
> > -            "in_i2c_block_read": true,
> > -            "start_transaction_on_status_read": true
> > +            "in_i2c_block_read": false,
> > +            "start_transaction_on_status_read": false
> >          },
> >          "ar.tmr.timer": "ff ff ff ff ff ff ff ff",
> >          "ar.tmr.overflow_time": "0x0000000000000000",
> >
> > This true/false flipping is consistent across various images (tried on
> > images generated on a few other targets).
>
> It looks like moving to python3 accidentally fixes a bug.
>
> This is VMSDFieldBool.read:
>
>     def read(self):
>         super(VMSDFieldBool, self).read()
>         if self.data[0] == 0:
>             self.data = False
>         else:
>             self.data = True
>         return self.data
>
> On python2, MigrationFile.readvar() returned a string, so the
> (self.data[0] == 0) check was never true.  This means all boolean
> fields were always initialized to True.
>
> On python3, MigrationFile.readvar() returns a bytearray, so the
> (self.data[0] == 0) check now works as expected.

Ah! nice surprise. Do you mind updating the commit message on commit?
Or should I resend?

thanks

-- 
Marc-André Lureau

Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2

[Cleber Rosa]

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

On Tue, Dec 10, 2019 at 02:14:18PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Dec 10, 2019 at 6:59 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > On Fri, Dec 06, 2019 at 09:27:23AM -0500, Cleber Rosa wrote:
> > > On Wed, Nov 27, 2019 at 02:10:38PM +0400, Marc-André Lureau wrote:
> > > > Use int.from_bytes() from python 3.2 instead.
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >  scripts/analyze-migration.py | 35 +++++++++++++++++++----------------
> > > >  1 file changed, 19 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> > > > index 2b835d9b70..96a31d3974 100755
> > > > --- a/scripts/analyze-migration.py
> > > > +++ b/scripts/analyze-migration.py
> > > > @@ -1,4 +1,4 @@
> > > > -#!/usr/bin/env python
> > > > +#!/usr/bin/env python3
> > [...]
> > >
> > > Marc-André, I couldn't yet pinpoint the reason yet, but this patch
> > > changes the parsing of bool fields.  This is a diff between the output
> > > pre/post this patch on the same images:
> > >
> > > $ diff -u out_x8664_pre out_x8664_post
> > > --- out_x8664_pre       2019-12-06 09:14:16.128943264 -0500
> > > +++ out_x8664_post      2019-12-06 09:23:35.861378600 -0500
> > > @@ -3039,7 +3039,7 @@
> > >              "mac_reg[RADV]": "0x00000000",
> > >              "mac_reg[TADV]": "0x00000000",
> > >              "mac_reg[ITR]": "0x00000000",
> > > -            "mit_irq_level": true
> > > +            "mit_irq_level": false
> > >          },
> > >          "e1000/full_mac_state": {
> > >              "mac_reg": [
> > > @@ -36010,10 +36010,10 @@
> > >              ],
> > >              "smb_auxctl": "0x02",
> > >              "smb_blkdata": "0x00",
> > > -            "i2c_enable": true,
> > > +            "i2c_enable": false,
> > >              "op_done": true,
> > > -            "in_i2c_block_read": true,
> > > -            "start_transaction_on_status_read": true
> > > +            "in_i2c_block_read": false,
> > > +            "start_transaction_on_status_read": false
> > >          },
> > >          "ar.tmr.timer": "ff ff ff ff ff ff ff ff",
> > >          "ar.tmr.overflow_time": "0x0000000000000000",
> > >
> > > This true/false flipping is consistent across various images (tried on
> > > images generated on a few other targets).
> >
> > It looks like moving to python3 accidentally fixes a bug.
> >
> > This is VMSDFieldBool.read:
> >
> >     def read(self):
> >         super(VMSDFieldBool, self).read()
> >         if self.data[0] == 0:
> >             self.data = False
> >         else:
> >             self.data = True
> >         return self.data
> >
> > On python2, MigrationFile.readvar() returned a string, so the
> > (self.data[0] == 0) check was never true.  This means all boolean
> > fields were always initialized to True.
> >
> > On python3, MigrationFile.readvar() returns a bytearray, so the
> > (self.data[0] == 0) check now works as expected.
> 
> Ah! nice surprise. Do you mind updating the commit message on commit?
> Or should I resend?
> 
> thanks
> 
> -- 
> Marc-André Lureau
> 

Yep, I'm queueing this with an updated commit message.

Eduardo, does your comment imply a "Reviewed-by"?

Cheers,
- Cleber.

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

Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2

[Cleber Rosa]

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

On Tue, Dec 10, 2019 at 12:49:11PM -0500, Cleber Rosa wrote:
> On Tue, Dec 10, 2019 at 02:14:18PM +0400, Marc-André Lureau wrote:
> > Hi
> > 
> > On Tue, Dec 10, 2019 at 6:59 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >
> > > On Fri, Dec 06, 2019 at 09:27:23AM -0500, Cleber Rosa wrote:
> > > > On Wed, Nov 27, 2019 at 02:10:38PM +0400, Marc-André Lureau wrote:
> > > > > Use int.from_bytes() from python 3.2 instead.
> > > > >
> > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > > ---
> > > > >  scripts/analyze-migration.py | 35 +++++++++++++++++++----------------
> > > > >  1 file changed, 19 insertions(+), 16 deletions(-)
> > > > >
> > > > > diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> > > > > index 2b835d9b70..96a31d3974 100755
> > > > > --- a/scripts/analyze-migration.py
> > > > > +++ b/scripts/analyze-migration.py
> > > > > @@ -1,4 +1,4 @@
> > > > > -#!/usr/bin/env python
> > > > > +#!/usr/bin/env python3
> > > [...]
> > > >
> > > > Marc-André, I couldn't yet pinpoint the reason yet, but this patch
> > > > changes the parsing of bool fields.  This is a diff between the output
> > > > pre/post this patch on the same images:
> > > >
> > > > $ diff -u out_x8664_pre out_x8664_post
> > > > --- out_x8664_pre       2019-12-06 09:14:16.128943264 -0500
> > > > +++ out_x8664_post      2019-12-06 09:23:35.861378600 -0500
> > > > @@ -3039,7 +3039,7 @@
> > > >              "mac_reg[RADV]": "0x00000000",
> > > >              "mac_reg[TADV]": "0x00000000",
> > > >              "mac_reg[ITR]": "0x00000000",
> > > > -            "mit_irq_level": true
> > > > +            "mit_irq_level": false
> > > >          },
> > > >          "e1000/full_mac_state": {
> > > >              "mac_reg": [
> > > > @@ -36010,10 +36010,10 @@
> > > >              ],
> > > >              "smb_auxctl": "0x02",
> > > >              "smb_blkdata": "0x00",
> > > > -            "i2c_enable": true,
> > > > +            "i2c_enable": false,
> > > >              "op_done": true,
> > > > -            "in_i2c_block_read": true,
> > > > -            "start_transaction_on_status_read": true
> > > > +            "in_i2c_block_read": false,
> > > > +            "start_transaction_on_status_read": false
> > > >          },
> > > >          "ar.tmr.timer": "ff ff ff ff ff ff ff ff",
> > > >          "ar.tmr.overflow_time": "0x0000000000000000",
> > > >
> > > > This true/false flipping is consistent across various images (tried on
> > > > images generated on a few other targets).
> > >
> > > It looks like moving to python3 accidentally fixes a bug.
> > >
> > > This is VMSDFieldBool.read:
> > >
> > >     def read(self):
> > >         super(VMSDFieldBool, self).read()
> > >         if self.data[0] == 0:
> > >             self.data = False
> > >         else:
> > >             self.data = True
> > >         return self.data
> > >
> > > On python2, MigrationFile.readvar() returned a string, so the
> > > (self.data[0] == 0) check was never true.  This means all boolean
> > > fields were always initialized to True.
> > >
> > > On python3, MigrationFile.readvar() returns a bytearray, so the
> > > (self.data[0] == 0) check now works as expected.
> > 
> > Ah! nice surprise. Do you mind updating the commit message on commit?
> > Or should I resend?
> > 
> > thanks
> > 
> > -- 
> > Marc-André Lureau
> > 
> 
> Yep, I'm queueing this with an updated commit message.
> 
> Eduardo, does your comment imply a "Reviewed-by"?
> 
> Cheers,
> - Cleber.

Eduardo,

I only noticed now that you queued patch 1/2.  Do you want me to queue
that one instead?  Or do you wanto to queue both on this series?

Cheers,
- Cleber.

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

Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2

[Eduardo Habkost]

On Tue, Dec 10, 2019 at 12:57:28PM -0500, Cleber Rosa wrote:
> On Tue, Dec 10, 2019 at 12:49:11PM -0500, Cleber Rosa wrote:
> > On Tue, Dec 10, 2019 at 02:14:18PM +0400, Marc-André Lureau wrote:
> > > Hi
> > > 
> > > On Tue, Dec 10, 2019 at 6:59 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > >
> > > > On Fri, Dec 06, 2019 at 09:27:23AM -0500, Cleber Rosa wrote:
> > > > > On Wed, Nov 27, 2019 at 02:10:38PM +0400, Marc-André Lureau wrote:
> > > > > > Use int.from_bytes() from python 3.2 instead.
> > > > > >
> > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > > > ---
> > > > > >  scripts/analyze-migration.py | 35 +++++++++++++++++++----------------
> > > > > >  1 file changed, 19 insertions(+), 16 deletions(-)
> > > > > >
> > > > > > diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> > > > > > index 2b835d9b70..96a31d3974 100755
> > > > > > --- a/scripts/analyze-migration.py
> > > > > > +++ b/scripts/analyze-migration.py
> > > > > > @@ -1,4 +1,4 @@
> > > > > > -#!/usr/bin/env python
> > > > > > +#!/usr/bin/env python3
> > > > [...]
> > > > >
> > > > > Marc-André, I couldn't yet pinpoint the reason yet, but this patch
> > > > > changes the parsing of bool fields.  This is a diff between the output
> > > > > pre/post this patch on the same images:
> > > > >
> > > > > $ diff -u out_x8664_pre out_x8664_post
> > > > > --- out_x8664_pre       2019-12-06 09:14:16.128943264 -0500
> > > > > +++ out_x8664_post      2019-12-06 09:23:35.861378600 -0500
> > > > > @@ -3039,7 +3039,7 @@
> > > > >              "mac_reg[RADV]": "0x00000000",
> > > > >              "mac_reg[TADV]": "0x00000000",
> > > > >              "mac_reg[ITR]": "0x00000000",
> > > > > -            "mit_irq_level": true
> > > > > +            "mit_irq_level": false
> > > > >          },
> > > > >          "e1000/full_mac_state": {
> > > > >              "mac_reg": [
> > > > > @@ -36010,10 +36010,10 @@
> > > > >              ],
> > > > >              "smb_auxctl": "0x02",
> > > > >              "smb_blkdata": "0x00",
> > > > > -            "i2c_enable": true,
> > > > > +            "i2c_enable": false,
> > > > >              "op_done": true,
> > > > > -            "in_i2c_block_read": true,
> > > > > -            "start_transaction_on_status_read": true
> > > > > +            "in_i2c_block_read": false,
> > > > > +            "start_transaction_on_status_read": false
> > > > >          },
> > > > >          "ar.tmr.timer": "ff ff ff ff ff ff ff ff",
> > > > >          "ar.tmr.overflow_time": "0x0000000000000000",
> > > > >
> > > > > This true/false flipping is consistent across various images (tried on
> > > > > images generated on a few other targets).
> > > >
> > > > It looks like moving to python3 accidentally fixes a bug.
> > > >
> > > > This is VMSDFieldBool.read:
> > > >
> > > >     def read(self):
> > > >         super(VMSDFieldBool, self).read()
> > > >         if self.data[0] == 0:
> > > >             self.data = False
> > > >         else:
> > > >             self.data = True
> > > >         return self.data
> > > >
> > > > On python2, MigrationFile.readvar() returned a string, so the
> > > > (self.data[0] == 0) check was never true.  This means all boolean
> > > > fields were always initialized to True.
> > > >
> > > > On python3, MigrationFile.readvar() returns a bytearray, so the
> > > > (self.data[0] == 0) check now works as expected.
> > > 
> > > Ah! nice surprise. Do you mind updating the commit message on commit?
> > > Or should I resend?
> > > 
> > > thanks
> > > 
> > > -- 
> > > Marc-André Lureau
> > > 
> > 
> > Yep, I'm queueing this with an updated commit message.
> > 
> > Eduardo, does your comment imply a "Reviewed-by"?

Sure!

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


> > 
> > Cheers,
> > - Cleber.
> 
> Eduardo,
> 
> I only noticed now that you queued patch 1/2.  Do you want me to queue
> that one instead?  Or do you wanto to queue both on this series?

If you are planning a new pull request soon, feel free to queue
both, and also to pull all patches from my python-next branch so
it is included in a single pull request.  Thanks!

-- 
Eduardo