[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 13/20] block/iscsi: use UNMAP to write zeroes

From: Peter Lieven
Subject: Re: [Qemu-devel] [PATCH v2 13/20] block/iscsi: use UNMAP to write zeroes if LBPRZ=1
Date: Thu, 21 Nov 2013 12:43:08 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0

On 19.11.2013 18:07, Paolo Bonzini wrote:
The latest revision of SCSI SBC clarifies the semantics of LBPRZ
in a way that lets the iscsi driver remove the distinction between
bdrv_unallocated_blocks_are_zero and bdrv_can_write_zeroes_with_unmap.
See Table 8:

     "[If] the LBA became mapped as the result of an autonomous transition,
     and no write command has specified that LBA since the LBA was mapped,
     [a read operation returns the] logical block data that would be returned
     if that autonomous transition had not occurred and the LBA had remained

Thus, we can assume that even after an UNMAP command, unallocated blocks
return zero.  The distinction may remain for other drivers, for example
qcow2 or qed or vmdk.

Signed-off-by: Paolo Bonzini <address@hidden>
  block/iscsi.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 93fee6d..63b451d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -984,6 +984,9 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, 
int64_t sector_num,
if ((flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->lbp.lbpws) {
          /* WRITE SAME with UNMAP is not supported by the target */
+        if (iscsilun->lbp.lbpu && iscsilun->lbprz) {
+            return iscsi_co_discard(bs, sector_num, nb_sectors);
+        }
          return -ENOTSUP;
@@ -1579,7 +1582,7 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
      IscsiLun *iscsilun = bs->opaque;
      bdi->unallocated_blocks_are_zero = !!iscsilun->lbprz;
-    bdi->can_write_zeroes_with_unmap = iscsilun->lbprz && iscsilun->lbp.lbpws;
+    bdi->can_write_zeroes_with_unmap = !!iscsilun->lbprz;
      return 0;
I would definetly not do that! I have seen at least my target to execute only 
parts of a discard request.
Additionally in our semantic a discard request may silently fail. In general 
this could lead to data corruption
due to broken implementations.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]