Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH

[Halil Pasic]

On 08/31/2017 07:51 AM, Thomas Huth wrote:
> On 30.08.2017 18:36, Halil Pasic wrote:
>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to
>> present cc 1 and the other way around, because css_do_xsch has the error
>> codes mixed up. Fixing the error codes also fixes the priority.
>>
>> Let us fix this.
> 
> (Nit: In case you respin, I'd suggest to remove the last sentence. You
> already used "fix" two times in the previous one)
> 
>> Signed-off-by: Halil Pasic <address@hidden>
>> Reported-by: Pierre Morel<address@hidden>
> 
> Space missing -------------^
> 

copy-paste :(

>> ---
>>  hw/s390x/css.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index 1880b1a0ff..a50fb0727e 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)
>>          (!(s->ctrl &
>>             (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | 
>> SCSW_ACTL_SUSP))) ||
>>          (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {
>> -        ret = -EINPROGRESS;
>> +        ret = -EBUSY;
>>          goto out;
>>      }
>>  
>>      if (s->ctrl & SCSW_CTRL_MASK_STCTL) {
>> -        ret = -EBUSY;
>> +        ret = -EINPROGRESS;
>>          goto out;
>>      }
> 
> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing
> to me here ... what's the difference between busy and in-progress? So
> while you're at it, maybe you could replace the code for CC 2 ("CANCEL
> SUBCHANNEL not applicable") with a different error code?
> 
>  Thomas
> 

Well, the idea of the series is to get rid of these artificial error codes,
so your concern of using EBUSY and EINPROGRESS will be dealt with in patch
5.

The idea was to first do the fixes and then do the transformation without
changing behavior.

Thanks for having a look!

Regards,

Halil