Re: [Qemu-devel] [PULL 03/10] pc-bios/s390-ccw: handle different sector sizes

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 27/06/14 15:04, Eugene "jno" Dvurechenski wrote:
> 
> 
> On 06/27/2014 03:55 PM, Christian Borntraeger wrote:
>>>> -    const int max_entries = (SECTOR_SIZE / sizeof(ScsiBlockPtr));
>>>> +    const int max_entries = (MAX_SECTOR_SIZE / sizeof(ScsiBlockPtr));
>>>
>>> Is this really safe to increase? Doesn't max_entries depend on the real sector size?
>>
>> I think this is now covered by this if statement:
>>             if (bprs[i].blockct == 0 && unused_space(&bprs[i + 1],
>>                 sizeof(ScsiBlockPtr))) {
>>
>> which was introduced by commit c77cd87cf54f003748f29c14ea1ddaecfc5c653f (pc-bios/s390-ccw: fix for fragmented SCSI bootmap).
>>
>> So strictly speaking this if statement might not be needed any more:
>>         if (i == (max_entries - 1)) {
>>
>> Eugene, can you confirm?  If yes we could add this patch later on as a cleanup:
> 
> I'd preserve both checks.
> In theory, we may catch a table that consumes all scratch space and
> leave no unused entry.
> 
> Plus, this check for zero counter and last entry is for "continuation"
> pointer, not for end-of-table by itself.
> 
> I think now, this code may need even few more checks to cover more cases...
> 
Ok. That means, that this patch as is, doesnt make anything worse. Correct?

I am expecting more fixes and cleanups for the bios code anyway, so as long as we dont add a regression here this should be good to go as it makes the whole code more flexible.

Christian