From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46768) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xifja-0006Ox-8t for qemu-devel@nongnu.org; Mon, 27 Oct 2014 04:33:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XifjV-0007F1-5H for qemu-devel@nongnu.org; Mon, 27 Oct 2014 04:33:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40599) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XifjU-0007Ed-Tu for qemu-devel@nongnu.org; Mon, 27 Oct 2014 04:32:57 -0400 Message-ID: <544E032F.40509@redhat.com> Date: Mon, 27 Oct 2014 09:32:47 +0100 From: Max Reitz MIME-Version: 1.0 References: <1414253919-3044-1-git-send-email-pl@kamp.de> <1414253919-3044-6-git-send-email-pl@kamp.de> In-Reply-To: <1414253919-3044-6-git-send-email-pl@kamp.de> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv5 5/6] block/iscsi: limit to INT_MAX throughout iscsi_refresh_limits List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, benoit@irqsave.net, ronniesahlberg@gmail.com, armbru@redhat.com, stefanha@redhat.com On 2014-10-25 at 18:18, Peter Lieven wrote: > As Max pointed out there is a hidden cast from int64_t to int. > So use the newly introduced nb_sectors_lun2qemu for all > limits. > > Signed-off-by: Peter Lieven > --- > block/iscsi.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 1ae4add..85131b7 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1468,23 +1468,23 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) > > if (iscsilun->lbp.lbpu) { > if (iscsilun->bl.max_unmap < 0xffffffff) { > - bs->bl.max_discard = sector_lun2qemu(iscsilun->bl.max_unmap, > - iscsilun); > + bs->bl.max_discard = nb_sectors_lun2qemu(iscsilun->bl.max_unmap, > + iscsilun); > } > - bs->bl.discard_alignment = sector_lun2qemu(iscsilun->bl.opt_unmap_gran, > - iscsilun); > + bs->bl.discard_alignment = nb_sectors_lun2qemu(iscsilun->bl.opt_unmap_gran, > + iscsilun); This looks wrong. I think an alignment should always be a power of two. The function however may return the unaligned INT_MAX. I think it should be capped to something like INT_MAX / 2 + 1 or just simply written out 0x40000000. > } > > if (iscsilun->bl.max_ws_len < 0xffffffff) { > - bs->bl.max_write_zeroes = sector_lun2qemu(iscsilun->bl.max_ws_len, > - iscsilun); > + bs->bl.max_write_zeroes = nb_sectors_lun2qemu(iscsilun->bl.max_ws_len, > + iscsilun); > } > if (iscsilun->lbp.lbpws) { > - bs->bl.write_zeroes_alignment = sector_lun2qemu(iscsilun->bl.opt_unmap_gran, > - iscsilun); > + bs->bl.write_zeroes_alignment = nb_sectors_lun2qemu(iscsilun->bl.opt_unmap_gran, > + iscsilun); Same here. > } > - bs->bl.opt_transfer_length = sector_lun2qemu(iscsilun->bl.opt_xfer_len, > - iscsilun); > + bs->bl.opt_transfer_length = nb_sectors_lun2qemu(iscsilun->bl.opt_xfer_len, > + iscsilun); > } > > /* Since iscsi_open() ignores bdrv_flags, there is nothing to do here in Max