Re: [kvm-unit-tests PATCH v8 6/6] s390x: SCLP unit test

David Hildenbrand
2020-01-22T09:55:36Z

[...]

> all: directories test_cases test_cases_binary
> diff --git a/s390x/sclp.c b/s390x/sclp.c
> new file mode 100644
> index 0000000..215347e
> --- /dev/null
> +++ b/s390x/sclp.c
> @@ -0,0 +1,474 @@
> +/*
> + * Service Call tests
> + *
> + * Copyright (c) 2019 IBM Corp

Should be 2020 now. Will fix that up.

[...]

> +/**
> + * Perform one test at the given address, optionally using the SCCB template,
> + * checking for the expected program interrupts and return codes.
> + *
> + * The parameter buf_len indicates the number of bytes of the template that
> + * should be copied to the test address, and should be 0 when the test
> + * address is invalid, in which case nothing is copied.
> + *
> + * The template is used to simplify tests where the same buffer content is
> + * used many times in a row, at different addresses.
> + *
> + * Returns true in case of success or false in case of failure
> + */
> +static bool test_one_sccb(uint32_t cmd, uint8_t *addr, uint16_t buf_len, uint64_t exp_pgm, uint16_t exp_rc)
> +{
> + SCCBHeader *h = (SCCBHeader *)addr;
> + int res, pgm;
> +
> + /* Copy the template to the test address if needed */
> + if (buf_len)
> + memcpy(addr, sccb_template, buf_len);
> + if (exp_pgm != PGM_NONE)
> + expect_pgm_int();

I still dislike the handling here. Any caller can simply do a

expect_pgm_int();
test_one_sccb(...)
check_pgm_int_code(PGM_);

Same thing goes for exp_rc. We really should drop exp_rc and exp_pgm
handling from the function completely and handle it in the caller. That
makes the tests actually readable.

But I feel like I am repeating myself. I'll queue this patch with the
2020 fixed up and might send a fixup myself (if I find some spare minutes).

--
Thanks,

David / dhildenb