From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsSOt-0008Md-Lw for qemu-devel@nongnu.org; Wed, 13 May 2015 04:52:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsSOp-0003b5-CK for qemu-devel@nongnu.org; Wed, 13 May 2015 04:52:23 -0400 Date: Wed, 13 May 2015 16:52:16 +0800 From: Fam Zheng Message-ID: <20150513085216.GA30644@ad.nay.redhat.com> References: <1431522928-5292-1-git-send-email-famz@redhat.com> <55530EAD.5070104@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55530EAD.5070104@redhat.com> Subject: Re: [Qemu-devel] [PATCH] iscsi: Remove pointless runtime check of macro value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org On Wed, 05/13 10:43, Paolo Bonzini wrote: > > > On 13/05/2015 15:15, Fam Zheng wrote: > > raw_bsd already has QEMU_BUILD_BUG_ON(BDRV_SECTOR_SIZE != 512), so iscsi > > should relax. > > It's okay to have the same QEMU_BUILD_BUG_ON in different places. > However, you're right that this should be a compile-time check rather > than run-time check. I agree with you, but I can't imaging this value could ever change, so there is little gain to duplicate in this case. Fam > > Paolo > > > Signed-off-by: Fam Zheng > > --- > > block/iscsi.c | 7 ------- > > 1 file changed, 7 deletions(-) > > > > diff --git a/block/iscsi.c b/block/iscsi.c > > index 8fca1d3..14e97a6 100644 > > --- a/block/iscsi.c > > +++ b/block/iscsi.c > > @@ -1323,13 +1323,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, > > const char *filename; > > int i, ret = 0; > > > > - if ((BDRV_SECTOR_SIZE % 512) != 0) { > > - error_setg(errp, "iSCSI: Invalid BDRV_SECTOR_SIZE. " > > - "BDRV_SECTOR_SIZE(%lld) is not a multiple " > > - "of 512", BDRV_SECTOR_SIZE); > > - return -EINVAL; > > - } > > - > > opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); > > qemu_opts_absorb_qdict(opts, options, &local_err); > > if (local_err) { > > >