[kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Claudio Imbrenda]

A recent patch broke make standalone. The function find_word is not
available when running make standalone, replace it with a simple grep.

Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 scripts/s390x/func.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
index 2a941bbb..6c75e89a 100644
--- a/scripts/s390x/func.bash
+++ b/scripts/s390x/func.bash
@@ -21,7 +21,7 @@ function arch_cmd_s390x()
 	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 
 	# run PV test case
-	if [ "$ACCEL" = 'tcg' ] || find_word "migration" "$groups"; then
+	if [ "$ACCEL" = 'tcg' ] || grep -q "migration" <<< "$groups"; then
 		return
 	fi
 	kernel=${kernel%.elf}.pv.bin
-- 
2.38.1

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Nico Boehr]

Quoting Claudio Imbrenda (2022-12-20 18:55:08)
> A recent patch broke make standalone. The function find_word is not
> available when running make standalone, replace it with a simple grep.
> 
> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

I am confused why find_word would not be available in standalone, since run() in runtime.bash uses it quite a few times.

Not that I mind the grep, but I fear more might be broken in standalone?

Anyways, to get this fixed ASAP:

Acked-by: Nico Boehr <nrb@linux.ibm.com>

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Nico Boehr]

Quoting Nico Boehr (2022-12-21 09:16:51)
> Quoting Claudio Imbrenda (2022-12-20 18:55:08)
> > A recent patch broke make standalone. The function find_word is not
> > available when running make standalone, replace it with a simple grep.
> > 
> > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> 
> I am confused why find_word would not be available in standalone, since run() in runtime.bash uses it quite a few times.
> 
> Not that I mind the grep, but I fear more might be broken in standalone?
> 
> Anyways, to get this fixed ASAP:
> 
> Acked-by: Nico Boehr <nrb@linux.ibm.com>

OK, I get it now, find_word is not available during _build time_.

Please make this a:

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Andrew Jones]

On Tue, Dec 20, 2022 at 06:55:08PM +0100, Claudio Imbrenda wrote:
> A recent patch broke make standalone. The function find_word is not
> available when running make standalone, replace it with a simple grep.
> 
> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>  scripts/s390x/func.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
> index 2a941bbb..6c75e89a 100644
> --- a/scripts/s390x/func.bash
> +++ b/scripts/s390x/func.bash
> @@ -21,7 +21,7 @@ function arch_cmd_s390x()
>  	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>  
>  	# run PV test case
> -	if [ "$ACCEL" = 'tcg' ] || find_word "migration" "$groups"; then
> +	if [ "$ACCEL" = 'tcg' ] || grep -q "migration" <<< "$groups"; then

What about the '-F' that find_word has?

>  		return
>  	fi
>  	kernel=${kernel%.elf}.pv.bin
> -- 
> 2.38.1
> 

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Andrew Jones]

On Wed, Dec 21, 2022 at 10:14:52AM +0100, Nico Boehr wrote:
> Quoting Nico Boehr (2022-12-21 09:16:51)
> > Quoting Claudio Imbrenda (2022-12-20 18:55:08)
> > > A recent patch broke make standalone. The function find_word is not
> > > available when running make standalone, replace it with a simple grep.
> > > 
> > > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > > Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > 
> > I am confused why find_word would not be available in standalone, since run() in runtime.bash uses it quite a few times.
> > 
> > Not that I mind the grep, but I fear more might be broken in standalone?

standalone tests don't currently include scripts/$ARCH/func.bash, which
may be an issue for s390x. That could be fixed, though.

> > 
> > Anyways, to get this fixed ASAP:
> > 
> > Acked-by: Nico Boehr <nrb@linux.ibm.com>
> 
> OK, I get it now, find_word is not available during _build time_.

That could be changed, but it'd need to be moved to somewhere that
mkstandalone.sh wants to source, which could be common.bash, but
then we'd need to include common.bash in the standalone tests. So,
a new file for find_word() would be cleaner, but that sounds like
overkill.

Thanks,
drew

> 
> Please make this a:
> 
> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Nina Schoetterl-Glausch]

