From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPWJC-0000ZD-7h for qemu-devel@nongnu.org; Wed, 02 May 2012 05:57:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SPWJ5-0007B5-Ky for qemu-devel@nongnu.org; Wed, 02 May 2012 05:57:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37310) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPWJ5-0007Aw-Ca for qemu-devel@nongnu.org; Wed, 02 May 2012 05:57:11 -0400 Message-ID: <4FA104F2.6020106@redhat.com> Date: Wed, 02 May 2012 11:57:06 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1335801160-321-1-git-send-email-mjt@msgid.tls.msk.ru> In-Reply-To: <1335801160-321-1-git-send-email-mjt@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] increase BlockConf.min_io_size type from uint16_t to uint32_t List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: qemu-devel@nongnu.org, Anthony Liguori Am 30.04.2012 17:52, schrieb Michael Tokarev: > This value is used currently for virtio-blk only. It was defined > as uint16_t before, which is the same as in kernel<=>user interface > (in virtio_blk.h, struct virtio_blk_config). But the problem is > that in kernel<=>user interface the units are sectors (which is > usually 512 bytes or more), while in qemu it is in bytes. However, > for, say, md raid5 arrays, it is typical to have actual min_io_size > of a host device to be large. For example, for raid5 device of > 3 drives with 64Kb chunk size, that value will be 128Kb, which does > not fit in a uint16_t anymore. > > Increase the value size from 16bits to 32bits for now. > > But apparently, the kernel<=>user interface needs to be fixed too > (while it is much more difficult due to compatibility issues), > because even with 512byte units, the 16bit value there will overflow > too with quite normal MD RAID configuration. > > Signed-off-by: Michael Tokarev > --- > block.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block.h b/block.h > index f163e54..cd5ae79 100644 > --- a/block.h > +++ b/block.h > @@ -425,7 +425,7 @@ typedef struct BlockConf { > BlockDriverState *bs; > uint16_t physical_block_size; > uint16_t logical_block_size; > - uint16_t min_io_size; > + uint32_t min_io_size; > uint32_t opt_io_size; > int32_t bootindex; > uint32_t discard_granularity; > @@ -450,7 +450,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) > _conf.logical_block_size, 512), \ > DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ > _conf.physical_block_size, 512), \ > - DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ > + DEFINE_PROP_UINT32("min_io_size", _state, _conf.min_io_size, 0), \ > DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ > DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ > DEFINE_PROP_UINT32("discard_granularity", _state, \ Don't you need an additional check in virtio-blk now so that you don't overflow the 16 bit field in the virtio protocol? And I guess the same for SCSI, where INQUIRY reports only a 16 bits sector count as well. Kevin