| Message ID | 20221220175508.57180-1-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,v1,1/1] s390x: fix make standalone | expand |
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>
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>
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 >
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>
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>
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
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> >
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>
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
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
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
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
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
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(-)