| Message ID | 20190329210804.22121-1-wainersm@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | tests/vm: Python 3, improve image caching, and misc | expand |
Ping. More reviews needed. I've already got Philippe's reviewed-by, thanks! - Wainer On 03/29/2019 06:07 PM, Wainer dos Santos Moschetta wrote: > Hi all! > > This series bundles the support to Python3, improvement to image caching, and miscellaneous changes for the vm-test (`make vm-build-*`). > > Git tree: http://github.com/wainersm/qemu > Branch: vm_test_python3_and_misc > Travis: https://travis-ci.org/wainersm/qemu/builds/513220300 > > Below you can find some general comments. > > Patch 01: > Uses python configured at build, although vm-test can be executed from the code tree (see docs/devel/testing.rst) too. In this case $(PYTHON) will be empty and it picks python pointed by the script's shebang, as a result failing on Python 3 only Linux distros. > > Patch 02: > Ported based.py to work with Python 3 (kept Python 2 compatibility). Fixed the follow errors: > - Raising "TypeError: Unicode-objects must be encoded before hashing" exception by hashlib.sha1(). > - get_default_jobs() is used to set the vm cpus, but it was returning an invalid float number. > - On check_sha256sum(), subprocess.check_output() returns bytes so that string comparison was failing. > > Patch 03: > Implemented a simple mechanism to detect the image file changed on http://download.patchew.org, as discussed in https://www.mail-archive.com/qemu-devel@nongnu.org/msg607839.html. I didn't implement the '--force' option that Paolo suggested, because it would require a major refactoring (technically the vm implementation decides whether use the cache or not). My intention is to keep the script as simple as possible, so IMO, if the image file changed on the server then it should equally be updated the checksum on QEMU's side. > > Patch 04: > Default network (NETWORK=1) backend is enough to have the docker-based tests run properly. Note: `make docker-test-block@centos7` is broken inside the CentOS VM and apparently on my host (Fedora 29 x86_64) too. > > Wainer dos Santos Moschetta (5): > tests/vm: Use python configured on build > tests/vm: Port basevm to Python 3 > tests/vm: Detect the image changed on server > tests/vm: Fix build-centos docker-based tests run > tests/vm: Add missing variables on help > > tests/vm/Makefile.include | 12 ++++++++---- > tests/vm/basevm.py | 39 ++++++++++++++++++++++++++++++++++----- > tests/vm/centos | 6 +++--- > 3 files changed, 45 insertions(+), 12 deletions(-) >
Hello Wainer, (answering because I dislike ignoring emails without giving any feedback:) On 04/23/19 21:28, Wainer dos Santos Moschetta wrote: > Ping. More reviews needed. > > I've already got Philippe's reviewed-by, thanks! I'm going to skip this one. According to "scripts/get_maintainer.pl", an ACK from either Alex or Fam should suffice. (They could also decide to queue your set with Phil's R-b, who's a designated reviewer on this topic.) Thanks Laszlo > On 03/29/2019 06:07 PM, Wainer dos Santos Moschetta wrote: >> Hi all! >> >> This series bundles the support to Python3, improvement to image >> caching, and miscellaneous changes for the vm-test (`make vm-build-*`). >> >> Git tree: http://github.com/wainersm/qemu >> Branch: vm_test_python3_and_misc >> Travis: https://travis-ci.org/wainersm/qemu/builds/513220300 >> >> Below you can find some general comments. >> >> Patch 01: >> Uses python configured at build, although vm-test can be executed from >> the code tree (see docs/devel/testing.rst) too. In this case $(PYTHON) >> will be empty and it picks python pointed by the script's shebang, as >> a result failing on Python 3 only Linux distros. >> >> Patch 02: >> Ported based.py to work with Python 3 (kept Python 2 compatibility). >> Fixed the follow errors: >> - Raising "TypeError: Unicode-objects must be encoded before >> hashing" exception by hashlib.sha1(). >> - get_default_jobs() is used to set the vm cpus, but it was >> returning an invalid float number. >> - On check_sha256sum(), subprocess.check_output() returns bytes so >> that string comparison was failing. >> >> Patch 03: >> Implemented a simple mechanism to detect the image file changed on >> http://download.patchew.org, as discussed in >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg607839.html. I >> didn't implement the '--force' option that Paolo suggested, because it >> would require a major refactoring (technically the vm implementation >> decides whether use the cache or not). My intention is to keep the >> script as simple as possible, so IMO, if the image file changed on the >> server then it should equally be updated the checksum on QEMU's side. >> >> Patch 04: >> Default network (NETWORK=1) backend is enough to have the docker-based >> tests run properly. Note: `make docker-test-block@centos7` is broken >> inside the CentOS VM and apparently on my host (Fedora 29 x86_64) too. >> >> Wainer dos Santos Moschetta (5): >> tests/vm: Use python configured on build >> tests/vm: Port basevm to Python 3 >> tests/vm: Detect the image changed on server >> tests/vm: Fix build-centos docker-based tests run >> tests/vm: Add missing variables on help >> >> tests/vm/Makefile.include | 12 ++++++++---- >> tests/vm/basevm.py | 39 ++++++++++++++++++++++++++++++++++----- >> tests/vm/centos | 6 +++--- >> 3 files changed, 45 insertions(+), 12 deletions(-) >> >
Hi Laszlo, On 04/24/2019 06:07 AM, Laszlo Ersek wrote: > Hello Wainer, > > (answering because I dislike ignoring emails without giving any feedback:) I appreciated that! thanks! > > On 04/23/19 21:28, Wainer dos Santos Moschetta wrote: >> Ping. More reviews needed. >> >> I've already got Philippe's reviewed-by, thanks! > I'm going to skip this one. According to "scripts/get_maintainer.pl", an > ACK from either Alex or Fam should suffice. (They could also decide to > queue your set with Phil's R-b, who's a designated reviewer on this topic.) Yes, that's ok. I'm CC'ing you because the fix on patch 3/5 has origin on a discussion on your series ... [PATCH for-4.1 v3 00/12] bundle edk2 platform firmware with QEMU ... see at https://www.mail-archive.com/qemu-devel@nongnu.org/msg606796.html Sorry, failed to explain the context when I sent this series. Thanks! - Wainer > > Thanks > Laszlo > >> On 03/29/2019 06:07 PM, Wainer dos Santos Moschetta wrote: >>> Hi all! >>> >>> This series bundles the support to Python3, improvement to image >>> caching, and miscellaneous changes for the vm-test (`make vm-build-*`). >>> >>> Git tree: http://github.com/wainersm/qemu >>> Branch: vm_test_python3_and_misc >>> Travis: https://travis-ci.org/wainersm/qemu/builds/513220300 >>> >>> Below you can find some general comments. >>> >>> Patch 01: >>> Uses python configured at build, although vm-test can be executed from >>> the code tree (see docs/devel/testing.rst) too. In this case $(PYTHON) >>> will be empty and it picks python pointed by the script's shebang, as >>> a result failing on Python 3 only Linux distros. >>> >>> Patch 02: >>> Ported based.py to work with Python 3 (kept Python 2 compatibility). >>> Fixed the follow errors: >>> - Raising "TypeError: Unicode-objects must be encoded before >>> hashing" exception by hashlib.sha1(). >>> - get_default_jobs() is used to set the vm cpus, but it was >>> returning an invalid float number. >>> - On check_sha256sum(), subprocess.check_output() returns bytes so >>> that string comparison was failing. >>> >>> Patch 03: >>> Implemented a simple mechanism to detect the image file changed on >>> http://download.patchew.org, as discussed in >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg607839.html. I >>> didn't implement the '--force' option that Paolo suggested, because it >>> would require a major refactoring (technically the vm implementation >>> decides whether use the cache or not). My intention is to keep the >>> script as simple as possible, so IMO, if the image file changed on the >>> server then it should equally be updated the checksum on QEMU's side. >>> >>> Patch 04: >>> Default network (NETWORK=1) backend is enough to have the docker-based >>> tests run properly. Note: `make docker-test-block@centos7` is broken >>> inside the CentOS VM and apparently on my host (Fedora 29 x86_64) too. >>> >>> Wainer dos Santos Moschetta (5): >>> tests/vm: Use python configured on build >>> tests/vm: Port basevm to Python 3 >>> tests/vm: Detect the image changed on server >>> tests/vm: Fix build-centos docker-based tests run >>> tests/vm: Add missing variables on help >>> >>> tests/vm/Makefile.include | 12 ++++++++---- >>> tests/vm/basevm.py | 39 ++++++++++++++++++++++++++++++++++----- >>> tests/vm/centos | 6 +++--- >>> 3 files changed, 45 insertions(+), 12 deletions(-) >>>
Wainer dos Santos Moschetta <wainersm@redhat.com> writes: > Ping. More reviews needed. > > I've already got Philippe's reviewed-by, thanks! I've queued patches 1,2,4,5 to testing/next, thanks. I'm going to see is 3 is obviated by Gerd's VM rebuild work but I've added it for now. Sorry about the delay. -- Alex Bennée