Linux-S390 Archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests
@ 2024-04-06 12:24 Nicholas Piggin
  2024-04-06 12:24 ` [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash Nicholas Piggin
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Nicholas Piggin @ 2024-04-06 12:24 UTC (permalink / raw
  To: Thomas Huth
  Cc: Nicholas Piggin, Janosch Frank, Claudio Imbrenda, Nico Böhr,
	David Hildenbrand, Andrew Jones, linux-s390, kvm

Shellcheck caught what looks like two bugs. May not have been too
noticable because one just seems like it allows TCG to run PV tests
which fails (and might be guarded out later anyway), and the other
might only affect running and individual .pv.bin test with s390x-run
and not unittests runs.

I don't know about running PV tests or have a system with KVM to try,
so these are not tested beyond gitlab CI. That might run some PV tests
with s390x-kvm, but I don't know so please test/review/ack/nack if
possible.

Thanks,
Nick

Nicholas Piggin (2):
  s390x: Fix misspelt variable name in func.bash
  s390x: Fix is_pv check in run script

 s390x/run               | 8 ++++----
 scripts/s390x/func.bash | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash
  2024-04-06 12:24 [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests Nicholas Piggin
@ 2024-04-06 12:24 ` Nicholas Piggin
  2024-04-08 11:59   ` Janosch Frank
  2024-04-06 12:24 ` [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script Nicholas Piggin
  2024-04-08 11:23 ` [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests Janosch Frank
  2 siblings, 1 reply; 14+ messages in thread
From: Nicholas Piggin @ 2024-04-06 12:24 UTC (permalink / raw
  To: Thomas Huth
  Cc: Nicholas Piggin, Janosch Frank, Claudio Imbrenda, Nico Böhr,
	David Hildenbrand, Andrew Jones, linux-s390, kvm

The if statement is intended to run non-migration tests with PV on KVM.
With the misspelling, they are run on KVM or TCG.

Reported-by: shellcheck SC2153
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
Signed-off-by: Nicholas Piggin <npiggin@gmail.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 6c75e89ae..fa47d0191 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' ] || grep -q "migration" <<< "$groups"; then
+	if [ "$accel" = 'tcg' ] || grep -q "migration" <<< "$groups"; then
 		return
 	fi
 	kernel=${kernel%.elf}.pv.bin
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script
  2024-04-06 12:24 [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests Nicholas Piggin
  2024-04-06 12:24 ` [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash Nicholas Piggin
@ 2024-04-06 12:24 ` Nicholas Piggin
  2024-04-08 11:36   ` Claudio Imbrenda
  2024-04-08 11:46   ` Janosch Frank
  2024-04-08 11:23 ` [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests Janosch Frank
  2 siblings, 2 replies; 14+ messages in thread
From: Nicholas Piggin @ 2024-04-06 12:24 UTC (permalink / raw
  To: Thomas Huth
  Cc: Nicholas Piggin, Janosch Frank, Claudio Imbrenda, Nico Böhr,
	David Hildenbrand, Andrew Jones, linux-s390, kvm

Shellcheck reports "is_pv references arguments, but none are ever
passed." and suggests "use is_pv "$@" if function's $1 should mean
script's $1."

The is_pv test does not evaluate to true for .pv.bin file names, only
for _PV suffix test names. The arch_cmd_s390x() function appends
.pv.bin to the file name AND _PV to the test name, so this does not
affect run_tests.sh runs, but it might prevent PV tests from being
run directly with the s390x-run command.

Reported-by: shellcheck SC2119, SC2120
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 s390x/run | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/s390x/run b/s390x/run
index e58fa4af9..34552c274 100755
--- a/s390x/run
+++ b/s390x/run
@@ -21,12 +21,12 @@ is_pv() {
 	return 1
 }
 
-if is_pv && [ "$ACCEL" = "tcg" ]; then
+if is_pv "$@" && [ "$ACCEL" = "tcg" ]; then
 	echo "Protected Virtualization isn't supported under TCG"
 	exit 2
 fi
 
-if is_pv && [ "$MIGRATION" = "yes" ]; then
+if is_pv "$@" && [ "$MIGRATION" = "yes" ]; then
 	echo "Migration isn't supported under Protected Virtualization"
 	exit 2
 fi
@@ -34,12 +34,12 @@ fi
 M='-machine s390-ccw-virtio'
 M+=",accel=$ACCEL$ACCEL_PROPS"
 
-if is_pv; then
+if is_pv "$@"; then
 	M+=",confidential-guest-support=pv0"
 fi
 
 command="$qemu -nodefaults -nographic $M"
-if is_pv; then
+if is_pv "$@"; then
 	command+=" -object s390-pv-guest,id=pv0"
 fi
 command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests
  2024-04-06 12:24 [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests Nicholas Piggin
  2024-04-06 12:24 ` [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash Nicholas Piggin
  2024-04-06 12:24 ` [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script Nicholas Piggin
@ 2024-04-08 11:23 ` Janosch Frank
  2 siblings, 0 replies; 14+ messages in thread
From: Janosch Frank @ 2024-04-08 11:23 UTC (permalink / raw
  To: Nicholas Piggin, Thomas Huth
  Cc: Claudio Imbrenda, Nico Böhr, David Hildenbrand, Andrew Jones,
	linux-s390, kvm

On 4/6/24 14:24, Nicholas Piggin wrote:
> Shellcheck caught what looks like two bugs. May not have been too
> noticable because one just seems like it allows TCG to run PV tests
> which fails (and might be guarded out later anyway), and the other
> might only affect running and individual .pv.bin test with s390x-run
> and not unittests runs.
> 
> I don't know about running PV tests or have a system with KVM to try,
> so these are not tested beyond gitlab CI. That might run some PV tests
> with s390x-kvm, but I don't know so please test/review/ack/nack if
> possible.
> 
> Thanks,
> Nick

Hey Nick, thanks for cleaning this up.
The changes work on my machine and I've thrown them into our CI.

Give me a sec to check the patches itself.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script
  2024-04-06 12:24 ` [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script Nicholas Piggin
@ 2024-04-08 11:36   ` Claudio Imbrenda
  2024-04-10  4:34     ` Nicholas Piggin
  2024-04-08 11:46   ` Janosch Frank
  1 sibling, 1 reply; 14+ messages in thread
From: Claudio Imbrenda @ 2024-04-08 11:36 UTC (permalink / raw
  To: Nicholas Piggin
  Cc: Thomas Huth, Janosch Frank, Nico Böhr, David Hildenbrand,
	Andrew Jones, linux-s390, kvm

On Sat,  6 Apr 2024 22:24:54 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> Shellcheck reports "is_pv references arguments, but none are ever
> passed." and suggests "use is_pv "$@" if function's $1 should mean
> script's $1."
> 
> The is_pv test does not evaluate to true for .pv.bin file names, only
> for _PV suffix test names. The arch_cmd_s390x() function appends
> .pv.bin to the file name AND _PV to the test name, so this does not
> affect run_tests.sh runs, but it might prevent PV tests from being
> run directly with the s390x-run command.
> 
> Reported-by: shellcheck SC2119, SC2120
> Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Fixes: bcedc5a2 ("s390x: run PV guests with confidential guest enabled")

although tbh I would rewrite it to check a variable, something like:

IS_PV=no
[ "${1: -7}" = ".pv.bin" -o "${TESTNAME: -3}" = "_PV" ] && IS_PV=yes

> ---
>  s390x/run | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/s390x/run b/s390x/run
> index e58fa4af9..34552c274 100755
> --- a/s390x/run
> +++ b/s390x/run
> @@ -21,12 +21,12 @@ is_pv() {
>  	return 1
>  }
>  
> -if is_pv && [ "$ACCEL" = "tcg" ]; then
> +if is_pv "$@" && [ "$ACCEL" = "tcg" ]; then

if [ "$IS_PV" = "yes" -a "$ACCEL" = "tcg" ]; then

etc...

>  	echo "Protected Virtualization isn't supported under TCG"
>  	exit 2
>  fi
>  
> -if is_pv && [ "$MIGRATION" = "yes" ]; then
> +if is_pv "$@" && [ "$MIGRATION" = "yes" ]; then
>  	echo "Migration isn't supported under Protected Virtualization"
>  	exit 2
>  fi
> @@ -34,12 +34,12 @@ fi
>  M='-machine s390-ccw-virtio'
>  M+=",accel=$ACCEL$ACCEL_PROPS"
>  
> -if is_pv; then
> +if is_pv "$@"; then
>  	M+=",confidential-guest-support=pv0"
>  fi
>  
>  command="$qemu -nodefaults -nographic $M"
> -if is_pv; then
> +if is_pv "$@"; then
>  	command+=" -object s390-pv-guest,id=pv0"
>  fi
>  command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script
  2024-04-06 12:24 ` [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script Nicholas Piggin
  2024-04-08 11:36   ` Claudio Imbrenda
@ 2024-04-08 11:46   ` Janosch Frank
  2024-04-10  4:34     ` Nicholas Piggin
  1 sibling, 1 reply; 14+ messages in thread
From: Janosch Frank @ 2024-04-08 11:46 UTC (permalink / raw
  To: Nicholas Piggin, Thomas Huth
  Cc: Claudio Imbrenda, Nico Böhr, David Hildenbrand, Andrew Jones,
	linux-s390, kvm

On 4/6/24 14:24, Nicholas Piggin wrote:
> Shellcheck reports "is_pv references arguments, but none are ever
> passed." and suggests "use is_pv "$@" if function's $1 should mean
> script's $1."
> 
> The is_pv test does not evaluate to true for .pv.bin file names, only
> for _PV suffix test names. The arch_cmd_s390x() function appends
> .pv.bin to the file name AND _PV to the test name, so this does not
> affect run_tests.sh runs, but it might prevent PV tests from being
> run directly with the s390x-run command.
> 

The only thing that changes with this patch is that we get the error 
message from s390x/run and not from QEMU which complains about the 
unpack facility (needed for PV) not being available (because TCG does 
not implement it).
And that's likely why we never ran into any problems.


Patch looks fine to me:
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash
  2024-04-06 12:24 ` [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash Nicholas Piggin
@ 2024-04-08 11:59   ` Janosch Frank
  2024-04-10  4:35     ` Nicholas Piggin
  0 siblings, 1 reply; 14+ messages in thread
From: Janosch Frank @ 2024-04-08 11:59 UTC (permalink / raw
  To: Nicholas Piggin, Thomas Huth
  Cc: Claudio Imbrenda, Nico Böhr, David Hildenbrand, Andrew Jones,
	linux-s390, kvm

On 4/6/24 14:24, Nicholas Piggin wrote:
> The if statement is intended to run non-migration tests with PV on KVM.
> With the misspelling, they are run on KVM or TCG.
> 

It's not misspelt, is it?
It's in the wrong case.


I'm fine with the code though.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script
  2024-04-08 11:36   ` Claudio Imbrenda
@ 2024-04-10  4:34     ` Nicholas Piggin
  2024-04-10 17:18       ` Claudio Imbrenda
  0 siblings, 1 reply; 14+ messages in thread
From: Nicholas Piggin @ 2024-04-10  4:34 UTC (permalink / raw
  To: Claudio Imbrenda
  Cc: Thomas Huth, Janosch Frank, Nico Böhr, David Hildenbrand,
	Andrew Jones, linux-s390, kvm

On Mon Apr 8, 2024 at 9:36 PM AEST, Claudio Imbrenda wrote:
> On Sat,  6 Apr 2024 22:24:54 +1000
> Nicholas Piggin <npiggin@gmail.com> wrote:
>
> > Shellcheck reports "is_pv references arguments, but none are ever
> > passed." and suggests "use is_pv "$@" if function's $1 should mean
> > script's $1."
> > 
> > The is_pv test does not evaluate to true for .pv.bin file names, only
> > for _PV suffix test names. The arch_cmd_s390x() function appends
> > .pv.bin to the file name AND _PV to the test name, so this does not
> > affect run_tests.sh runs, but it might prevent PV tests from being
> > run directly with the s390x-run command.
> > 
> > Reported-by: shellcheck SC2119, SC2120
> > Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Fixes: bcedc5a2 ("s390x: run PV guests with confidential guest enabled")

Thanks.

> although tbh I would rewrite it to check a variable, something like:
>
> IS_PV=no
> [ "${1: -7}" = ".pv.bin" -o "${TESTNAME: -3}" = "_PV" ] && IS_PV=yes

I don't have a problem if you want to fix it a different way
instead. I don't have a good way to test at the moment and
this seemed the simplest fix. Shout out if you don't want it
going upstream as is.

Thanks,
Nick

>
> > ---
> >  s390x/run | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/s390x/run b/s390x/run
> > index e58fa4af9..34552c274 100755
> > --- a/s390x/run
> > +++ b/s390x/run
> > @@ -21,12 +21,12 @@ is_pv() {
> >  	return 1
> >  }
> >  
> > -if is_pv && [ "$ACCEL" = "tcg" ]; then
> > +if is_pv "$@" && [ "$ACCEL" = "tcg" ]; then
>
> if [ "$IS_PV" = "yes" -a "$ACCEL" = "tcg" ]; then
>
> etc...
>
> >  	echo "Protected Virtualization isn't supported under TCG"
> >  	exit 2
> >  fi
> >  
> > -if is_pv && [ "$MIGRATION" = "yes" ]; then
> > +if is_pv "$@" && [ "$MIGRATION" = "yes" ]; then
> >  	echo "Migration isn't supported under Protected Virtualization"
> >  	exit 2
> >  fi
> > @@ -34,12 +34,12 @@ fi
> >  M='-machine s390-ccw-virtio'
> >  M+=",accel=$ACCEL$ACCEL_PROPS"
> >  
> > -if is_pv; then
> > +if is_pv "$@"; then
> >  	M+=",confidential-guest-support=pv0"
> >  fi
> >  
> >  command="$qemu -nodefaults -nographic $M"
> > -if is_pv; then
> > +if is_pv "$@"; then
> >  	command+=" -object s390-pv-guest,id=pv0"
> >  fi
> >  command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script
  2024-04-08 11:46   ` Janosch Frank
@ 2024-04-10  4:34     ` Nicholas Piggin
  0 siblings, 0 replies; 14+ messages in thread
From: Nicholas Piggin @ 2024-04-10  4:34 UTC (permalink / raw
  To: Janosch Frank, Thomas Huth
  Cc: Claudio Imbrenda, Nico Böhr, David Hildenbrand, Andrew Jones,
	linux-s390, kvm

On Mon Apr 8, 2024 at 9:46 PM AEST, Janosch Frank wrote:
> On 4/6/24 14:24, Nicholas Piggin wrote:
> > Shellcheck reports "is_pv references arguments, but none are ever
> > passed." and suggests "use is_pv "$@" if function's $1 should mean
> > script's $1."
> > 
> > The is_pv test does not evaluate to true for .pv.bin file names, only
> > for _PV suffix test names. The arch_cmd_s390x() function appends
> > .pv.bin to the file name AND _PV to the test name, so this does not
> > affect run_tests.sh runs, but it might prevent PV tests from being
> > run directly with the s390x-run command.
> > 
>
> The only thing that changes with this patch is that we get the error 
> message from s390x/run and not from QEMU which complains about the 
> unpack facility (needed for PV) not being available (because TCG does 
> not implement it).
> And that's likely why we never ran into any problems.

Yeah it did look relatively minor. Thanks for taking a look.

Thanks,
Nick

>
>
> Patch looks fine to me:
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash
  2024-04-08 11:59   ` Janosch Frank
@ 2024-04-10  4:35     ` Nicholas Piggin
  2024-04-11  9:40       ` Janosch Frank
  0 siblings, 1 reply; 14+ messages in thread
From: Nicholas Piggin @ 2024-04-10  4:35 UTC (permalink / raw
  To: Janosch Frank, Thomas Huth
  Cc: Claudio Imbrenda, Nico Böhr, David Hildenbrand, Andrew Jones,
	linux-s390, kvm

On Mon Apr 8, 2024 at 9:59 PM AEST, Janosch Frank wrote:
> On 4/6/24 14:24, Nicholas Piggin wrote:
> > The if statement is intended to run non-migration tests with PV on KVM.
> > With the misspelling, they are run on KVM or TCG.
> > 
>
> It's not misspelt, is it?
> It's in the wrong case.

Yes, that's the right word.

>
>
> I'm fine with the code though.

Thanks, I'll take that as an Acked-by: you

Thanks,
Nick

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script
  2024-04-10  4:34     ` Nicholas Piggin
@ 2024-04-10 17:18       ` Claudio Imbrenda
  0 siblings, 0 replies; 14+ messages in thread
From: Claudio Imbrenda @ 2024-04-10 17:18 UTC (permalink / raw
  To: Nicholas Piggin
  Cc: Thomas Huth, Janosch Frank, Nico Böhr, David Hildenbrand,
	Andrew Jones, linux-s390, kvm

On Wed, 10 Apr 2024 14:34:25 +1000
"Nicholas Piggin" <npiggin@gmail.com> wrote:

> On Mon Apr 8, 2024 at 9:36 PM AEST, Claudio Imbrenda wrote:
> > On Sat,  6 Apr 2024 22:24:54 +1000
> > Nicholas Piggin <npiggin@gmail.com> wrote:
> >  
> > > Shellcheck reports "is_pv references arguments, but none are ever
> > > passed." and suggests "use is_pv "$@" if function's $1 should mean
> > > script's $1."
> > > 
> > > The is_pv test does not evaluate to true for .pv.bin file names, only
> > > for _PV suffix test names. The arch_cmd_s390x() function appends
> > > .pv.bin to the file name AND _PV to the test name, so this does not
> > > affect run_tests.sh runs, but it might prevent PV tests from being
> > > run directly with the s390x-run command.
> > > 
> > > Reported-by: shellcheck SC2119, SC2120
> > > Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>  
> >
> > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > Fixes: bcedc5a2 ("s390x: run PV guests with confidential guest enabled")  
> 
> Thanks.
> 
> > although tbh I would rewrite it to check a variable, something like:
> >
> > IS_PV=no
> > [ "${1: -7}" = ".pv.bin" -o "${TESTNAME: -3}" = "_PV" ] && IS_PV=yes  
> 
> I don't have a problem if you want to fix it a different way
> instead. I don't have a good way to test at the moment and
> this seemed the simplest fix. Shout out if you don't want it
> going upstream as is.

well, I did give you a r-b for the current version of your patch :)

it's not so important, as long as it works correctly 

> 
> Thanks,
> Nick
> 
> >  
> > > ---
> > >  s390x/run | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/s390x/run b/s390x/run
> > > index e58fa4af9..34552c274 100755
> > > --- a/s390x/run
> > > +++ b/s390x/run
> > > @@ -21,12 +21,12 @@ is_pv() {
> > >  	return 1
> > >  }
> > >  
> > > -if is_pv && [ "$ACCEL" = "tcg" ]; then
> > > +if is_pv "$@" && [ "$ACCEL" = "tcg" ]; then  
> >
> > if [ "$IS_PV" = "yes" -a "$ACCEL" = "tcg" ]; then
> >
> > etc...
> >  
> > >  	echo "Protected Virtualization isn't supported under TCG"
> > >  	exit 2
> > >  fi
> > >  
> > > -if is_pv && [ "$MIGRATION" = "yes" ]; then
> > > +if is_pv "$@" && [ "$MIGRATION" = "yes" ]; then
> > >  	echo "Migration isn't supported under Protected Virtualization"
> > >  	exit 2
> > >  fi
> > > @@ -34,12 +34,12 @@ fi
> > >  M='-machine s390-ccw-virtio'
> > >  M+=",accel=$ACCEL$ACCEL_PROPS"
> > >  
> > > -if is_pv; then
> > > +if is_pv "$@"; then
> > >  	M+=",confidential-guest-support=pv0"
> > >  fi
> > >  
> > >  command="$qemu -nodefaults -nographic $M"
> > > -if is_pv; then
> > > +if is_pv "$@"; then
> > >  	command+=" -object s390-pv-guest,id=pv0"
> > >  fi
> > >  command+=" -chardev stdio,id=con0 -device sclpconsole,chardev=con0"  
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash
  2024-04-10  4:35     ` Nicholas Piggin
@ 2024-04-11  9:40       ` Janosch Frank
  2024-04-16  2:29         ` Nicholas Piggin
  0 siblings, 1 reply; 14+ messages in thread
From: Janosch Frank @ 2024-04-11  9:40 UTC (permalink / raw
  To: Nicholas Piggin, Thomas Huth
  Cc: Claudio Imbrenda, Nico Böhr, David Hildenbrand, Andrew Jones,
	linux-s390, kvm

On 4/10/24 06:35, Nicholas Piggin wrote:
> On Mon Apr 8, 2024 at 9:59 PM AEST, Janosch Frank wrote:
>> On 4/6/24 14:24, Nicholas Piggin wrote:
>>> The if statement is intended to run non-migration tests with PV on KVM.
>>> With the misspelling, they are run on KVM or TCG.
>>>
>>
>> It's not misspelt, is it?
>> It's in the wrong case.
> 
> Yes, that's the right word.
> 
>>
>>
>> I'm fine with the code though.
> 
> Thanks, I'll take that as an Acked-by: you

Could you send out a fixed version that I can pick or do you want me to 
fix that up?


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash
  2024-04-11  9:40       ` Janosch Frank
@ 2024-04-16  2:29         ` Nicholas Piggin
  2024-04-18 11:10           ` Janosch Frank
  0 siblings, 1 reply; 14+ messages in thread
From: Nicholas Piggin @ 2024-04-16  2:29 UTC (permalink / raw
  To: Janosch Frank, Thomas Huth
  Cc: Claudio Imbrenda, Nico Böhr, David Hildenbrand, Andrew Jones,
	linux-s390, kvm

On Thu Apr 11, 2024 at 7:40 PM AEST, Janosch Frank wrote:
> On 4/10/24 06:35, Nicholas Piggin wrote:
> > On Mon Apr 8, 2024 at 9:59 PM AEST, Janosch Frank wrote:
> >> On 4/6/24 14:24, Nicholas Piggin wrote:
> >>> The if statement is intended to run non-migration tests with PV on KVM.
> >>> With the misspelling, they are run on KVM or TCG.
> >>>
> >>
> >> It's not misspelt, is it?
> >> It's in the wrong case.
> > 
> > Yes, that's the right word.
> > 
> >>
> >>
> >> I'm fine with the code though.
> > 
> > Thanks, I'll take that as an Acked-by: you
>
> Could you send out a fixed version that I can pick or do you want me to 
> fix that up?

I was going to at some point, but was juggling a bunch of other
things and have some travel and vacation. You are welcome to take
over them if you like it would be helpful.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash
  2024-04-16  2:29         ` Nicholas Piggin
@ 2024-04-18 11:10           ` Janosch Frank
  0 siblings, 0 replies; 14+ messages in thread
From: Janosch Frank @ 2024-04-18 11:10 UTC (permalink / raw
  To: Nicholas Piggin, Thomas Huth
  Cc: Claudio Imbrenda, Nico Böhr, David Hildenbrand, Andrew Jones,
	linux-s390, kvm

On 4/16/24 04:29, Nicholas Piggin wrote:
> On Thu Apr 11, 2024 at 7:40 PM AEST, Janosch Frank wrote:
>> On 4/10/24 06:35, Nicholas Piggin wrote:
>>> On Mon Apr 8, 2024 at 9:59 PM AEST, Janosch Frank wrote:
>>>> On 4/6/24 14:24, Nicholas Piggin wrote:
>>>>> The if statement is intended to run non-migration tests with PV on KVM.
>>>>> With the misspelling, they are run on KVM or TCG.
>>>>>
>>>>
>>>> It's not misspelt, is it?
>>>> It's in the wrong case.
>>>
>>> Yes, that's the right word.
>>>
>>>>
>>>>
>>>> I'm fine with the code though.
>>>
>>> Thanks, I'll take that as an Acked-by: you
>>
>> Could you send out a fixed version that I can pick or do you want me to
>> fix that up?
> 
> I was going to at some point, but was juggling a bunch of other
> things and have some travel and vacation. You are welcome to take
> over them if you like it would be helpful.
> 

Same here, I'll be on vacation starting next week so I've prepared a 
branch with your patches for Nico and Claudio which they can send out 
when I'm on vacation.

https://gitlab.com/frankja/kvm-unit-tests/-/commits/queue/?ref_type=heads

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-04-18 11:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-06 12:24 [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests Nicholas Piggin
2024-04-06 12:24 ` [kvm-unit-tests PATCH 1/2] s390x: Fix misspelt variable name in func.bash Nicholas Piggin
2024-04-08 11:59   ` Janosch Frank
2024-04-10  4:35     ` Nicholas Piggin
2024-04-11  9:40       ` Janosch Frank
2024-04-16  2:29         ` Nicholas Piggin
2024-04-18 11:10           ` Janosch Frank
2024-04-06 12:24 ` [kvm-unit-tests PATCH 2/2] s390x: Fix is_pv check in run script Nicholas Piggin
2024-04-08 11:36   ` Claudio Imbrenda
2024-04-10  4:34     ` Nicholas Piggin
2024-04-10 17:18       ` Claudio Imbrenda
2024-04-08 11:46   ` Janosch Frank
2024-04-10  4:34     ` Nicholas Piggin
2024-04-08 11:23 ` [kvm-unit-tests PATCH 0/2] s390x: run script fixes for PV tests Janosch Frank

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).