Re: [kvm-unit-tests PATCH 2/6] s390x: uv-host: Add uninitialized UV tests

Janosch Frank
2022-05-16T15:16:12Z

On 5/16/22 17:02, Steffen Eiden wrote:
>
>
> On 5/13/22 11:50, Janosch Frank wrote:
>> Let's also test for rc 0x3
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
> I, however, have some nits below.
>
>> ---
>> s390x/uv-host.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 76 insertions(+), 2 deletions(-)
>>
>> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
>> index 0f0b18a1..f846fc42 100644
>> --- a/s390x/uv-host.c
>> +++ b/s390x/uv-host.c
>> @@ -83,6 +83,24 @@ static void test_priv(void)
>> report_prefix_pop();
>> }
>>
>> +static void test_uv_uninitialized(void)
>> +{
>> + struct uv_cb_header uvcb = {};
>> + int i;
>> +
>> + report_prefix_push("uninitialized");
>> +
>> + /* i = 1 to skip over initialize */
>> + for (i = 1; cmds[i].name; i++) {
>> + expect_pgm_int();
>> + uvcb.cmd = cmds[i].cmd;
>> + uvcb.len = cmds[i].len;
>> + uv_call_once(0, (uint64_t)&uvcb);
>> + report(uvcb.rc == UVC_RC_INV_STATE, "%s", cmds[i].name);
>> + }
>> + report_prefix_pop();
>> +}
>> +
>> static void test_config_destroy(void)
>> {
>> int rc;
>> @@ -477,13 +495,68 @@ static void test_invalid(void)
>> report_prefix_pop();
>> }
>>
>> +static void test_clear_setup(void)
> maybe rename this to setup_test_clear(void)
> I initially mistook this function as a test and not a setup function for
> a test

Sure

>
>> +{
>> + unsigned long vsize;
>> + int rc;
>> +
[...]
>> static void setup_vmem(void)
>> @@ -514,6 +587,7 @@ int main(void)
>>
>> test_priv();
>> test_invalid();
>> + test_uv_uninitialized();
>> test_query();
>> test_init();
> IIRC this test must be done last, as a following test has an
> uninitialized UV. Maybe add a short comment for that here.

You're referring to the test_init()?

The test_clear() function must be done last but you're commenting under
the test_init() call. So I'm a bit confused about what you want me to do
here.

>>
>