On Mon, 2022-12-26 at 19:41 +0100, Andrew Jones wrote:
> On Wed, Dec 21, 2022 at 10:14:52AM +0100, Nico Boehr wrote:
> > Quoting Nico Boehr (2022-12-21 09:16:51)
> > > Quoting Claudio Imbrenda (2022-12-20 18:55:08)
> > > > A recent patch broke make standalone. The function find_word is not
> > > > available when running make standalone, replace it with a simple grep.
> > > > 
> > > > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > > > Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > > 
> > > I am confused why find_word would not be available in standalone, since run() in runtime.bash uses it quite a few times.
> > > 
> > > Not that I mind the grep, but I fear more might be broken in standalone?
> 
> standalone tests don't currently include scripts/$ARCH/func.bash, which
> may be an issue for s390x. That could be fixed, though.
> 
> > > 
> > > Anyways, to get this fixed ASAP:
> > > 
> > > Acked-by: Nico Boehr <nrb@linux.ibm.com>
> > 
> > OK, I get it now, find_word is not available during _build time_.
> 
> That could be changed, but it'd need to be moved to somewhere that
> mkstandalone.sh wants to source, which could be common.bash, but
> then we'd need to include common.bash in the standalone tests. So,

What is wrong with including common.bash?

> a new file for find_word() would be cleaner, but that sounds like
> overkill.
> 
> Thanks,
> drew
> 
> > 
> > Please make this a:
> > 
> > Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Nina Schoetterl-Glausch]

On Tue, 2022-12-20 at 18:55 +0100, Claudio Imbrenda wrote:
> A recent patch broke make standalone. The function find_word is not
> available when running make standalone, replace it with a simple grep.
> 
> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
>  scripts/s390x/func.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
> index 2a941bbb..6c75e89a 100644
> --- a/scripts/s390x/func.bash
> +++ b/scripts/s390x/func.bash
> @@ -21,7 +21,7 @@ function arch_cmd_s390x()
>  	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>  
>  	# run PV test case
> -	if [ "$ACCEL" = 'tcg' ] || find_word "migration" "$groups"; then
> +	if [ "$ACCEL" = 'tcg' ] || grep -q "migration" <<< "$groups"; then
>  		return
>  	fi
>  	kernel=${kernel%.elf}.pv.bin

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Andrew Jones]

On Tue, Jan 03, 2023 at 03:04:01PM +0100, Nina Schoetterl-Glausch wrote:
> On Mon, 2022-12-26 at 19:41 +0100, Andrew Jones wrote:
> > On Wed, Dec 21, 2022 at 10:14:52AM +0100, Nico Boehr wrote:
> > > Quoting Nico Boehr (2022-12-21 09:16:51)
> > > > Quoting Claudio Imbrenda (2022-12-20 18:55:08)
> > > > > A recent patch broke make standalone. The function find_word is not
> > > > > available when running make standalone, replace it with a simple grep.
> > > > > 
> > > > > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > > > > Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> > > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > > > 
> > > > I am confused why find_word would not be available in standalone, since run() in runtime.bash uses it quite a few times.
> > > > 
> > > > Not that I mind the grep, but I fear more might be broken in standalone?
> > 
> > standalone tests don't currently include scripts/$ARCH/func.bash, which
> > may be an issue for s390x. That could be fixed, though.
> > 
> > > > 
> > > > Anyways, to get this fixed ASAP:
> > > > 
> > > > Acked-by: Nico Boehr <nrb@linux.ibm.com>
> > > 
> > > OK, I get it now, find_word is not available during _build time_.
> > 
> > That could be changed, but it'd need to be moved to somewhere that
> > mkstandalone.sh wants to source, which could be common.bash, but
> > then we'd need to include common.bash in the standalone tests. So,
> 
> What is wrong with including common.bash?

for_each_unittest() isn't something that standalone tests need and,
theoretically, other non-standalone related functions could be introduced
there. Packaging functions for standalone tests which aren't needed by
the standalone tests isn't super clean. But, it's not really a problem
either.

Thanks,
drew

