[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1.1 1/1] Fix geometry sector calculation
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PATCH 1.1 1/1] Fix geometry sector calculation |
Date: |
Thu, 24 May 2012 15:26:42 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 |
On 24/05/12 15:06, Alexander Graf wrote:
>
>
> On 24.05.2012, at 13:48, Paolo Bonzini <address@hidden> wrote:
>
>> Il 24/05/2012 13:22, Christian Borntraeger ha scritto:
>>> Currently the sector value for the geometry is masked, even if the
>>> user usesa command line parameter that explicitely gives a number.
>>> This breaks dasd devices on s390. A dasd device can have
>>> a physical block size of 4096 (== same for logical block size)
>>> and a typcial geometry of 15 heads and 12 sectors per cyl.
>>> The ibm partition detection relies on a correct geometry
>>> reported by the device. Unfortunately the current code changes
>>> 12 to 8. This would be necessary if the total size is
>>> not a multiple of logical sector size, but for dasd this
>>> is not the case.
>>>
>>> This patch checks the device size and only applies sector
>>> mask if necessary.
>>
>> Rereading the code, I have no idea what the masking is for. Perhaps we
>> can even remove it. However, your patch makes sense, it is safe, and it
>> would be nice to apply it even for 1.1.
>
> I also don't understand why this code is in virtio-blk.c. What does block
> geometry adjustment have to do with virtio?
Indeed,my first version of the patch was just
- blkcfg.sectors = secs & ~s->sector_mask;
+ blkcfg.sectors = secs;
But then I read the commit message of this line, and the idea seems to be that
a given geometry should be able to cover the full capacity of a disk (no half
filled
cylinders)
>From an s390 perspective all cases are fine and I would just remove the
>masking.
I was just trying to keep the behaviour.
Christian