On 24.05.2012, at 13:48, Paolo Bonzini <[email protected]> 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 <[email protected]> > > Paolo > >> Signed-off-by: Christian Borntraeger <[email protected]> >> CC: Christoph Hellwig <[email protected]> >> --- >> 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; >