> 
> > a new file for find_word() would be cleaner, but that sounds like
> > overkill.
> > 
> > Thanks,
> > drew
> > 
> > > 
> > > Please make this a:
> > > 
> > > Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
> 

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Claudio Imbrenda]

On Mon, 26 Dec 2022 19:41:12 +0100
Andrew Jones <andrew.jones@linux.dev> wrote:

> On Wed, Dec 21, 2022 at 10:14:52AM +0100, Nico Boehr wrote:
> > Quoting Nico Boehr (2022-12-21 09:16:51)  
> > > Quoting Claudio Imbrenda (2022-12-20 18:55:08)  
> > > > A recent patch broke make standalone. The function find_word is not
> > > > available when running make standalone, replace it with a simple grep.
> > > > 
> > > > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > > > Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>  
> > > 
> > > I am confused why find_word would not be available in standalone, since run() in runtime.bash uses it quite a few times.
> > > 
> > > Not that I mind the grep, but I fear more might be broken in standalone?  
> 
> standalone tests don't currently include scripts/$ARCH/func.bash, which
> may be an issue for s390x. That could be fixed, though.
> 
> > > 
> > > Anyways, to get this fixed ASAP:
> > > 
> > > Acked-by: Nico Boehr <nrb@linux.ibm.com>  
> > 
> > OK, I get it now, find_word is not available during _build time_.  
> 
> That could be changed, but it'd need to be moved to somewhere that
> mkstandalone.sh wants to source, which could be common.bash, but
> then we'd need to include common.bash in the standalone tests. So,
> a new file for find_word() would be cleaner, but that sounds like
> overkill.

the hack I posted here was meant to be "clean enough" and
arch-only (since we are the only ones with this issue). To be
honest, I don't really care __how__ we fix the problem, only that we do
fix it :)

what do you think would be the cleanest solution?

> 
> Thanks,
> drew
> 
> > 
> > Please make this a:
> > 
> > Reviewed-by: Nico Boehr <nrb@linux.ibm.com>  

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Thomas Huth]

On 26/12/2022 19.36, Andrew Jones wrote:
> On Tue, Dec 20, 2022 at 06:55:08PM +0100, Claudio Imbrenda wrote:
>> A recent patch broke make standalone. The function find_word is not
>> available when running make standalone, replace it with a simple grep.
>>
>> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
>> Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>>   scripts/s390x/func.bash | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
>> index 2a941bbb..6c75e89a 100644
>> --- a/scripts/s390x/func.bash
>> +++ b/scripts/s390x/func.bash
>> @@ -21,7 +21,7 @@ function arch_cmd_s390x()
>>   	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>>   
>>   	# run PV test case
>> -	if [ "$ACCEL" = 'tcg' ] || find_word "migration" "$groups"; then
>> +	if [ "$ACCEL" = 'tcg' ] || grep -q "migration" <<< "$groups"; then
> 
> What about the '-F' that find_word has?

"migration" is only one string without regular expressions in it, so I 
assume the -F does not matter here, does it?

  Thomas

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Thomas Huth]

On 04/01/2023 12.07, Claudio Imbrenda wrote:
> On Mon, 26 Dec 2022 19:41:12 +0100
> Andrew Jones <andrew.jones@linux.dev> wrote:
> 
>> On Wed, Dec 21, 2022 at 10:14:52AM +0100, Nico Boehr wrote:
>>> Quoting Nico Boehr (2022-12-21 09:16:51)
>>>> Quoting Claudio Imbrenda (2022-12-20 18:55:08)
>>>>> A recent patch broke make standalone. The function find_word is not
>>>>> available when running make standalone, replace it with a simple grep.
>>>>>
>>>>> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
>>>>> Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
>>>>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>>>>
>>>> I am confused why find_word would not be available in standalone, since run() in runtime.bash uses it quite a few times.
>>>>
>>>> Not that I mind the grep, but I fear more might be broken in standalone?
>>
>> standalone tests don't currently include scripts/$ARCH/func.bash, which
>> may be an issue for s390x. That could be fixed, though.
>>
>>>>
>>>> Anyways, to get this fixed ASAP:
>>>>
>>>> Acked-by: Nico Boehr <nrb@linux.ibm.com>
>>>
>>> OK, I get it now, find_word is not available during _build time_.
>>
>> That could be changed, but it'd need to be moved to somewhere that
>> mkstandalone.sh wants to source, which could be common.bash, but
>> then we'd need to include common.bash in the standalone tests. So,
>> a new file for find_word() would be cleaner, but that sounds like
>> overkill.
> 
> the hack I posted here was meant to be "clean enough" and
> arch-only (since we are the only ones with this issue). To be
> honest, I don't really care __how__ we fix the problem, only that we do
> fix it :)
> 
> what do you think would be the cleanest solution?

