Re: [kvm-unit-tests PATCH v8 10/12] s390x: css: stsch, enumeration test

Pierre Morel
2020-06-09T12:20:42Z

On 2020-06-09 09:39, Thomas Huth wrote:
> On 08/06/2020 10.12, Pierre Morel wrote:

...snip...

>> +int css_enumerate(void)
>> +{
>> + struct pmcw *pmcw = &schib.pmcw;
>> + int scn_found = 0;
>> + int dev_found = 0;
>> + int schid = 0;
>> + int cc;
>> + int scn;
>> +
>> + for (scn = 0; scn < 0xffff; scn++) {
>> + cc = stsch(scn | SCHID_ONE, &schib);
>> + switch (cc) {
>> + case 0: /* 0 means SCHIB stored */
>> + break;
>> + case 3: /* 3 means no more channels */
>> + goto out;
>> + default: /* 1 or 2 should never happened for STSCH */
>> + report_info("Unexpected error %d on subchannel %08x",
>> + cc, scn | SCHID_ONE);
>
> Should this maybe even be a report_abort() instead? Or leave the error
> reporting to the caller...

report_abort() should be fine as if this happens something is really broken.

>
>> + return cc;
>> + }

...snip...

>> +static void test_enumerate(void)
>> +{
>> + test_device_sid = css_enumerate();
>> + if (test_device_sid & SCHID_ONE) {
>> + report(1, "First device schid: 0x%08x", test_device_sid);
>> + return;
>> + }
>> +
>> + switch (test_device_sid) {
>> + case 0:
>> + report (0, "No I/O device found");
>> + break;
>> + default: /* 1 or 2 should never happened for STSCH */
>> + report(0, "Unexpected cc=%d during enumeration",
>> + test_device_sid);
>> + return;
>> + }
>
> Ok, so here is now the test failure for the cc=1 or 2 that should never
> happen. That means currently you print out the CC for this error twice.
> One time should be enough, either here, or use an report_abort() in the
> css_enumerate(), I'd say.
>
> Anyway, can you please replace this switch statement with a "if
> (!test_device_sid)" instead? Or do you plan to add more "case"
> statements later?

I will use the repor_abort() in the css_enumerate() so there
is only two case, I find a channel or not, so I don't even need the
second if :) .

Thanks,
Pierre

--
Pierre Morel
IBM Lab Boeblingen