Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
On 9/18/19 9:05 AM, Thomas Huth wrote: On 18/09/2019 00.29, John Snow wrote: On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: [...] Finally do we support something except linux for iotests? for bash tests _supported_os also used only with "Linux" in 87 tests.. The iotests in the "auto" group are supposed to work on other OSes beside Linux, too, since they are run automatically during "make check" now. You can use github with cirrus-ci to check FreeBSD and macOS, see e.g.: https://cirrus-ci.com/build/5114679677943808 Travis has support for macOS, too. And to test them on OpenBSD (or FreeBSD), you can use the VM tests, e.g. something like this: make vm-build-openbsd J=8 BUILD_TARGET=check-block \ EXTRA_CONFIGURE_OPTS=--target-list=x86_64-softmmu aaand lastly, running `make check` doesn't happen to call any of the tests that appear broken on FreeBSD right now, so I'm just going to go ahead and say we can open Pandora's box and make the default python test behavior to run on any OS, and start re-blacklisting the edge-cases as we find them. Sounds good. If it breaks on FreeBSD or macOS, we'll find out with cirrus-ci or Travis pretty soon. Yeah. It's annoying, but genuinely the quickest way to figure it out. Keep an eye on the v5 of this series for fallout. --js
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
18.09.2019 13:30, Vladimir Sementsov-Ogievskiy wrote: > 18.09.2019 1:29, John Snow wrote: >> >> >> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: >>> 12.09.2019 3:16, John Snow wrote: Like script_main, but doesn't require a single point of entry. Replace all existing initialization sections with this drop-in replacement. This brings debug support to all existing script-style iotests. Note: supported_oses=['linux'] was omitted, as it is a default argument. >>> >>> But after this patch all test which didn't check os start to check linux >>> (as it's default).. So all tests which worked on other platforms will now >>> be skipped on these other platforms? >>> >>> Finally do we support something except linux for iotests? >>> for bash tests _supported_os also used only with "Linux" in 87 tests.. >>> >>> May be we'd better drop both _supported_os and supported_oses alltogether, >>> and don't care? >>> >>> Anyway, if we support only linux, any reason to skip almost all tests, >>> if someone tries to run test on other platform? Let him run what he wants. >>> >> >> Currently, the following tests are python: >> >> 030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147 >> 148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209 >> 210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246 >> 248 254 255 256 257 258 262 266 >> >> And as it stands, none of the script-style python tests we've selected >> to run in `make check` fail on the FreeBSD platform. >> >> So as an experiment, I lifted the restriction on python tests. I kept >> running ./check until I found some invocation that they didn't skip. >> >> Failures: 045 147 149 169 194 199 211 >> >> Not too bad... >> >> 045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist >> 149: Wants to use 'sudo', but our freebsd image doesn't have that. >> 194: +OSError: AF_UNIX path too long >> 211: >> -[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true, >> "offset": 1024}, >> -{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data": >> true, "offset": 4096}] >> +[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data": >> true, "offset": 1024}, >> +{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data": >> true, "offset": 32768}] >> >> >> 149 can probably be fixed, and 194 and 211 I have fail in similar ways >> on my own Linux machine, so that's probably not BSD's fault. >> >> Interestingly, 169 and 199, bitmap migration related tests, cause a >> SIGSEGV in QEMU ... >> >> >> 169: >> + >> +WARNING:qemu.machine:qemu received signal 6: >> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 >> -chardev >> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock >> -mon chardev=mon,mode=control -display none -vga none -qtest >> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock >> -machine accel=qtest -nodefaults -display none -machine accel=qtest >> -incoming defer -drive >> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback >> >> The common thread in 169 is the +migbitmap trait, which ... makes me a >> little nervous, of course! >> >> >> 199: >> +WARNING:qemu.machine:qemu received signal 6: >> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 >> -chardev >> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock >> -mon chardev=mon,mode=control -display none -vga none -qtest >> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock >> -machine accel=qtest -nodefaults -display none -machine accel=qtest >> -drive >> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none >> -incoming exec: cat >> '/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo' >> +E >> >> >> Vladimir, I was able to provoke this error by editing >> ./tests/vm/Makefile.include and removing the --snapshot invocation, then >> using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd` >> to open up a shell. >> >> Then the tricks are the usual ones; navigate to iotests directory, >> ./check -v -qcow2 169, etc. You'll need to create a build that allows >> Python tests to run; do it before you run the snapshot-less build. >> >> > > Interesting, I'll try to reproduce. Could you provide more detailed steps? # make vm-build-freebsd VM-IMAGE freebsd ### Downloading install iso ... ### Preparing iso and disk image ... /root/.cache/qemu-vm/images/freebsd.img.install.iso.xz (1/1) 100 % 595,0 MiB / 851,1 MiB = 0,699 153 MiB/s 0:05 Failed to prepare
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
On 9/18/19 12:44 PM, Vladimir Sementsov-Ogievskiy wrote: > 18.09.2019 13:30, Vladimir Sementsov-Ogievskiy wrote: >> 18.09.2019 1:29, John Snow wrote: >>> >>> >>> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: 12.09.2019 3:16, John Snow wrote: > Like script_main, but doesn't require a single point of entry. > Replace all existing initialization sections with this drop-in > replacement. > > This brings debug support to all existing script-style iotests. > > Note: supported_oses=['linux'] was omitted, as it is a default argument. But after this patch all test which didn't check os start to check linux (as it's default).. So all tests which worked on other platforms will now be skipped on these other platforms? Finally do we support something except linux for iotests? for bash tests _supported_os also used only with "Linux" in 87 tests.. May be we'd better drop both _supported_os and supported_oses alltogether, and don't care? Anyway, if we support only linux, any reason to skip almost all tests, if someone tries to run test on other platform? Let him run what he wants. >>> >>> Currently, the following tests are python: >>> >>> 030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147 >>> 148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209 >>> 210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246 >>> 248 254 255 256 257 258 262 266 >>> >>> And as it stands, none of the script-style python tests we've selected >>> to run in `make check` fail on the FreeBSD platform. >>> >>> So as an experiment, I lifted the restriction on python tests. I kept >>> running ./check until I found some invocation that they didn't skip. >>> >>> Failures: 045 147 149 169 194 199 211 >>> >>> Not too bad... >>> >>> 045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist >>> 149: Wants to use 'sudo', but our freebsd image doesn't have that. >>> 194: +OSError: AF_UNIX path too long >>> 211: >>> -[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true, >>> "offset": 1024}, >>> -{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data": >>> true, "offset": 4096}] >>> +[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data": >>> true, "offset": 1024}, >>> +{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data": >>> true, "offset": 32768}] >>> >>> >>> 149 can probably be fixed, and 194 and 211 I have fail in similar ways >>> on my own Linux machine, so that's probably not BSD's fault. >>> >>> Interestingly, 169 and 199, bitmap migration related tests, cause a >>> SIGSEGV in QEMU ... >>> >>> >>> 169: >>> + >>> +WARNING:qemu.machine:qemu received signal 6: >>> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 >>> -chardev >>> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock >>> -mon chardev=mon,mode=control -display none -vga none -qtest >>> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock >>> -machine accel=qtest -nodefaults -display none -machine accel=qtest >>> -incoming defer -drive >>> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback >>> >>> The common thread in 169 is the +migbitmap trait, which ... makes me a >>> little nervous, of course! >>> >>> >>> 199: >>> +WARNING:qemu.machine:qemu received signal 6: >>> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 >>> -chardev >>> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock >>> -mon chardev=mon,mode=control -display none -vga none -qtest >>> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock >>> -machine accel=qtest -nodefaults -display none -machine accel=qtest >>> -drive >>> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none >>> -incoming exec: cat >>> '/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo' >>> +E >>> >>> >>> Vladimir, I was able to provoke this error by editing >>> ./tests/vm/Makefile.include and removing the --snapshot invocation, then >>> using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd` >>> to open up a shell. >>> >>> Then the tricks are the usual ones; navigate to iotests directory, >>> ./check -v -qcow2 169, etc. You'll need to create a build that allows >>> Python tests to run; do it before you run the snapshot-less build. >>> >>> >> >> Interesting, I'll try to reproduce. > > Could you provide more detailed steps? > > # make vm-build-freebsd > VM-IMAGE freebsd > ### Downloading install iso
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
On 18/09/2019 00.29, John Snow wrote: > > > On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: [...] >> Finally do we support something except linux for iotests? >> for bash tests _supported_os also used only with "Linux" in 87 tests.. The iotests in the "auto" group are supposed to work on other OSes beside Linux, too, since they are run automatically during "make check" now. You can use github with cirrus-ci to check FreeBSD and macOS, see e.g.: https://cirrus-ci.com/build/5114679677943808 Travis has support for macOS, too. And to test them on OpenBSD (or FreeBSD), you can use the VM tests, e.g. something like this: make vm-build-openbsd J=8 BUILD_TARGET=check-block \ EXTRA_CONFIGURE_OPTS=--target-list=x86_64-softmmu > aaand lastly, running `make check` doesn't happen to call any of the > tests that appear broken on FreeBSD right now, so I'm just going to go > ahead and say we can open Pandora's box and make the default python test > behavior to run on any OS, and start re-blacklisting the edge-cases as > we find them. Sounds good. If it breaks on FreeBSD or macOS, we'll find out with cirrus-ci or Travis pretty soon. Thomas
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
18.09.2019 1:29, John Snow wrote: > > > On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: >> 12.09.2019 3:16, John Snow wrote: >>> Like script_main, but doesn't require a single point of entry. >>> Replace all existing initialization sections with this drop-in replacement. >>> >>> This brings debug support to all existing script-style iotests. >>> >>> Note: supported_oses=['linux'] was omitted, as it is a default argument. >> >> But after this patch all test which didn't check os start to check linux >> (as it's default).. So all tests which worked on other platforms will now >> be skipped on these other platforms? >> >> Finally do we support something except linux for iotests? >> for bash tests _supported_os also used only with "Linux" in 87 tests.. >> >> May be we'd better drop both _supported_os and supported_oses alltogether, >> and don't care? >> >> Anyway, if we support only linux, any reason to skip almost all tests, >> if someone tries to run test on other platform? Let him run what he wants. >> > > Currently, the following tests are python: > > 030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147 > 148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209 > 210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246 > 248 254 255 256 257 258 262 266 > > And as it stands, none of the script-style python tests we've selected > to run in `make check` fail on the FreeBSD platform. > > So as an experiment, I lifted the restriction on python tests. I kept > running ./check until I found some invocation that they didn't skip. > > Failures: 045 147 149 169 194 199 211 > > Not too bad... > > 045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist > 149: Wants to use 'sudo', but our freebsd image doesn't have that. > 194: +OSError: AF_UNIX path too long > 211: > -[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true, > "offset": 1024}, > -{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data": > true, "offset": 4096}] > +[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data": > true, "offset": 1024}, > +{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data": > true, "offset": 32768}] > > > 149 can probably be fixed, and 194 and 211 I have fail in similar ways > on my own Linux machine, so that's probably not BSD's fault. > > Interestingly, 169 and 199, bitmap migration related tests, cause a > SIGSEGV in QEMU ... > > > 169: > + > +WARNING:qemu.machine:qemu received signal 6: > /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 > -chardev > socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock > -mon chardev=mon,mode=control -display none -vga none -qtest > unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock > -machine accel=qtest -nodefaults -display none -machine accel=qtest > -incoming defer -drive > if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback > > The common thread in 169 is the +migbitmap trait, which ... makes me a > little nervous, of course! > > > 199: > +WARNING:qemu.machine:qemu received signal 6: > /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 > -chardev > socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock > -mon chardev=mon,mode=control -display none -vga none -qtest > unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock > -machine accel=qtest -nodefaults -display none -machine accel=qtest > -drive > if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none > -incoming exec: cat > '/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo' > +E > > > Vladimir, I was able to provoke this error by editing > ./tests/vm/Makefile.include and removing the --snapshot invocation, then > using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd` > to open up a shell. > > Then the tricks are the usual ones; navigate to iotests directory, > ./check -v -qcow2 169, etc. You'll need to create a build that allows > Python tests to run; do it before you run the snapshot-less build. > > Interesting, I'll try to reproduce. > > > aaand lastly, running `make check` doesn't happen to call any of the > tests that appear broken on FreeBSD right now, so I'm just going to go > ahead and say we can open Pandora's box and make the default python test > behavior to run on any OS, and start re-blacklisting the edge-cases as > we find them. > > As far as iotests go, there are a few new broken ones that surface, but > they won't gate Peter Maydell's build process
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: > 12.09.2019 3:16, John Snow wrote: >> Like script_main, but doesn't require a single point of entry. >> Replace all existing initialization sections with this drop-in replacement. >> >> This brings debug support to all existing script-style iotests. >> >> Note: supported_oses=['linux'] was omitted, as it is a default argument. > > But after this patch all test which didn't check os start to check linux > (as it's default).. So all tests which worked on other platforms will now > be skipped on these other platforms? > > Finally do we support something except linux for iotests? > for bash tests _supported_os also used only with "Linux" in 87 tests.. > > May be we'd better drop both _supported_os and supported_oses alltogether, > and don't care? > > Anyway, if we support only linux, any reason to skip almost all tests, > if someone tries to run test on other platform? Let him run what he wants. > Currently, the following tests are python: 030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147 148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209 210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246 248 254 255 256 257 258 262 266 And as it stands, none of the script-style python tests we've selected to run in `make check` fail on the FreeBSD platform. So as an experiment, I lifted the restriction on python tests. I kept running ./check until I found some invocation that they didn't skip. Failures: 045 147 149 169 194 199 211 Not too bad... 045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist 149: Wants to use 'sudo', but our freebsd image doesn't have that. 194: +OSError: AF_UNIX path too long 211: -[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true, "offset": 1024}, -{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data": true, "offset": 4096}] +[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data": true, "offset": 1024}, +{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data": true, "offset": 32768}] 149 can probably be fixed, and 194 and 211 I have fail in similar ways on my own Linux machine, so that's probably not BSD's fault. Interestingly, 169 and 199, bitmap migration related tests, cause a SIGSEGV in QEMU ... 169: + +WARNING:qemu.machine:qemu received signal 6: /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 -chardev socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock -machine accel=qtest -nodefaults -display none -machine accel=qtest -incoming defer -drive if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback The common thread in 169 is the +migbitmap trait, which ... makes me a little nervous, of course! 199: +WARNING:qemu.machine:qemu received signal 6: /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 -chardev socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock -mon chardev=mon,mode=control -display none -vga none -qtest unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock -machine accel=qtest -nodefaults -display none -machine accel=qtest -drive if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none -incoming exec: cat '/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo' +E Vladimir, I was able to provoke this error by editing ./tests/vm/Makefile.include and removing the --snapshot invocation, then using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd` to open up a shell. Then the tricks are the usual ones; navigate to iotests directory, ./check -v -qcow2 169, etc. You'll need to create a build that allows Python tests to run; do it before you run the snapshot-less build. aaand lastly, running `make check` doesn't happen to call any of the tests that appear broken on FreeBSD right now, so I'm just going to go ahead and say we can open Pandora's box and make the default python test behavior to run on any OS, and start re-blacklisting the edge-cases as we find them. As far as iotests go, there are a few new broken ones that surface, but they won't gate Peter Maydell's build process because they don't get run by 'make check'. I think it's a safe move to make. --js
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
On 9/16/19 12:39 PM, Vladimir Sementsov-Ogievskiy wrote: > 16.09.2019 19:32, John Snow wrote: >> >> >> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: >>> 12.09.2019 3:16, John Snow wrote: Like script_main, but doesn't require a single point of entry. Replace all existing initialization sections with this drop-in replacement. This brings debug support to all existing script-style iotests. Note: supported_oses=['linux'] was omitted, as it is a default argument. >>> >>> But after this patch all test which didn't check os start to check linux >>> (as it's default).. So all tests which worked on other platforms will now >>> be skipped on these other platforms? >>> >> >> def verify_platform(supported_oses=['linux']): >> if True not in [sys.platform.startswith(x) for x in supported_oses]: >> notrun('not suitable for this OS: %s' % sys.platform) >> >> >> It was already the default. I didn't *make* it the default. > > Yes. But for some tests, verify_platform wasn't called before this patch at > all. > And now it is called and checks "linux". It's the change. Or what I miss? > I guess there are more than I thought. (I thought it was just one, based on a discussion with Philippe.) So, you're right: there are more tests now that will refuse to run on non-Linux platforms. Those tests are: 206 207 208 209 210 211 212 213 218 219 235 236 237 238 242 246 248 254 255 256 What sets these apart from the other python-based tests? Nothing in particular? Just depends on who copied from whom that day. I don't think there's any rhyme or reason to any of it. I don't really have a BSD or OSX machine to test which ones should and should not be limited, either. Kevin, do you remember why we added these checks? --js
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
16.09.2019 19:32, John Snow wrote: > > > On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: >> 12.09.2019 3:16, John Snow wrote: >>> Like script_main, but doesn't require a single point of entry. >>> Replace all existing initialization sections with this drop-in replacement. >>> >>> This brings debug support to all existing script-style iotests. >>> >>> Note: supported_oses=['linux'] was omitted, as it is a default argument. >> >> But after this patch all test which didn't check os start to check linux >> (as it's default).. So all tests which worked on other platforms will now >> be skipped on these other platforms? >> > > def verify_platform(supported_oses=['linux']): > if True not in [sys.platform.startswith(x) for x in supported_oses]: > notrun('not suitable for this OS: %s' % sys.platform) > > > It was already the default. I didn't *make* it the default. Yes. But for some tests, verify_platform wasn't called before this patch at all. And now it is called and checks "linux". It's the change. Or what I miss? > There is no > change here. Feel free to propose a fix, but I don't think it's within > the scope of this series. > >> Finally do we support something except linux for iotests? >> for bash tests _supported_os also used only with "Linux" in 87 tests.. >> > > Evidently, not really. See bc521696607c5348fcd8a9e57b408d0ac0dbe2f8 > >> May be we'd better drop both _supported_os and supported_oses alltogether, >> and don't care? >> > > Beyond the scope of this series. > >> Anyway, if we support only linux, any reason to skip almost all tests, >> if someone tries to run test on other platform? Let him run what he wants. > > Maybe true, maybe not. > >> >>> Signed-off-by: John Snow >>> --- >> >> [..] >> >>> +def execute_test(test_function=None, *args, **kwargs): >>> +"""Run either unittest or script-style tests.""" >>> + >>> +debug = execute_setup_common(*args, **kwargs) >>>if not test_function: >>> -execute_unittest(output, verbosity, debug) >>> +execute_unittest(debug) >>>else: >>>test_function() >>> >>> +# This is called from script-style iotests without a single point of entry >>> +def script_initialize(*args, **kwargs): >>> +"""Initialize script-style tests without running any tests.""" >>> +execute_setup_common(*args, **kwargs) >>> + >>> +# This is called from script-style iotests with a single point of entry >>>def script_main(test_function, *args, **kwargs): >>>"""Run script-style tests outside of the unittest framework""" >>>execute_test(test_function, *args, **kwargs) >>> >>> +# This is called from unittest style iotests >>>def main(*args, **kwargs): >>>"""Run tests using the unittest framework""" >> >> >> Hmm, now two different styles of code documenting used: comment and >> doc-string, >> both containing almost equal meaning.. I don't like it, still don't really >> mind. > > I think I was trying to document what the function /does/ separately > from a note about explaining how and where it is used. It's quite nearly > redundant and if it's distracting I can remove it. > -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote: > 12.09.2019 3:16, John Snow wrote: >> Like script_main, but doesn't require a single point of entry. >> Replace all existing initialization sections with this drop-in replacement. >> >> This brings debug support to all existing script-style iotests. >> >> Note: supported_oses=['linux'] was omitted, as it is a default argument. > > But after this patch all test which didn't check os start to check linux > (as it's default).. So all tests which worked on other platforms will now > be skipped on these other platforms? > def verify_platform(supported_oses=['linux']): if True not in [sys.platform.startswith(x) for x in supported_oses]: notrun('not suitable for this OS: %s' % sys.platform) It was already the default. I didn't *make* it the default. There is no change here. Feel free to propose a fix, but I don't think it's within the scope of this series. > Finally do we support something except linux for iotests? > for bash tests _supported_os also used only with "Linux" in 87 tests.. > Evidently, not really. See bc521696607c5348fcd8a9e57b408d0ac0dbe2f8 > May be we'd better drop both _supported_os and supported_oses alltogether, > and don't care? > Beyond the scope of this series. > Anyway, if we support only linux, any reason to skip almost all tests, > if someone tries to run test on other platform? Let him run what he wants. Maybe true, maybe not. > >> Signed-off-by: John Snow >> --- > > [..] > >> +def execute_test(test_function=None, *args, **kwargs): >> +"""Run either unittest or script-style tests.""" >> + >> +debug = execute_setup_common(*args, **kwargs) >> if not test_function: >> -execute_unittest(output, verbosity, debug) >> +execute_unittest(debug) >> else: >> test_function() >> >> +# This is called from script-style iotests without a single point of entry >> +def script_initialize(*args, **kwargs): >> +"""Initialize script-style tests without running any tests.""" >> +execute_setup_common(*args, **kwargs) >> + >> +# This is called from script-style iotests with a single point of entry >> def script_main(test_function, *args, **kwargs): >> """Run script-style tests outside of the unittest framework""" >> execute_test(test_function, *args, **kwargs) >> >> +# This is called from unittest style iotests >> def main(*args, **kwargs): >> """Run tests using the unittest framework""" > > > Hmm, now two different styles of code documenting used: comment and > doc-string, > both containing almost equal meaning.. I don't like it, still don't really > mind. I think I was trying to document what the function /does/ separately from a note about explaining how and where it is used. It's quite nearly redundant and if it's distracting I can remove it.
Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
12.09.2019 3:16, John Snow wrote: > Like script_main, but doesn't require a single point of entry. > Replace all existing initialization sections with this drop-in replacement. > > This brings debug support to all existing script-style iotests. > > Note: supported_oses=['linux'] was omitted, as it is a default argument. But after this patch all test which didn't check os start to check linux (as it's default).. So all tests which worked on other platforms will now be skipped on these other platforms? Finally do we support something except linux for iotests? for bash tests _supported_os also used only with "Linux" in 87 tests.. May be we'd better drop both _supported_os and supported_oses alltogether, and don't care? Anyway, if we support only linux, any reason to skip almost all tests, if someone tries to run test on other platform? Let him run what he wants. > Signed-off-by: John Snow > --- [..] > +def execute_test(test_function=None, *args, **kwargs): > +"""Run either unittest or script-style tests.""" > + > +debug = execute_setup_common(*args, **kwargs) > if not test_function: > -execute_unittest(output, verbosity, debug) > +execute_unittest(debug) > else: > test_function() > > +# This is called from script-style iotests without a single point of entry > +def script_initialize(*args, **kwargs): > +"""Initialize script-style tests without running any tests.""" > +execute_setup_common(*args, **kwargs) > + > +# This is called from script-style iotests with a single point of entry > def script_main(test_function, *args, **kwargs): > """Run script-style tests outside of the unittest framework""" > execute_test(test_function, *args, **kwargs) > > +# This is called from unittest style iotests > def main(*args, **kwargs): > """Run tests using the unittest framework""" Hmm, now two different styles of code documenting used: comment and doc-string, both containing almost equal meaning.. I don't like it, still don't really mind. > execute_test(None, *args, **kwargs) > -- Best regards, Vladimir