On 19.07.19 09:47, Christian Borntraeger wrote:
> The comment is true for all stable versions.
>
> This patch is part of a larger series that enables ESE volumes.
> I think it should not go alone as other patches like
> 5e2b17e712cf s390/dasd: Add dynamic formatting support for ESE volumes
> are needed to actually work with ESE volumes.
> So I suggest to drop this patch.
> Jan, Stefan, do you agree?
>
This patch is a requirement for ESE volumes to work and doesn't
add any value alone. I suggest to drop this patch as well.
Jan
>
>
> On 19.07.19 05:56, Sasha Levin wrote:
>> From: Jan Höppner <hoeppner@xxxxxxxxxxxxx>
>>
>> [ Upstream commit ce6915f5343f5f2a2a937b683d8ffbf12dab3ad4 ]
>>
>> The disk layout and volume information of a DASD reside in the first two
>> tracks of cylinder 0. When a DASD is set online, currently the first
>> three tracks are read and analysed to confirm an expected layout.
>>
>> For CDL (Compatible Disk Layout) only count area data of the first track
>> is evaluated and checked against expected key and data lengths. For LDL
>> (Linux Disk Layout) the first and third track is evaluated. However,
>> an LDL formatted volume is expected to be in the same format across all
>> tracks. Checking the third track therefore doesn't have any more value
>> than checking any other track at random.
>>
>> Now, an Extent Space Efficient (ESE) DASD is initialised by only
>> formatting the first two tracks, as those tracks always contain all
>> information necessarry.
>>
>> Checking the third track on an ESE volume will therefore most likely
>> fail with a record not found error, as the third track will be empty.
>> This in turn leads to the device being recognised with a volume size of
>> 0. Attempts to write volume information on the first two tracks then
>> fail with "no space left on device" errors.
>>
>> Initialising the first three tracks for an ESE volume is not a viable
>> solution, because the third track is already a regular track and could
>> contain user data. With that there is potential for data corruption.
>>
>> Instead, always only analyse the first two tracks, as it is sufficiant
>> for both CDL and LDL, and allow ESE volumes to be recognised as well.
>>
>> Signed-off-by: Jan Höppner <hoeppner@xxxxxxxxxxxxx>
>> Reviewed-by: Stefan Haberland <sth@xxxxxxxxxxxxx>
>> Signed-off-by: Vasily Gorbik <gor@xxxxxxxxxxxxx>
>> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
>> ---
>> drivers/s390/block/dasd_eckd.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
>> index c09039eea707..c7aec1b44b7c 100644
>> --- a/drivers/s390/block/dasd_eckd.c
>> +++ b/drivers/s390/block/dasd_eckd.c
>> @@ -157,7 +157,7 @@ static const int sizes_trk0[] = { 28, 148, 84 };
>> #define LABEL_SIZE 140
>>
>> /* head and record addresses of count_area read in analysis ccw */
>> -static const int count_area_head[] = { 0, 0, 0, 0, 2 };
>> +static const int count_area_head[] = { 0, 0, 0, 0, 1 };
>> static const int count_area_rec[] = { 1, 2, 3, 4, 1 };
>>
>> static inline unsigned int
>> @@ -1823,8 +1823,8 @@ dasd_eckd_analysis_ccw(struct dasd_device *device)
>> if (IS_ERR(cqr))
>> return cqr;
>> ccw = cqr->cpaddr;
>> - /* Define extent for the first 3 tracks. */
>> - define_extent(ccw++, cqr->data, 0, 2,
>> + /* Define extent for the first 2 tracks. */
>> + define_extent(ccw++, cqr->data, 0, 1,
>> DASD_ECKD_CCW_READ_COUNT, device, 0);
>> LO_data = cqr->data + sizeof(struct DE_eckd_data);
>> /* Locate record for the first 4 records on track 0. */
>> @@ -1843,9 +1843,9 @@ dasd_eckd_analysis_ccw(struct dasd_device *device)
>> count_data++;
>> }
>>
>> - /* Locate record for the first record on track 2. */
>> + /* Locate record for the first record on track 1. */
>> ccw[-1].flags |= CCW_FLAG_CC;
>> - locate_record(ccw++, LO_data++, 2, 0, 1,
>> + locate_record(ccw++, LO_data++, 1, 0, 1,
>> DASD_ECKD_CCW_READ_COUNT, device, 0);
>> /* Read count ccw. */
>> ccw[-1].flags |= CCW_FLAG_CC;
>> @@ -1967,7 +1967,7 @@ static int dasd_eckd_end_analysis(struct dasd_block *block)
>> }
>> }
>> if (i == 3)
>> - count_area = &private->count_area[4];
>> + count_area = &private->count_area[3];
>>
>> if (private->uses_cdl == 0) {
>> for (i = 0; i < 5; i++) {
>>
