| Message ID | 20190219115937.21587-1-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | iotests: drop unnecessary accel=kvm | expand |
On 19/02/2019 12.59, Stefan Hajnoczi wrote: > Tests 235 and 238 do not require the kvm accelerator. TCG works fine. > > Use the default accelerator instead of requiring kvm. > > Suggested-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > tests/qemu-iotests/235 | 1 - > tests/qemu-iotests/238 | 1 - > 2 files changed, 2 deletions(-) > > diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 > index d6edd97ab4..329da8f0c2 100755 > --- a/tests/qemu-iotests/235 > +++ b/tests/qemu-iotests/235 > @@ -49,7 +49,6 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, > str(size)) > > vm = QEMUMachine(iotests.qemu_prog) > -vm.add_args('-machine', 'accel=kvm') According to the initial commit log of 235: "iotests: simple mirror test with kvm on 1G image" ... so I assume KVM was used on purpose here? Thomas
19.02.2019 15:02, Thomas Huth wrote: > On 19/02/2019 12.59, Stefan Hajnoczi wrote: >> Tests 235 and 238 do not require the kvm accelerator. TCG works fine. >> >> Use the default accelerator instead of requiring kvm. >> >> Suggested-by: Thomas Huth <thuth@redhat.com> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> tests/qemu-iotests/235 | 1 - >> tests/qemu-iotests/238 | 1 - >> 2 files changed, 2 deletions(-) >> >> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 >> index d6edd97ab4..329da8f0c2 100755 >> --- a/tests/qemu-iotests/235 >> +++ b/tests/qemu-iotests/235 >> @@ -49,7 +49,6 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >> str(size)) >> >> vm = QEMUMachine(iotests.qemu_prog) >> -vm.add_args('-machine', 'accel=kvm') > > According to the initial commit log of 235: > > "iotests: simple mirror test with kvm on 1G image" > > ... so I assume KVM was used on purpose here? > > Thomas > As I remember kvm is not really necessary, and test have a comment about it: # And it didn't reproduce if at least one of the following: ... # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still # reproduces, if just drop kvm, but gdb failed to produce full backtraces # for me) But the comment should be updated with this patch.
On Tue, Feb 19, 2019 at 12:12 PM Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > 19.02.2019 15:02, Thomas Huth wrote: > > On 19/02/2019 12.59, Stefan Hajnoczi wrote: > >> Tests 235 and 238 do not require the kvm accelerator. TCG works fine. > >> > >> Use the default accelerator instead of requiring kvm. > >> > >> Suggested-by: Thomas Huth <thuth@redhat.com> > >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > >> --- > >> tests/qemu-iotests/235 | 1 - > >> tests/qemu-iotests/238 | 1 - > >> 2 files changed, 2 deletions(-) > >> > >> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 > >> index d6edd97ab4..329da8f0c2 100755 > >> --- a/tests/qemu-iotests/235 > >> +++ b/tests/qemu-iotests/235 > >> @@ -49,7 +49,6 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, > >> str(size)) > >> > >> vm = QEMUMachine(iotests.qemu_prog) > >> -vm.add_args('-machine', 'accel=kvm') > > > > According to the initial commit log of 235: > > > > "iotests: simple mirror test with kvm on 1G image" > > > > ... so I assume KVM was used on purpose here? > > > > Thomas > > > > As I remember kvm is not really necessary, and test have a comment about it: > # And it didn't reproduce if at least one of the following: > ... > # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still > # reproduces, if just drop kvm, but gdb failed to produce full backtraces > # for me) > > But the comment should be updated with this patch. Will fix in v2. Stefan
On 19/02/2019 14.07, Stefan Hajnoczi wrote: > On Tue, Feb 19, 2019 at 12:12 PM Vladimir Sementsov-Ogievskiy > <vsementsov@virtuozzo.com> wrote: >> 19.02.2019 15:02, Thomas Huth wrote: >>> On 19/02/2019 12.59, Stefan Hajnoczi wrote: >>>> Tests 235 and 238 do not require the kvm accelerator. TCG works fine. >>>> >>>> Use the default accelerator instead of requiring kvm. >>>> >>>> Suggested-by: Thomas Huth <thuth@redhat.com> >>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >>>> --- >>>> tests/qemu-iotests/235 | 1 - >>>> tests/qemu-iotests/238 | 1 - >>>> 2 files changed, 2 deletions(-) >>>> >>>> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 >>>> index d6edd97ab4..329da8f0c2 100755 >>>> --- a/tests/qemu-iotests/235 >>>> +++ b/tests/qemu-iotests/235 >>>> @@ -49,7 +49,6 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>> str(size)) >>>> >>>> vm = QEMUMachine(iotests.qemu_prog) >>>> -vm.add_args('-machine', 'accel=kvm') >>> >>> According to the initial commit log of 235: >>> >>> "iotests: simple mirror test with kvm on 1G image" >>> >>> ... so I assume KVM was used on purpose here? >>> >>> Thomas >>> >> >> As I remember kvm is not really necessary, and test have a comment about it: >> # And it didn't reproduce if at least one of the following: >> ... >> # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still >> # reproduces, if just drop kvm, but gdb failed to produce full backtraces >> # for me) >> >> But the comment should be updated with this patch. > > Will fix in v2. Any chance that you could even make it somehow work with -M accel=qtest instead? ... in case someone compiled their QEMU with --disable-tcg, too ...? Thomas
19.02.2019 16:20, Thomas Huth wrote: > On 19/02/2019 14.07, Stefan Hajnoczi wrote: >> On Tue, Feb 19, 2019 at 12:12 PM Vladimir Sementsov-Ogievskiy >> <vsementsov@virtuozzo.com> wrote: >>> 19.02.2019 15:02, Thomas Huth wrote: >>>> On 19/02/2019 12.59, Stefan Hajnoczi wrote: >>>>> Tests 235 and 238 do not require the kvm accelerator. TCG works fine. >>>>> >>>>> Use the default accelerator instead of requiring kvm. >>>>> >>>>> Suggested-by: Thomas Huth <thuth@redhat.com> >>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >>>>> --- >>>>> tests/qemu-iotests/235 | 1 - >>>>> tests/qemu-iotests/238 | 1 - >>>>> 2 files changed, 2 deletions(-) >>>>> >>>>> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 >>>>> index d6edd97ab4..329da8f0c2 100755 >>>>> --- a/tests/qemu-iotests/235 >>>>> +++ b/tests/qemu-iotests/235 >>>>> @@ -49,7 +49,6 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>> str(size)) >>>>> >>>>> vm = QEMUMachine(iotests.qemu_prog) >>>>> -vm.add_args('-machine', 'accel=kvm') >>>> >>>> According to the initial commit log of 235: >>>> >>>> "iotests: simple mirror test with kvm on 1G image" >>>> >>>> ... so I assume KVM was used on purpose here? >>>> >>>> Thomas >>>> >>> >>> As I remember kvm is not really necessary, and test have a comment about it: >>> # And it didn't reproduce if at least one of the following: >>> ... >>> # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still >>> # reproduces, if just drop kvm, but gdb failed to produce full backtraces >>> # for me) >>> >>> But the comment should be updated with this patch. >> >> Will fix in v2. > > Any chance that you could even make it somehow work with -M accel=qtest > instead? ... in case someone compiled their QEMU with --disable-tcg, too > ...? > I didn't investigate why bug not reproduced with qtest.. I think, the simplest option should be a helper like iotests.needs_tcg(), to skip the test if it is unsupported. Or you case is that kvm is OK for you but tcg is not? Stefan, do you have strong reason for removing kvm from iotests? Could it be something like if not iotests.supports_tcg(): vm.add_args('-machine', 'accel=kvm') ? Hmm, maybe, good option would be topmost option, like we have for format -qcow2, -raw, etc. So, it would be -qtest, -kvm, -tcg.. But in this case nobody will run all test on something other than qtest.. It's not bad to run this test on qtest. The problem is we can miss the bug, if not run this test on kvm or tcg.
On 19/02/2019 14.36, Vladimir Sementsov-Ogievskiy wrote: > 19.02.2019 16:20, Thomas Huth wrote: >> On 19/02/2019 14.07, Stefan Hajnoczi wrote: >>> On Tue, Feb 19, 2019 at 12:12 PM Vladimir Sementsov-Ogievskiy >>> <vsementsov@virtuozzo.com> wrote: >>>> 19.02.2019 15:02, Thomas Huth wrote: >>>>> On 19/02/2019 12.59, Stefan Hajnoczi wrote: >>>>>> Tests 235 and 238 do not require the kvm accelerator. TCG works fine. >>>>>> >>>>>> Use the default accelerator instead of requiring kvm. >>>>>> >>>>>> Suggested-by: Thomas Huth <thuth@redhat.com> >>>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >>>>>> --- >>>>>> tests/qemu-iotests/235 | 1 - >>>>>> tests/qemu-iotests/238 | 1 - >>>>>> 2 files changed, 2 deletions(-) >>>>>> >>>>>> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 >>>>>> index d6edd97ab4..329da8f0c2 100755 >>>>>> --- a/tests/qemu-iotests/235 >>>>>> +++ b/tests/qemu-iotests/235 >>>>>> @@ -49,7 +49,6 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>>> str(size)) >>>>>> >>>>>> vm = QEMUMachine(iotests.qemu_prog) >>>>>> -vm.add_args('-machine', 'accel=kvm') >>>>> >>>>> According to the initial commit log of 235: >>>>> >>>>> "iotests: simple mirror test with kvm on 1G image" >>>>> >>>>> ... so I assume KVM was used on purpose here? >>>> >>>> As I remember kvm is not really necessary, and test have a comment about it: >>>> # And it didn't reproduce if at least one of the following: >>>> ... >>>> # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still >>>> # reproduces, if just drop kvm, but gdb failed to produce full backtraces >>>> # for me) >>>> >>>> But the comment should be updated with this patch. >>> >>> Will fix in v2. >> >> Any chance that you could even make it somehow work with -M accel=qtest >> instead? ... in case someone compiled their QEMU with --disable-tcg, too >> ...? > > I didn't investigate why bug not reproduced with qtest.. I think, the simplest > option should be a helper like iotests.needs_tcg(), to skip the test if > it is unsupported. > > Or you case is that kvm is OK for you but tcg is not? Stefan, do you have strong reason > for removing kvm from iotests? Could it be something like > > if not iotests.supports_tcg(): > vm.add_args('-machine', 'accel=kvm') > > ? You can also use "accel=kvm:tcg" which tries to use kvm first and then falls back to tcg if KVM is not available. I just have this weird condition where I try to compile qemu with --disable-tcg, and then run the iotests on a CI system where KVM is also not available at runtime. So qtest is the only available accelerator there... Thomas
diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 index d6edd97ab4..329da8f0c2 100755 --- a/tests/qemu-iotests/235 +++ b/tests/qemu-iotests/235 @@ -49,7 +49,6 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, str(size)) vm = QEMUMachine(iotests.qemu_prog) -vm.add_args('-machine', 'accel=kvm') if iotests.qemu_default_machine == 's390-ccw-virtio': vm.add_args('-no-shutdown') vm.add_args('-drive', 'id=src,file=' + disk) diff --git a/tests/qemu-iotests/238 b/tests/qemu-iotests/238 index f81ee1112f..e490bb4298 100755 --- a/tests/qemu-iotests/238 +++ b/tests/qemu-iotests/238 @@ -33,7 +33,6 @@ else: virtio_scsi_device = 'virtio-scsi-pci' vm = QEMUMachine(iotests.qemu_prog) -vm.add_args('-machine', 'accel=kvm') vm.launch() log(vm.qmp('blockdev-add', node_name='hd0', driver='null-co'))
Tests 235 and 238 do not require the kvm accelerator. TCG works fine. Use the default accelerator instead of requiring kvm. Suggested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- tests/qemu-iotests/235 | 1 - tests/qemu-iotests/238 | 1 - 2 files changed, 2 deletions(-)