From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NrSzs-0004Nt-Et for qemu-devel@nongnu.org; Tue, 16 Mar 2010 05:23:32 -0400 Received: from [199.232.76.173] (port=47754 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NrSzq-0004N7-HE for qemu-devel@nongnu.org; Tue, 16 Mar 2010 05:23:30 -0400 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NrSzn-0007En-Tw for qemu-devel@nongnu.org; Tue, 16 Mar 2010 05:23:30 -0400 Received: from mx20.gnu.org ([199.232.41.8]:47306) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NrSzn-0007Dz-Gy for qemu-devel@nongnu.org; Tue, 16 Mar 2010 05:23:27 -0400 Received: from verein.lst.de ([213.95.11.210]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NrSzk-0001iq-RD for qemu-devel@nongnu.org; Tue, 16 Mar 2010 05:23:25 -0400 Date: Tue, 16 Mar 2010 10:23:16 +0100 From: Christoph Hellwig Subject: Re: [Qemu-devel] [PATCH] block: add logical_block_size property Message-ID: <20100316092316.GA21432@lst.de> References: <20100304132017.GA2200@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100304132017.GA2200@lst.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Christian Borntraeger ping? On Thu, Mar 04, 2010 at 02:20:17PM +0100, Christoph Hellwig wrote: > > Add a logical block size attribute as various guest side tools only > increase the filesystem sector size based on it, not the advisory > physical block size. > > For scsi we already have support for a different logical block size > in place for CDROMs that we can built upon. Only my recent block > device characteristics VPD page needs some fixups. Note that we > leave the logial block size for CDROMs hardcoded as the 2k value > is expected for it in general. > > For virtio-blk we already have a feature flag claiming to support > a variable logical block size that was added for the s390 kuli > hypervisor. Interestingly it does not actually change the units > in which the protocol works, which is still fixed at 512 bytes, > but only communicates a different minimum I/O granularity. So > all we need to do in virtio is to add a trap for unaligned I/O > and round down the device size to the next multiple of the logical > block size. > > IDE does not support any other logical block size than 512 bytes. > > Signed-off-by: Christoph Hellwig > > Index: qemu/block_int.h > =================================================================== > --- qemu.orig/block_int.h 2010-03-03 19:16:13.408253228 +0100 > +++ qemu/block_int.h 2010-03-03 19:16:43.030003751 +0100 > @@ -209,6 +209,7 @@ struct DriveInfo; > typedef struct BlockConf { > struct DriveInfo *dinfo; > uint16_t physical_block_size; > + uint16_t logical_block_size; > uint16_t min_io_size; > uint32_t opt_io_size; > } BlockConf; > @@ -226,6 +227,8 @@ static inline unsigned int get_physical_ > > #define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ > DEFINE_PROP_DRIVE("drive", _state, _conf.dinfo), \ > + DEFINE_PROP_UINT16("logical_block_size", _state, \ > + _conf.logical_block_size, 512), \ > DEFINE_PROP_UINT16("physical_block_size", _state, \ > _conf.physical_block_size, 512), \ > DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 512), \ > Index: qemu/hw/scsi-disk.c > =================================================================== > --- qemu.orig/hw/scsi-disk.c 2010-03-03 19:16:13.419254346 +0100 > +++ qemu/hw/scsi-disk.c 2010-03-03 19:16:43.031004240 +0100 > @@ -397,8 +397,10 @@ static int scsi_disk_emulate_inquiry(SCS > } > case 0xb0: /* block device characteristics */ > { > - unsigned int min_io_size = s->qdev.conf.min_io_size >> 9; > - unsigned int opt_io_size = s->qdev.conf.opt_io_size >> 9; > + unsigned int min_io_size = > + s->qdev.conf.min_io_size / s->qdev.blocksize; > + unsigned int opt_io_size = > + s->qdev.conf.opt_io_size / s->qdev.blocksize; > > /* required VPD size with unmap support */ > outbuf[3] = buflen = 0x3c; > @@ -1028,11 +1030,12 @@ static int scsi_disk_initfn(SCSIDevice * > s->bs = s->qdev.conf.dinfo->bdrv; > > if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) { > - s->cluster_size = 4; > + s->qdev.blocksize = 2048; > } else { > - s->cluster_size = 1; > + s->qdev.blocksize = s->qdev.conf.logical_block_size; > } > - s->qdev.blocksize = 512 * s->cluster_size; > + s->cluster_size = s->qdev.blocksize / 512; > + > s->qdev.type = TYPE_DISK; > bdrv_get_geometry(s->bs, &nb_sectors); > nb_sectors /= s->cluster_size; > Index: qemu/hw/virtio-blk.c > =================================================================== > --- qemu.orig/hw/virtio-blk.c 2010-03-03 19:16:13.426273971 +0100 > +++ qemu/hw/virtio-blk.c 2010-03-03 19:35:16.636028605 +0100 > @@ -27,6 +27,7 @@ typedef struct VirtIOBlock > void *rq; > QEMUBH *bh; > BlockConf *conf; > + unsigned short sector_mask; > } VirtIOBlock; > > static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev) > @@ -250,6 +251,11 @@ static void virtio_blk_handle_flush(Virt > static void virtio_blk_handle_write(BlockRequest *blkreq, int *num_writes, > VirtIOBlockReq *req, BlockDriverState **old_bs) > { > + if (req->out->sector & req->dev->sector_mask) { > + virtio_blk_rw_complete(req, -EIO); > + return; > + } > + > if (req->dev->bs != *old_bs || *num_writes == 32) { > if (*old_bs != NULL) { > do_multiwrite(*old_bs, blkreq, *num_writes); > @@ -272,6 +278,11 @@ static void virtio_blk_handle_read(VirtI > { > BlockDriverAIOCB *acb; > > + if (req->out->sector & req->dev->sector_mask) { > + virtio_blk_rw_complete(req, -EIO); > + return; > + } > + > acb = bdrv_aio_readv(req->dev->bs, req->out->sector, &req->qiov, > req->qiov.size / 512, virtio_blk_rw_complete, req); > if (!acb) { > @@ -404,12 +415,13 @@ static void virtio_blk_update_config(Vir > stl_raw(&blkcfg.seg_max, 128 - 2); > stw_raw(&blkcfg.cylinders, cylinders); > blkcfg.heads = heads; > - blkcfg.sectors = secs; > + blkcfg.sectors = secs & ~s->sector_mask; > + blkcfg.blk_size = s->conf->logical_block_size; > blkcfg.size_max = 0; > blkcfg.physical_block_exp = get_physical_block_exp(s->conf); > blkcfg.alignment_offset = 0; > - blkcfg.min_io_size = s->conf->min_io_size / 512; > - blkcfg.opt_io_size = s->conf->opt_io_size / 512; > + blkcfg.min_io_size = s->conf->min_io_size / blkcfg.blk_size; > + blkcfg.opt_io_size = s->conf->opt_io_size / blkcfg.blk_size; > memcpy(config, &blkcfg, sizeof(struct virtio_blk_config)); > } > > @@ -420,6 +432,7 @@ static uint32_t virtio_blk_get_features( > features |= (1 << VIRTIO_BLK_F_SEG_MAX); > features |= (1 << VIRTIO_BLK_F_GEOMETRY); > features |= (1 << VIRTIO_BLK_F_TOPOLOGY); > + features |= (1 << VIRTIO_BLK_F_BLK_SIZE); > > if (bdrv_enable_write_cache(s->bs)) > features |= (1 << VIRTIO_BLK_F_WCACHE); > @@ -479,6 +492,7 @@ VirtIODevice *virtio_blk_init(DeviceStat > s->bs = conf->dinfo->bdrv; > s->conf = conf; > s->rq = NULL; > + s->sector_mask = (s->conf->logical_block_size / 512) - 1; > bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs); > bdrv_set_geometry_hint(s->bs, cylinders, heads, secs); > > Index: qemu/hw/virtio-blk.h > =================================================================== > --- qemu.orig/hw/virtio-blk.h 2010-03-03 19:16:13.434257349 +0100 > +++ qemu/hw/virtio-blk.h 2010-03-03 19:16:43.036004100 +0100 > @@ -42,7 +42,7 @@ struct virtio_blk_config > uint16_t cylinders; > uint8_t heads; > uint8_t sectors; > - uint32_t _blk_size; /* structure pad, currently unused */ > + uint32_t blk_size; > uint8_t physical_block_exp; > uint8_t alignment_offset; > uint16_t min_io_size; > ---end quoted text---