[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: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH 1.1 1/1] Fix geometry sector calculation |
Date: |
Thu, 24 May 2012 15:06:19 +0200 |
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?
Alex
>
> Reviewed-by: Paolo Bonzini <address@hidden>
>
> Paolo
>
>> Signed-off-by: Christian Borntraeger <address@hidden>
>> CC: Christoph Hellwig <address@hidden>
>> ---
>> hw/virtio-blk.c | 17 ++++++++++++++++-
>> 1 files changed, 16 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
>> index f9e1896..41c5bae 100644
>> --- a/hw/virtio-blk.c
>> +++ b/hw/virtio-blk.c
>> @@ -489,7 +489,22 @@ static void virtio_blk_update_config(VirtIODevice
>> *vdev, uint8_t *config)
>> stw_raw(&blkcfg.min_io_size, s->conf->min_io_size / blk_size);
>> stw_raw(&blkcfg.opt_io_size, s->conf->opt_io_size / blk_size);
>> blkcfg.heads = heads;
>> - blkcfg.sectors = secs & ~s->sector_mask;
>> + /*
>> + * We must ensure that the block device capacity is a multiple of
>> + * the logical block size. If that is not the case, lets use
>> + * sector_mask to adopt the geometry to have a correct picture.
>> + * For those devices where the capacity is ok for the given geometry
>> + * we dont touch the sector value of the geometry, since some devices
>> + * (like s390 dasd) need a specific value. Here the capacity is already
>> + * cyls*heads*secs*blz_size and the sector value is not block size
>> + * divided by 512 - instead it is the amount of blk_size blocks
>> + * per track (cylinder).
>> + */
>> + if (bdrv_getlength(s->bs) / heads / secs % blk_size) {
>> + blkcfg.sectors = secs & ~s->sector_mask;
>> + } else {
>> + blkcfg.sectors = secs;
>> + }
>> blkcfg.size_max = 0;
>> blkcfg.physical_block_exp = get_physical_block_exp(s->conf);
>> blkcfg.alignment_offset = 0;
>