[kvm-unit-tests PATCH 0/3] scripts: Fix PV run handling

[kvm-unit-tests PATCH 0/3] scripts: Fix PV run handling

[Janosch Frank]

There are some issues that make our current PV handling in
run_tests.sh and s390x/run a bit awkward:

 * With ACCEL=tcg or if KVM is not available we will try to run PV
   tests which won't work
 * If a host key cert has been specified but it does not exists then
   the compilation will break

This series is based on the common script fixes I just sent out.

Janosch Frank (3):
  s390x: Don't run PV testcases under tcg
  configure: s390x: Check if the host key document exists
  s390x: run: Skip PV tests when tcg is the accelerator

 configure               | 5 +++++
 s390x/run               | 5 +++++
 scripts/s390x/func.bash | 3 +++
 3 files changed, 13 insertions(+)

-- 
2.27.0

[kvm-unit-tests PATCH 1/3] s390x: Don't run PV testcases under tcg

[Janosch Frank]

The UV call facility is only available on hardware.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 scripts/s390x/func.bash | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
index b3912081..bf799a56 100644
--- a/scripts/s390x/func.bash
+++ b/scripts/s390x/func.bash
@@ -21,6 +21,9 @@ function arch_cmd_s390x()
 	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
 
 	# run PV test case
+	if [ "$ACCEL" = 'tcg' ]; then
+		return
+	fi
 	kernel=${kernel%.elf}.pv.bin
 	testname=${testname}_PV
 	if [ ! -f "${kernel}" ]; then
-- 
2.27.0

Re: [kvm-unit-tests PATCH 1/3] s390x: Don't run PV testcases under tcg

[Thomas Huth]

On 18/03/2021 13.50, Janosch Frank wrote:
> The UV call facility is only available on hardware.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   scripts/s390x/func.bash | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
> index b3912081..bf799a56 100644
> --- a/scripts/s390x/func.bash
> +++ b/scripts/s390x/func.bash
> @@ -21,6 +21,9 @@ function arch_cmd_s390x()
>   	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>   
>   	# run PV test case
> +	if [ "$ACCEL" = 'tcg' ]; then
> +		return
> +	fi
>   	kernel=${kernel%.elf}.pv.bin
>   	testname=${testname}_PV
>   	if [ ! -f "${kernel}" ]; then
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH 1/3] s390x: Don't run PV testcases under tcg

[David Hildenbrand]

On 18.03.21 13:50, Janosch Frank wrote:
> The UV call facility is only available on hardware.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   scripts/s390x/func.bash | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/s390x/func.bash b/scripts/s390x/func.bash
> index b3912081..bf799a56 100644
> --- a/scripts/s390x/func.bash
> +++ b/scripts/s390x/func.bash
> @@ -21,6 +21,9 @@ function arch_cmd_s390x()
>   	"$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
>   
>   	# run PV test case
> +	if [ "$ACCEL" = 'tcg' ]; then
> +		return
> +	fi
>   	kernel=${kernel%.elf}.pv.bin
>   	testname=${testname}_PV
>   	if [ ! -f "${kernel}" ]; then
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

[kvm-unit-tests PATCH 2/3] configure: s390x: Check if the host key document exists

[Janosch Frank]

I'd rather have a readable error message than make failing the build
with cryptic errors.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 configure | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/configure b/configure
index cdcd34e9..4d4bb646 100755
--- a/configure
+++ b/configure
@@ -121,6 +121,11 @@ while [[ "$1" = -* ]]; do
     esac
 done
 
+if [ "$host_key_document" ] && [ ! -f "$host_key_document" ]; then
+    echo "Host key document doesn't exist at the specified location."
+    exit 1
+fi
+
 if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
     echo "erratatxt: $erratatxt does not exist or is not a regular file"
     exit 1
-- 
2.27.0

Re: [kvm-unit-tests PATCH 2/3] configure: s390x: Check if the host key document exists

[Thomas Huth]

On 18/03/2021 13.50, Janosch Frank wrote:
> I'd rather have a readable error message than make failing the build
> with cryptic errors.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   configure | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/configure b/configure
> index cdcd34e9..4d4bb646 100755
> --- a/configure
> +++ b/configure
> @@ -121,6 +121,11 @@ while [[ "$1" = -* ]]; do
>       esac
>   done
>   
> +if [ "$host_key_document" ] && [ ! -f "$host_key_document" ]; then

Use [ -n "$host_key_document" ] instead of just
  [ "$host_key_document" ] ?

With that fixed:

Reviewed-by: Thomas Huth <thuth@redhat.com>


> +    echo "Host key document doesn't exist at the specified location."
> +    exit 1
> +fi
> +
>   if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
>       echo "erratatxt: $erratatxt does not exist or is not a regular file"
>       exit 1
> 

Re: [kvm-unit-tests PATCH 2/3] configure: s390x: Check if the host key document exists

[David Hildenbrand]

