On Thu, May 24, 2012 at 2:26 PM, Christian Borntraeger <[email protected]> wrote: > On 24/05/12 15:06, Alexander Graf wrote: >> >> >> 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? > > 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)
Yes, this is how I interpret it as well. We also perform checks on individual virtio-blk requests to make sure they meet the block size - this ensures we don't violate alignment requirements. Stefan
