From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56812) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WVS1l-0005Ex-08 for qemu-devel@nongnu.org; Wed, 02 Apr 2014 16:44:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WVS1f-0003Wz-8W for qemu-devel@nongnu.org; Wed, 02 Apr 2014 16:44:52 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:40972 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WVS1e-0003Wt-Tz for qemu-devel@nongnu.org; Wed, 02 Apr 2014 16:44:47 -0400 Message-ID: <533C76B9.90507@kamp.de> Date: Wed, 02 Apr 2014 22:44:41 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1396446284-5594-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1396446284-5594-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.0] iscsi: always query max WRITE SAME length List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Am 02.04.2014 15:44, schrieb Paolo Bonzini: > Max WRITE SAME length is also used when the UNMAP bit is zero, so it > should be queried even if LBPWS=0. Same for the optimal transfer > length. > > However, the write_zeroes_alignment only matters for UNMAP=1 so we > still restrict it to LBPWS=1. > > Signed-off-by: Paolo Bonzini > --- > block/iscsi.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 00316af..bb88b4b 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1335,18 +1335,20 @@ static int iscsi_refresh_limits(BlockDriverState *bs) > > /* We don't actually refresh here, but just return data queried in > * iscsi_open(): iscsi targets don't change their limits. */ > - if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) { > + if (iscsilun->lbp.lbpu) { > if (iscsilun->bl.max_unmap < 0xffffffff) { > bs->bl.max_discard = sector_lun2qemu(iscsilun->bl.max_unmap, > iscsilun); > } > bs->bl.discard_alignment = sector_lun2qemu(iscsilun->bl.opt_unmap_gran, > iscsilun); > + } > > - if (iscsilun->bl.max_ws_len < 0xffffffff) { > - bs->bl.max_write_zeroes = sector_lun2qemu(iscsilun->bl.max_ws_len, > - iscsilun); > - } > + if (iscsilun->bl.max_ws_len < 0xffffffff) { > + bs->bl.max_write_zeroes = sector_lun2qemu(iscsilun->bl.max_ws_len, > + iscsilun); > + } > + if (iscsilun->lbp.lbpws) { > bs->bl.write_zeroes_alignment = sector_lun2qemu(iscsilun->bl.opt_unmap_gran, > iscsilun); > } Reviewed-by: Peter Lieven PS: One thing that came into my mind while looking at this. If a target has LBPWS == 0 and we send a write zeroes request with BDRV_REQ_MAY_UNMAP we fail with -ENOTSUP. Shouldn't we fall back to WRITESAME without UNMAP if iscsilun->has_write_same == true? Peter