On 07.06.21 11:48, Thomas Huth wrote:
> On 18/03/2021 13.50, Janosch Frank wrote:
>> I'd rather have a readable error message than make failing the build
>> with cryptic errors.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>    configure | 5 +++++
>>    1 file changed, 5 insertions(+)
>>
>> diff --git a/configure b/configure
>> index cdcd34e9..4d4bb646 100755
>> --- a/configure
>> +++ b/configure
>> @@ -121,6 +121,11 @@ while [[ "$1" = -* ]]; do
>>        esac
>>    done
>>    
>> +if [ "$host_key_document" ] && [ ! -f "$host_key_document" ]; then
> 
> Use [ -n "$host_key_document" ] instead of just
>    [ "$host_key_document" ] ?
> 
> With that fixed:
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
> 
>> +    echo "Host key document doesn't exist at the specified location."
>> +    exit 1
>> +fi
>> +
>>    if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
^should we adjust that then as well?


-- 
Thanks,

David / dhildenb

[kvm-unit-tests PATCH 3/3] s390x: run: Skip PV tests when tcg is the accelerator

[Janosch Frank]

TCG doesn't support PV.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/run | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/s390x/run b/s390x/run
index df7ef5ca..82922701 100755
--- a/s390x/run
+++ b/s390x/run
@@ -19,6 +19,11 @@ else
     ACCEL=$DEF_ACCEL
 fi
 
+if [ "${1: -7}" == ".pv.bin" ] || [ "${TESTNAME: -3}" == "_PV" ] && [ $ACCEL == "tcg" ]; then
+	echo "Protected Virtualization isn't supported under TCG"
+	exit 2
+fi
+
 M='-machine s390-ccw-virtio'
 M+=",accel=$ACCEL"
 command="$qemu -nodefaults -nographic $M"
-- 
2.27.0

Re: [kvm-unit-tests PATCH 3/3] s390x: run: Skip PV tests when tcg is the accelerator

[Thomas Huth]

On 18/03/2021 13.50, Janosch Frank wrote:
> TCG doesn't support PV.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   s390x/run | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/s390x/run b/s390x/run
> index df7ef5ca..82922701 100755
> --- a/s390x/run
> +++ b/s390x/run
> @@ -19,6 +19,11 @@ else
>       ACCEL=$DEF_ACCEL
>   fi
>   
> +if [ "${1: -7}" == ".pv.bin" ] || [ "${TESTNAME: -3}" == "_PV" ] && [ $ACCEL == "tcg" ]; then

Put $ACCEL in quotes?

With that nit fixed:

Reviewed-by: Thomas Huth <thuth@redhat.com>


> +	echo "Protected Virtualization isn't supported under TCG"
> +	exit 2
> +fi
> +
>   M='-machine s390-ccw-virtio'
>   M+=",accel=$ACCEL"
>   command="$qemu -nodefaults -nographic $M"
> 

Re: [kvm-unit-tests PATCH 3/3] s390x: run: Skip PV tests when tcg is the accelerator

[David Hildenbrand]

On 07.06.21 11:54, Thomas Huth wrote:
> On 18/03/2021 13.50, Janosch Frank wrote:
>> TCG doesn't support PV.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>    s390x/run | 5 +++++
>>    1 file changed, 5 insertions(+)
>>
>> diff --git a/s390x/run b/s390x/run
>> index df7ef5ca..82922701 100755
>> --- a/s390x/run
>> +++ b/s390x/run
>> @@ -19,6 +19,11 @@ else
>>        ACCEL=$DEF_ACCEL
>>    fi
>>    
>> +if [ "${1: -7}" == ".pv.bin" ] || [ "${TESTNAME: -3}" == "_PV" ] && [ $ACCEL == "tcg" ]; then
> 
> Put $ACCEL in quotes?
> 
> With that nit fixed:
> 

Should these "==" be "=" ? Bash string comparisons always mess with my mind.


-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests PATCH 3/3] s390x: run: Skip PV tests when tcg is the accelerator

[Thomas Huth]

On 07/06/2021 11.57, David Hildenbrand wrote:
> On 07.06.21 11:54, Thomas Huth wrote:
>> On 18/03/2021 13.50, Janosch Frank wrote:
>>> TCG doesn't support PV.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>    s390x/run | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/s390x/run b/s390x/run
>>> index df7ef5ca..82922701 100755
>>> --- a/s390x/run
>>> +++ b/s390x/run
>>> @@ -19,6 +19,11 @@ else
>>>        ACCEL=$DEF_ACCEL
>>>    fi
>>> +if [ "${1: -7}" == ".pv.bin" ] || [ "${TESTNAME: -3}" == "_PV" ] && [ 
>>> $ACCEL == "tcg" ]; then
>>
>> Put $ACCEL in quotes?
>>
>> With that nit fixed:
>>
> 
> Should these "==" be "=" ? Bash string comparisons always mess with my mind.

Oh, right. I also always have to check "man test" to get the right answer, 
but "=" is more portable, indeed. (Well, k-u-t are hard-wired to bash, but 
it's better to write clean shell code anyway)

  Thomas