[kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Thomas Huth]

Instead of failing the tests, we should rather skip them if ncat is
not available.
While we're at it, also mention ncat in the README.md file as a
requirement for the migration tests.

Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 README.md             | 4 ++++
 scripts/arch-run.bash | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 6e6a9d0..a82da56 100644
--- a/README.md
+++ b/README.md
@@ -54,6 +54,10 @@ ACCEL=name environment variable:
 
     ACCEL=kvm ./x86-run ./x86/msr.flat
 
+For running tests that involve migration from one QEMU instance to another
+you also need to have the "ncat" binary (from the nmap.org project) installed,
+otherwise the related tests will be skipped.
+
 # Tests configuration file
 
 The test case may need specific runtime configurations, for
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 43da998..cd92ed9 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -108,7 +108,7 @@ run_migration ()
 {
 	if ! command -v ncat >/dev/null 2>&1; then
 		echo "${FUNCNAME[0]} needs ncat (netcat)" >&2
-		return 2
+		return 77
 	fi
 
 	migsock=$(mktemp -u -t mig-helper-socket.XXXXXXXXXX)
-- 
2.27.0

Re: [kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Paolo Bonzini]

On 12/21/21 10:21, Thomas Huth wrote:
> Instead of failing the tests, we should rather skip them if ncat is
> not available.
> While we're at it, also mention ncat in the README.md file as a
> requirement for the migration tests.
> 
> Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
> Signed-off-by: Thomas Huth <thuth@redhat.com>

I would rather remove the migration tests.  There's really no reason for 
them, the KVM selftests in the Linux tree are much better: they can find 
migration bugs deterministically and they are really really easy to 
debug.  The only disadvantage is that they are harder to write.

Paolo

> ---
>   README.md             | 4 ++++
>   scripts/arch-run.bash | 2 +-
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/README.md b/README.md
> index 6e6a9d0..a82da56 100644
> --- a/README.md
> +++ b/README.md
> @@ -54,6 +54,10 @@ ACCEL=name environment variable:
>   
>       ACCEL=kvm ./x86-run ./x86/msr.flat
>   
> +For running tests that involve migration from one QEMU instance to another
> +you also need to have the "ncat" binary (from the nmap.org project) installed,
> +otherwise the related tests will be skipped.
> +
>   # Tests configuration file
>   
>   The test case may need specific runtime configurations, for
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 43da998..cd92ed9 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -108,7 +108,7 @@ run_migration ()
>   {
>   	if ! command -v ncat >/dev/null 2>&1; then
>   		echo "${FUNCNAME[0]} needs ncat (netcat)" >&2
> -		return 2
> +		return 77
>   	fi
>   
>   	migsock=$(mktemp -u -t mig-helper-socket.XXXXXXXXXX)
> 

Re: [kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Thomas Huth]

On 21/12/2021 10.58, Paolo Bonzini wrote:
> On 12/21/21 10:21, Thomas Huth wrote:
>> Instead of failing the tests, we should rather skip them if ncat is
>> not available.
>> While we're at it, also mention ncat in the README.md file as a
>> requirement for the migration tests.
>>
>> Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> I would rather remove the migration tests.  There's really no reason for 
> them, the KVM selftests in the Linux tree are much better: they can find 
> migration bugs deterministically and they are really really easy to debug.  
> The only disadvantage is that they are harder to write.

I disagree: We're testing migration with QEMU here - the KVM selftests don't 
include QEMU, so we'd lose some test coverage here.
And at least the powerpc/sprs.c test helped to find some nasty bugs in the 
past already.

  Thomas

Re: [kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Eric Auger]

Hi,
On 12/21/21 11:12 AM, Thomas Huth wrote:
> On 21/12/2021 10.58, Paolo Bonzini wrote:
>> On 12/21/21 10:21, Thomas Huth wrote:
>>> Instead of failing the tests, we should rather skip them if ncat is
>>> not available.
>>> While we're at it, also mention ncat in the README.md file as a
>>> requirement for the migration tests.
>>>
>>> Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> I would rather remove the migration tests.  There's really no reason
>> for them, the KVM selftests in the Linux tree are much better: they
>> can find migration bugs deterministically and they are really really
>> easy to debug.  The only disadvantage is that they are harder to write.
>
> I disagree: We're testing migration with QEMU here - the KVM selftests
> don't include QEMU, so we'd lose some test coverage here.
> And at least the powerpc/sprs.c test helped to find some nasty bugs in
> the past already.
I do agree. The ITS migration tests were good reproducer for upstream bugs.

Thanks

Eric
>
>  Thomas
>

Re: [kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Paolo Bonzini]

On 12/21/21 11:12, Thomas Huth wrote:
> On 21/12/2021 10.58, Paolo Bonzini wrote:
>> On 12/21/21 10:21, Thomas Huth wrote:
>>> Instead of failing the tests, we should rather skip them if ncat is
>>> not available.
>>> While we're at it, also mention ncat in the README.md file as a
>>> requirement for the migration tests.
>>>
>>> Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> I would rather remove the migration tests.  There's really no reason 
>> for them, the KVM selftests in the Linux tree are much better: they 
>> can find migration bugs deterministically and they are really really 
>> easy to debug. The only disadvantage is that they are harder to write.
> 
> I disagree: We're testing migration with QEMU here - the KVM selftests 
> don't include QEMU, so we'd lose some test coverage here.
> And at least the powerpc/sprs.c test helped to find some nasty bugs in 
> the past already.

I understand that this is testing QEMU, but I'm not sure that testing 
QEMU should be part of kvm-unit-tests.  Migrating an instance of QEMU 
that runs kvm-unit-tests would be done more easily in avocado-vt or 
avocado-qemu.

Paolo

Re: [kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Andrew Jones]

On Tue, Dec 21, 2021 at 06:25:30PM +0100, Paolo Bonzini wrote:
> On 12/21/21 11:12, Thomas Huth wrote:
> > On 21/12/2021 10.58, Paolo Bonzini wrote:
> > > On 12/21/21 10:21, Thomas Huth wrote:
> > > > Instead of failing the tests, we should rather skip them if ncat is
> > > > not available.
> > > > While we're at it, also mention ncat in the README.md file as a
> > > > requirement for the migration tests.
> > > > 
> > > > Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
> > > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > 
> > > I would rather remove the migration tests.  There's really no reason
> > > for them, the KVM selftests in the Linux tree are much better: they
> > > can find migration bugs deterministically and they are really really
> > > easy to debug. The only disadvantage is that they are harder to
> > > write.
> > 
> > I disagree: We're testing migration with QEMU here - the KVM selftests
> > don't include QEMU, so we'd lose some test coverage here.
> > And at least the powerpc/sprs.c test helped to find some nasty bugs in
> > the past already.
> 
> I understand that this is testing QEMU, but I'm not sure that testing QEMU
> should be part of kvm-unit-tests.  Migrating an instance of QEMU that runs
> kvm-unit-tests would be done more easily in avocado-vt or avocado-qemu.
>

Migrating is easier with avocado*, but if we want to migrate kut unit
tests, and the unit tests want to ensure the guest is in a specific state
at the time of the migration, then we'll still need the getchar() stuff.
And, if we need the getchar() stuff, then I think we also need a
lightweight way to test migration, which is currently the ncat-based
run_migration bash function. IOW, I vote we keep the code we have, but I'm
also in favor of people building new test harnesses for the kut *.flat
files which can better exercise QEMU or whatever.

Thanks,
drew

Re: [kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Thomas Huth]

On 21/12/2021 18.25, Paolo Bonzini wrote:
> On 12/21/21 11:12, Thomas Huth wrote:
>> On 21/12/2021 10.58, Paolo Bonzini wrote:
>>> On 12/21/21 10:21, Thomas Huth wrote:
>>>> Instead of failing the tests, we should rather skip them if ncat is
>>>> not available.
>>>> While we're at it, also mention ncat in the README.md file as a
>>>> requirement for the migration tests.
>>>>
>>>> Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>
>>> I would rather remove the migration tests.  There's really no reason for 
>>> them, the KVM selftests in the Linux tree are much better: they can find 
>>> migration bugs deterministically and they are really really easy to 
>>> debug. The only disadvantage is that they are harder to write.
>>
>> I disagree: We're testing migration with QEMU here - the KVM selftests 
>> don't include QEMU, so we'd lose some test coverage here.
>> And at least the powerpc/sprs.c test helped to find some nasty bugs in the 
>> past already.
> 
> I understand that this is testing QEMU, but I'm not sure that testing QEMU 
> should be part of kvm-unit-tests.

It's the combination of QEMU (migration handling) + KVM in the kernel 
(register saving and restoring) that we are testing here. If you say that 
QEMU should not be involved at all, we could also say that all 
kvm-unit-tests should be converted to KVM selftests instead...

>  Migrating an instance of QEMU that runs 
> kvm-unit-tests would be done more easily in avocado-vt or avocado-qemu.

But we don't have the environment for compiling small, Linux-independent 
test kernels there, do we? So unless there is a way to write and compile 
small test kernels in that framework, I think kvm-unit-tests is the better 
fit for these kind of tests.

  Thomas

Re: [kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Andrew Jones]

On Tue, Dec 21, 2021 at 10:21:30AM +0100, Thomas Huth wrote:
> Instead of failing the tests, we should rather skip them if ncat is
> not available.
> While we're at it, also mention ncat in the README.md file as a
> requirement for the migration tests.
> 
> Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  README.md             | 4 ++++
>  scripts/arch-run.bash | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/README.md b/README.md
> index 6e6a9d0..a82da56 100644
> --- a/README.md
> +++ b/README.md
> @@ -54,6 +54,10 @@ ACCEL=name environment variable:
>  
>      ACCEL=kvm ./x86-run ./x86/msr.flat
>  
> +For running tests that involve migration from one QEMU instance to another
> +you also need to have the "ncat" binary (from the nmap.org project) installed,
> +otherwise the related tests will be skipped.
> +
>  # Tests configuration file
>  
>  The test case may need specific runtime configurations, for
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 43da998..cd92ed9 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -108,7 +108,7 @@ run_migration ()
>  {
>  	if ! command -v ncat >/dev/null 2>&1; then
>  		echo "${FUNCNAME[0]} needs ncat (netcat)" >&2
> -		return 2
> +		return 77
>  	fi
>  
>  	migsock=$(mktemp -u -t mig-helper-socket.XXXXXXXXXX)
> -- 
> 2.27.0
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [kvm-unit-tests PATCH] scripts/arch-run: Mark migration tests as SKIP if ncat is not available

[Paolo Bonzini]

On 12/21/21 10:21, Thomas Huth wrote:
> Instead of failing the tests, we should rather skip them if ncat is
> not available.
> While we're at it, also mention ncat in the README.md file as a
> requirement for the migration tests.
> 
> Resolves: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/issues/4
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   README.md             | 4 ++++
>   scripts/arch-run.bash | 2 +-
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/README.md b/README.md
> index 6e6a9d0..a82da56 100644
> --- a/README.md
> +++ b/README.md
> @@ -54,6 +54,10 @@ ACCEL=name environment variable:
>   
>       ACCEL=kvm ./x86-run ./x86/msr.flat
>   
> +For running tests that involve migration from one QEMU instance to another
> +you also need to have the "ncat" binary (from the nmap.org project) installed,
> +otherwise the related tests will be skipped.
> +
>   # Tests configuration file
>   
>   The test case may need specific runtime configurations, for
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 43da998..cd92ed9 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -108,7 +108,7 @@ run_migration ()
>   {
>   	if ! command -v ncat >/dev/null 2>&1; then
>   		echo "${FUNCNAME[0]} needs ncat (netcat)" >&2
> -		return 2
> +		return 77
>   	fi
>   
>   	migsock=$(mktemp -u -t mig-helper-socket.XXXXXXXXXX)

Queued, thanks.

Paolo