* [PATCH 0/2] analyze-migration.py: require python 3 @ 2019-11-27 10:10 Marc-André Lureau 2019-11-27 10:10 ` [PATCH 1/2] analyze-migration.py: fix find() type error Marc-André Lureau 2019-11-27 10:10 ` [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 Marc-André Lureau 0 siblings, 2 replies; 10+ messages in thread From: Marc-André Lureau @ 2019-11-27 10:10 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, Eduardo Habkost, Cleber Rosa 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] analyze-migration.py: fix find() type error 2019-11-27 10:10 [PATCH 0/2] analyze-migration.py: require python 3 Marc-André Lureau @ 2019-11-27 10:10 ` Marc-André Lureau 2019-11-27 23:37 ` Eduardo Habkost 2019-11-27 10:10 ` [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 Marc-André Lureau 1 sibling, 1 reply; 10+ messages in thread From: Marc-André Lureau @ 2019-11-27 10:10 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, Eduardo Habkost, Cleber Rosa 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] analyze-migration.py: fix find() type error 2019-11-27 10:10 ` [PATCH 1/2] analyze-migration.py: fix find() type error Marc-André Lureau @ 2019-11-27 23:37 ` Eduardo Habkost 0 siblings, 0 replies; 10+ messages in thread From: Eduardo Habkost @ 2019-11-27 23:37 UTC (permalink / raw) To: Marc-André Lureau; +Cc: qemu-devel, Cleber Rosa 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 2019-11-27 10:10 [PATCH 0/2] analyze-migration.py: require python 3 Marc-André Lureau 2019-11-27 10:10 ` [PATCH 1/2] analyze-migration.py: fix find() type error Marc-André Lureau @ 2019-11-27 10:10 ` Marc-André Lureau 2019-12-06 14:27 ` Cleber Rosa 1 sibling, 1 reply; 10+ messages in thread From: Marc-André Lureau @ 2019-11-27 10:10 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, Eduardo Habkost, Cleber Rosa 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 2019-11-27 10:10 ` [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 Marc-André Lureau @ 2019-12-06 14:27 ` Cleber Rosa 2019-12-10 2:58 ` Eduardo Habkost 0 siblings, 1 reply; 10+ messages in thread From: Cleber Rosa @ 2019-12-06 14:27 UTC (permalink / raw) To: Marc-André Lureau; +Cc: qemu-devel, Eduardo Habkost [-- 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 --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 2019-12-06 14:27 ` Cleber Rosa @ 2019-12-10 2:58 ` Eduardo Habkost 2019-12-10 10:14 ` Marc-André Lureau 0 siblings, 1 reply; 10+ messages in thread From: Eduardo Habkost @ 2019-12-10 2:58 UTC (permalink / raw) To: Cleber Rosa; +Cc: Marc-André Lureau, qemu-devel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 2019-12-10 2:58 ` Eduardo Habkost @ 2019-12-10 10:14 ` Marc-André Lureau 2019-12-10 17:49 ` Cleber Rosa 0 siblings, 1 reply; 10+ messages in thread From: Marc-André Lureau @ 2019-12-10 10:14 UTC (permalink / raw) To: Eduardo Habkost; +Cc: QEMU, Cleber Rosa 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 2019-12-10 10:14 ` Marc-André Lureau @ 2019-12-10 17:49 ` Cleber Rosa 2019-12-10 17:57 ` Cleber Rosa 0 siblings, 1 reply; 10+ messages in thread From: Cleber Rosa @ 2019-12-10 17:49 UTC (permalink / raw) To: Marc-André Lureau; +Cc: Eduardo Habkost, QEMU [-- 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 --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 2019-12-10 17:49 ` Cleber Rosa @ 2019-12-10 17:57 ` Cleber Rosa 2019-12-10 19:26 ` Eduardo Habkost 0 siblings, 1 reply; 10+ messages in thread From: Cleber Rosa @ 2019-12-10 17:57 UTC (permalink / raw) To: Marc-André Lureau; +Cc: Eduardo Habkost, QEMU [-- 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 --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 2019-12-10 17:57 ` Cleber Rosa @ 2019-12-10 19:26 ` Eduardo Habkost 0 siblings, 0 replies; 10+ messages in thread From: Eduardo Habkost @ 2019-12-10 19:26 UTC (permalink / raw) To: Cleber Rosa; +Cc: Marc-André Lureau, QEMU 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-10 19:27 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-27 10:10 [PATCH 0/2] analyze-migration.py: require python 3 Marc-André Lureau 2019-11-27 10:10 ` [PATCH 1/2] analyze-migration.py: fix find() type error Marc-André Lureau 2019-11-27 23:37 ` Eduardo Habkost 2019-11-27 10:10 ` [PATCH 2/2] analyze-migration.py: replace numpy with python 3.2 Marc-André Lureau 2019-12-06 14:27 ` Cleber Rosa 2019-12-10 2:58 ` Eduardo Habkost 2019-12-10 10:14 ` Marc-André Lureau 2019-12-10 17:49 ` Cleber Rosa 2019-12-10 17:57 ` Cleber Rosa 2019-12-10 19:26 ` Eduardo Habkost
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).