I think your patch is good enough for fixing the issue, so I went ahead and 
pushed it. If someone wants to figure out a nice way to make find_word 
available to the standalone builds, too, feel free to send a patch on top of 
this.

  Thanks,
   Thomas

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Andrew Jones]

On Wed, Jan 04, 2023 at 12:07:20PM +0100, Claudio Imbrenda wrote:
> On Mon, 26 Dec 2022 19:41:12 +0100
> Andrew Jones <andrew.jones@linux.dev> wrote:
> 
> > On Wed, Dec 21, 2022 at 10:14:52AM +0100, Nico Boehr wrote:
> > > Quoting Nico Boehr (2022-12-21 09:16:51)  
> > > > Quoting Claudio Imbrenda (2022-12-20 18:55:08)  
> > > > > A recent patch broke make standalone. The function find_word is not
> > > > > available when running make standalone, replace it with a simple grep.
> > > > > 
> > > > > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > > > > Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> > > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>  
> > > > 
> > > > I am confused why find_word would not be available in standalone, since run() in runtime.bash uses it quite a few times.
> > > > 
> > > > Not that I mind the grep, but I fear more might be broken in standalone?  
> > 
> > standalone tests don't currently include scripts/$ARCH/func.bash, which
> > may be an issue for s390x. That could be fixed, though.
> > 
> > > > 
> > > > Anyways, to get this fixed ASAP:
> > > > 
> > > > Acked-by: Nico Boehr <nrb@linux.ibm.com>  
> > > 
> > > OK, I get it now, find_word is not available during _build time_.  
> > 
> > That could be changed, but it'd need to be moved to somewhere that
> > mkstandalone.sh wants to source, which could be common.bash, but
> > then we'd need to include common.bash in the standalone tests. So,
> > a new file for find_word() would be cleaner, but that sounds like
> > overkill.
> 
> the hack I posted here was meant to be "clean enough" and
> arch-only (since we are the only ones with this issue). To be
> honest, I don't really care __how__ we fix the problem, only that we do
> fix it :)
> 
> what do you think would be the cleanest solution?

A new file, but, like I said, that may be overkill at this time.

Thanks,
drew

Re: [kvm-unit-tests PATCH v1 1/1] s390x: fix make standalone

[Andrew Jones]

On Wed, Jan 04, 2023 at 01:52:21PM +0100, Thomas Huth wrote:
> On 26/12/2022 19.36, Andrew Jones wrote:
> > On Tue, Dec 20, 2022 at 06:55:08PM +0100, Claudio Imbrenda wrote:
> > > A recent patch broke make standalone. The function find_word is not
> > > available when running make standalone, replace it with a simple grep.
> > > 
> > > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > > Fixes: 743cacf7 ("s390x: don't run migration tests under PV")
> > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > > ---
> > >   scripts/s390x/func.bash | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
> > > index 2a941bbb..6c75e89a 100644
> > > --- a/scripts/s390x/func.bash
> > > +++ b/scripts/s390x/func.bash
> > > @@ -21,7 +21,7 @@ function arch_cmd_s390x()
> > >   	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> > >   	# run PV test case
> > > -	if [ "$ACCEL" = 'tcg' ] || find_word "migration" "$groups"; then
> > > +	if [ "$ACCEL" = 'tcg' ] || grep -q "migration" <<< "$groups"; then
> > 
> > What about the '-F' that find_word has?
> 
> "migration" is only one string without regular expressions in it, so I
> assume the -F does not matter here, does it?

You're right. I got carried away at checking equivalence.

Thanks,
drew