Re: [Qemu-devel] [PATCH v2] iotests: handle TypeError for Python3 in test 242
On Fri, Feb 22, 2019 at 02:26:13PM +0300, Andrey Shinkevich wrote: > The data type for bytes in Python3 differs from the one in Python2. > Those cases should be managed separately. > > v1: > In the first version, the TypeError in Python3 was handled as the > exception. > Discussed in the e-mail thread with the Message ID: > <1550519997-253534-1-git-send-email-andrey.shinkev...@virtuozzo.com> > > Signed-off-by: Andrey Shinkevich > Reported-by: Kevin Wolf > --- > tests/qemu-iotests/242 | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242 > index 16c65ed..446fbf8 100755 > --- a/tests/qemu-iotests/242 > +++ b/tests/qemu-iotests/242 > @@ -20,6 +20,7 @@ > > import iotests > import json > +import sys > from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \ > file_path, img_info_log, log, filter_qemu_io > > @@ -65,9 +66,12 @@ def toggle_flag(offset): > with open(disk, "r+b") as f: > f.seek(offset, 0) > c = f.read(1) > -toggled = chr(ord(c) ^ bitmap_flag_unknown) > +toggled = ord(c) ^ bitmap_flag_unknown > f.seek(-1, 1) > -f.write(toggled) > +if sys.version_info.major >= 3: > +f.write(bytes([toggled])) > +else: > +f.write(chr(toggled)) Pretending we are dealing with text strings and using str/ord is a python2-specific quirk. I think it would be nice to get rid of it. Python 2 has bytearray(), which behaves more similarly to the bytes type from Python 3. If we use it, we can make the code more python3-like: diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242 index 16c65edcd7..7794fd4a70 100755 --- a/tests/qemu-iotests/242 +++ b/tests/qemu-iotests/242 @@ -64,10 +64,12 @@ def write_to_disk(offset, size): def toggle_flag(offset): with open(disk, "r+b") as f: f.seek(offset, 0) -c = f.read(1) -toggled = chr(ord(c) ^ bitmap_flag_unknown) +# The casts to bytearray() below are only necessary +# for Python 2 compatibility +c = bytearray(f.read(1))[0] +toggled = c ^ bitmap_flag_unknown f.seek(-1, 1) -f.write(toggled) +f.write(bytearray([toggled])) qemu_img_create('-f', iotests.imgfmt, disk, '1M') -- Eduardo
Re: [Qemu-devel] [PATCH v2] iotests: handle TypeError for Python3 in test 242
On 22/02/2019 18:20, Cleber Rosa wrote: > > > On 2/22/19 6:26 AM, Andrey Shinkevich wrote: >> The data type for bytes in Python3 differs from the one in Python2. >> Those cases should be managed separately. >> >> v1: >> In the first version, the TypeError in Python3 was handled as the >> exception. >> Discussed in the e-mail thread with the Message ID: >> <1550519997-253534-1-git-send-email-andrey.shinkev...@virtuozzo.com> >> >> Signed-off-by: Andrey Shinkevich >> Reported-by: Kevin Wolf >> --- >> tests/qemu-iotests/242 | 8 ++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242 >> index 16c65ed..446fbf8 100755 >> --- a/tests/qemu-iotests/242 >> +++ b/tests/qemu-iotests/242 >> @@ -20,6 +20,7 @@ >> >> import iotests >> import json >> +import sys >> from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \ >> file_path, img_info_log, log, filter_qemu_io >> >> @@ -65,9 +66,12 @@ def toggle_flag(offset): >> with open(disk, "r+b") as f: >> f.seek(offset, 0) >> c = f.read(1) >> -toggled = chr(ord(c) ^ bitmap_flag_unknown) >> +toggled = ord(c) ^ bitmap_flag_unknown >> f.seek(-1, 1) >> -f.write(toggled) >> +if sys.version_info.major >= 3: >> +f.write(bytes([toggled])) >> +else: >> +f.write(chr(toggled)) >> > > I originally suggested: > > sys.version_info.major == 2: > ... > > Because this is already present on other tests, and IIRC Max mentioned > using this as an easy to find flag the moment Python 2 support is to be > dropped. But, looking for "sys.version_info.major" is just as > effective, so: > > Reviewed-by: Cleber Rosa > Thank you very much, Cleber. Your review is helpful. >> >> qemu_img_create('-f', iotests.imgfmt, disk, '1M') >> -- With the best regards, Andrey Shinkevich
Re: [Qemu-devel] [PATCH v2] iotests: handle TypeError for Python3 in test 242
On 22/02/2019 17:53, Eric Blake wrote: > On 2/22/19 5:26 AM, Andrey Shinkevich wrote: >> The data type for bytes in Python3 differs from the one in Python2. >> Those cases should be managed separately. >> >> v1: >> In the first version, the TypeError in Python3 was handled as the >> exception. >> Discussed in the e-mail thread with the Message ID: >> <1550519997-253534-1-git-send-email-andrey.shinkev...@virtuozzo.com> > > This paragraph belongs... > >> >> Signed-off-by: Andrey Shinkevich >> Reported-by: Kevin Wolf >> --- > > ...here, after the --- marker. It is useful to reviewers, but does not > need to land in the long-term git history (a year from now, we won't > care if it was v2 or v10 that landed, nor how it changed from the > versions that didn't land). I can clean it up on staging. > > Reviewed-by: Eric Blake > > Will stage through my NBD tree. > Yes, please, Eric. Actually, I had missed to put that paragraph bellow the dashes while composing it as a cover letter. That was my fault. Acknowledged with thanks. I will take your comment into account for future. -- With the best regards, Andrey Shinkevich
Re: [Qemu-devel] [PATCH v2] iotests: handle TypeError for Python3 in test 242
22.02.2019 14:26, Andrey Shinkevich wrote: > The data type for bytes in Python3 differs from the one in Python2. > Those cases should be managed separately. > > v1: > In the first version, the TypeError in Python3 was handled as the > exception. > Discussed in the e-mail thread with the Message ID: > <1550519997-253534-1-git-send-email-andrey.shinkev...@virtuozzo.com> > > Signed-off-by: Andrey Shinkevich > Reported-by: Kevin Wolf > --- > tests/qemu-iotests/242 | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242 > index 16c65ed..446fbf8 100755 > --- a/tests/qemu-iotests/242 > +++ b/tests/qemu-iotests/242 > @@ -20,6 +20,7 @@ > > import iotests > import json > +import sys > from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \ > file_path, img_info_log, log, filter_qemu_io > > @@ -65,9 +66,12 @@ def toggle_flag(offset): > with open(disk, "r+b") as f: > f.seek(offset, 0) > c = f.read(1) > -toggled = chr(ord(c) ^ bitmap_flag_unknown) > +toggled = ord(c) ^ bitmap_flag_unknown > f.seek(-1, 1) > -f.write(toggled) > +if sys.version_info.major >= 3: > +f.write(bytes([toggled])) > +else: > +f.write(chr(toggled)) > > > qemu_img_create('-f', iotests.imgfmt, disk, '1M') > Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH v2] iotests: handle TypeError for Python3 in test 242
On 2/22/19 5:26 AM, Andrey Shinkevich wrote: > The data type for bytes in Python3 differs from the one in Python2. > Those cases should be managed separately. > > v1: > In the first version, the TypeError in Python3 was handled as the > exception. > Discussed in the e-mail thread with the Message ID: > <1550519997-253534-1-git-send-email-andrey.shinkev...@virtuozzo.com> This paragraph belongs... > > Signed-off-by: Andrey Shinkevich > Reported-by: Kevin Wolf > --- ...here, after the --- marker. It is useful to reviewers, but does not need to land in the long-term git history (a year from now, we won't care if it was v2 or v10 that landed, nor how it changed from the versions that didn't land). I can clean it up on staging. Reviewed-by: Eric Blake Will stage through my NBD tree. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2] iotests: handle TypeError for Python3 in test 242
On 2/22/19 6:26 AM, Andrey Shinkevich wrote: > The data type for bytes in Python3 differs from the one in Python2. > Those cases should be managed separately. > > v1: > In the first version, the TypeError in Python3 was handled as the > exception. > Discussed in the e-mail thread with the Message ID: > <1550519997-253534-1-git-send-email-andrey.shinkev...@virtuozzo.com> > > Signed-off-by: Andrey Shinkevich > Reported-by: Kevin Wolf > --- > tests/qemu-iotests/242 | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242 > index 16c65ed..446fbf8 100755 > --- a/tests/qemu-iotests/242 > +++ b/tests/qemu-iotests/242 > @@ -20,6 +20,7 @@ > > import iotests > import json > +import sys > from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \ > file_path, img_info_log, log, filter_qemu_io > > @@ -65,9 +66,12 @@ def toggle_flag(offset): > with open(disk, "r+b") as f: > f.seek(offset, 0) > c = f.read(1) > -toggled = chr(ord(c) ^ bitmap_flag_unknown) > +toggled = ord(c) ^ bitmap_flag_unknown > f.seek(-1, 1) > -f.write(toggled) > +if sys.version_info.major >= 3: > +f.write(bytes([toggled])) > +else: > +f.write(chr(toggled)) > I originally suggested: sys.version_info.major == 2: ... Because this is already present on other tests, and IIRC Max mentioned using this as an easy to find flag the moment Python 2 support is to be dropped. But, looking for "sys.version_info.major" is just as effective, so: Reviewed-by: Cleber Rosa > > qemu_img_create('-f', iotests.imgfmt, disk, '1M') >