[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 03/10] pc-bios/s390-ccw: handle different sector
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PULL 03/10] pc-bios/s390-ccw: handle different sector sizes |
Date: |
Fri, 27 Jun 2014 15:11:28 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 |
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
- [Qemu-devel] [PULL 00/10] for-2.1: s390-ccw bios patches, Cornelia Huck, 2014/06/27
- [Qemu-devel] [PULL 02/10] pc-bios/s390-ccw: cleanup and enhance bootmap defintions, Cornelia Huck, 2014/06/27
- [Qemu-devel] [PULL 04/10] pc-bios/s390-ccw: add some utility code, Cornelia Huck, 2014/06/27
- [Qemu-devel] [PULL 05/10] pc-bios/s390-ccw: Unify error handling, Cornelia Huck, 2014/06/27
- [Qemu-devel] [PULL 01/10] pc-bios/s390-ccw: make checkpatch happy, Cornelia Huck, 2014/06/27
- [Qemu-devel] [PULL 03/10] pc-bios/s390-ccw: handle different sector sizes, Cornelia Huck, 2014/06/27
- Re: [Qemu-devel] [PULL 03/10] pc-bios/s390-ccw: handle different sector sizes, Eugene \"jno\" Dvurechenski, 2014/06/27
[Qemu-devel] [PULL 07/10] pc-bios/s390-ccw: factor out ipl code, Cornelia Huck, 2014/06/27
[Qemu-devel] [PULL 09/10] pc-bios/s390-ccw: IPL from LDL/CMS-formatted ECKD DASD, Cornelia Huck, 2014/06/27
[Qemu-devel] [PULL 06/10] pc-bios/s390-ccw: Add fill_hex_val func to provide better msgs, Cornelia Huck, 2014/06/27
[Qemu-devel] [PULL 10/10] pc-bios/s390-ccw: update binary, Cornelia Huck, 2014/06/27
[Qemu-devel] [PULL 08/10] pc-bios/s390-ccw: IPL from CDL-formatted ECKD DASD, Cornelia Huck, 2014/06/27
Re: [Qemu-devel] [PULL 00/10] for-2.1: s390-ccw bios patches, Peter Maydell, 2014/06/29