Re: [kvm-unit-tests PATCH v2 3/4] s390x: uv-guest: remove duplicated checks

[Steffen Eiden <seiden@linux.ibm.com>]

On 2/3/22 17:31, Claudio Imbrenda wrote:
> On Thu,  3 Feb 2022 09:19:34 +0000
> Steffen Eiden <seiden@linux.ibm.com> wrote:
> 
>> Removing some tests which are done at other points in the code
>> implicitly.

[...]
>>
>> diff --git a/s390x/uv-guest.c b/s390x/uv-guest.c
>> index 44ad2154..97ae4687 100644
>> --- a/s390x/uv-guest.c
>> +++ b/s390x/uv-guest.c
>> @@ -69,23 +69,15 @@ static void test_query(void)
>>   	cc = uv_call(0, (u64)&uvcb);
>>   	report(cc == 1 && uvcb.header.rc == UVC_RC_INV_LEN, "length");
>>   
>> -	uvcb.header.len = sizeof(uvcb);
>> -	cc = uv_call(0, (u64)&uvcb);
>> -	report((!cc && uvcb.header.rc == UVC_RC_EXECUTED) ||
>> -	       (cc == 1 && uvcb.header.rc == 0x100),
>> -		"successful query");
>> -
> 
> ok fair enough, an unsuccessful query would have caused an assert in
> the setup code, but I don't think it hurts, and I think it would be
> nice to have for completeness.
>
Janosch explicitly asked me to remove this while I am editing uv_guest.

[...]

> 
> also, what happens if only one of the two bits is set? (which is very
> wrong). In that scenario, I would like this test to fail, not skip.
> this means that we can't rely on uv_os_is_guest to decide whether to
> skip this test.
>
That is true and a test if both bits are present xor none would be a 
great addition. However, if just one bit is set, uv_os_is_guest would
return false and this part will never be reached anyway.

I can add a test before the uv_os_is_guest fence to verify that both
xor none SHARED flags are set.

>>   	 */
>> -	report(test_bit_inv(BIT_UVC_CMD_QUI, &uvcb.inst_calls_list[0]),
>> -	       "query indicated");
>> -	report(test_bit_inv(BIT_UVC_CMD_SET_SHARED_ACCESS, &uvcb.inst_calls_list[0]),
>> -	       "share indicated");
>> -	report(test_bit_inv(BIT_UVC_CMD_REMOVE_SHARED_ACCESS, &uvcb.inst_calls_list[0]),
>> -	       "unshare indicated");
>> +	report(uv_query_test_call(BIT_UVC_CMD_QUI), "query indicated");
>>   	report_prefix_pop();
>>   }
>>   
> 

Steffen