[Qemu-devel] [PATCH 0/2] iscsi: support for is_allocated and inproved has_zero_init

[Qemu-devel] [PATCH 0/2] iscsi: support for is_allocated and inproved has_zero_init

[Peter Lieven]

These two patches add the possibility for qemu-img convert to reliably skip
zero blocks when writing to an iscsi target. 

Peter Lieven (2):
  iscsi: add support for bdrv_co_is_allocated()
  iscsi: add intelligent has_zero_init check

 block/iscsi.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated()

[Peter Lieven]

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/iscsi.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index 0bbf0b1..e6b966d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -49,6 +49,7 @@ typedef struct IscsiLun {
     uint64_t num_blocks;
     int events;
     QEMUTimer *nop_timer;
+    uint8_t lbpme;
 } IscsiLun;
 
 typedef struct IscsiAIOCB {
@@ -800,6 +801,60 @@ iscsi_getlength(BlockDriverState *bs)
     return len;
 }
 
+static int coroutine_fn iscsi_co_is_allocated(BlockDriverState *bs,
+                                              int64_t sector_num,
+                                              int nb_sectors, int *pnum)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    struct scsi_task *task = NULL;
+    struct scsi_get_lba_status *lbas = NULL;
+    struct scsi_lba_status_descriptor *lbasd = NULL;
+    int ret;
+
+    *pnum = nb_sectors;
+
+    if (iscsilun->lbpme == 0) {
+        return 1;
+    }
+
+    /* in-flight requests could invalidate the lba status result */
+    while (iscsi_process_flush(iscsilun)) {
+        qemu_aio_wait();
+    }
+
+    task = iscsi_get_lba_status_sync(iscsilun->iscsi, iscsilun->lun,
+                                     sector_qemu2lun(sector_num, iscsilun),
+                                     8+16);
+
+    if (task == NULL || task->status != SCSI_STATUS_GOOD) {
+        scsi_free_scsi_task(task);
+        return 1;
+    }
+
+    lbas = scsi_datain_unmarshall(task);
+    if (lbas == NULL) {
+        scsi_free_scsi_task(task);
+        return 1;
+    }
+
+    lbasd = &lbas->descriptors[0];
+
+    if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
+        return 1;
+    }
+
+    *pnum = lbasd->num_blocks * (iscsilun->block_size / BDRV_SECTOR_SIZE);
+    if (*pnum > nb_sectors) {
+        *pnum = nb_sectors;
+    }
+
+    ret = (lbasd->provisioning == SCSI_PROVISIONING_TYPE_MAPPED) ? 1 : 0;
+
+    scsi_free_scsi_task(task);
+
+    return ret;
+}
+
 static int parse_chap(struct iscsi_context *iscsi, const char *target)
 {
     QemuOptsList *list;
@@ -948,6 +1003,7 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun)
                 } else {
                     iscsilun->block_size = rc16->block_length;
                     iscsilun->num_blocks = rc16->returned_lba + 1;
+                    iscsilun->lbpme = rc16->lbpme;
                 }
             }
             break;
@@ -1274,6 +1330,7 @@ static BlockDriver bdrv_iscsi = {
 
     .bdrv_aio_discard = iscsi_aio_discard,
     .bdrv_has_zero_init = iscsi_has_zero_init,
+    .bdrv_co_is_allocated = iscsi_co_is_allocated,
 
 #ifdef __linux__
     .bdrv_ioctl       = iscsi_ioctl,
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/2] iscsi: add intelligent has_zero_init check

[Peter Lieven]

iscsi targets are not created by bdrv_create and thus we cannot
blindly assume that a target is empty. to avoid writing and allocating
blocks of zeroes we now check if all blocks of an existing target
are unallocated and return 1 for bdrv_has_zero_init if the
target is completely unalloacted and unallocated blocks read
as zeroes.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/iscsi.c |   25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index e6b966d..fe41d9a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -50,6 +50,7 @@ typedef struct IscsiLun {
     int events;
     QEMUTimer *nop_timer;
     uint8_t lbpme;
+    uint8_t lbprz;
 } IscsiLun;
 
 typedef struct IscsiAIOCB {
@@ -1004,6 +1005,7 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun)
                     iscsilun->block_size = rc16->block_length;
                     iscsilun->num_blocks = rc16->returned_lba + 1;
                     iscsilun->lbpme = rc16->lbpme;
+                    iscsilun->lbprz = rc16->lbprz;
                 }
             }
             break;
@@ -1249,7 +1251,28 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
 
 static int iscsi_has_zero_init(BlockDriverState *bs)
 {
-    return 0;
+    IscsiLun *iscsilun = bs->opaque;
+    uint64_t lba;
+    int n, ret, nb_sectors;
+
+    if (iscsilun->lbprz == 0) {
+        return 0;
+    }
+
+    for (lba = 0; lba < iscsilun->num_blocks; lba += 1 << 26) {
+        nb_sectors = 1 << 26;
+        if (lba + nb_sectors > iscsilun->num_blocks) {
+            nb_sectors = iscsilun->num_blocks - lba;
+        }
+        nb_sectors *= (iscsilun->block_size / BDRV_SECTOR_SIZE);
+        n = 0;
+        ret = iscsi_co_is_allocated(bs, lba, nb_sectors, &n);
+        if (ret || n != nb_sectors) {
+            return 0;
+        }
+    }
+
+    return 1;
 }
 
 static int iscsi_create(const char *filename, QEMUOptionParameter *options)
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/2] iscsi: add intelligent has_zero_init check

[Peter Lieven]

iscsi targets are not created by bdrv_create and thus we cannot
blindly assume that a target is empty. to avoid writing and allocating
blocks of zeroes we now check if all blocks of an existing target
are unallocated and return 1 for bdrv_has_zero_init if the
target is completely unalloacted and unallocated blocks read
as zeroes.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/iscsi.c |   25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index e6b966d..fe41d9a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -50,6 +50,7 @@ typedef struct IscsiLun {
     int events;
     QEMUTimer *nop_timer;
     uint8_t lbpme;
+    uint8_t lbprz;
 } IscsiLun;
 
 typedef struct IscsiAIOCB {
@@ -1004,6 +1005,7 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun)
                     iscsilun->block_size = rc16->block_length;
                     iscsilun->num_blocks = rc16->returned_lba + 1;
                     iscsilun->lbpme = rc16->lbpme;
+                    iscsilun->lbprz = rc16->lbprz;
                 }
             }
             break;
@@ -1249,7 +1251,28 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
 
 static int iscsi_has_zero_init(BlockDriverState *bs)
 {
-    return 0;
+    IscsiLun *iscsilun = bs->opaque;
+    uint64_t lba;
+    int n, ret, nb_sectors;
+
+    if (iscsilun->lbprz == 0) {
+        return 0;
+    }
+
+    for (lba = 0; lba < iscsilun->num_blocks; lba += 1 << 26) {
+        nb_sectors = 1 << 26;
+        if (lba + nb_sectors > iscsilun->num_blocks) {
+            nb_sectors = iscsilun->num_blocks - lba;
+        }
+        nb_sectors *= (iscsilun->block_size / BDRV_SECTOR_SIZE);
+        n = 0;
+        ret = iscsi_co_is_allocated(bs, lba, nb_sectors, &n);
+        if (ret || n != nb_sectors) {
+            return 0;
+        }
+    }
+
+    return 1;
 }
 
 static int iscsi_create(const char *filename, QEMUOptionParameter *options)
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 2/2] iscsi: add intelligent has_zero_init check

[Paolo Bonzini]

Il 20/06/2013 20:20, Peter Lieven ha scritto:
> +    for (lba = 0; lba < iscsilun->num_blocks; lba += 1 << 26) {
> +        nb_sectors = 1 << 26;
> +        if (lba + nb_sectors > iscsilun->num_blocks) {
> +            nb_sectors = iscsilun->num_blocks - lba;
> +        }
> +        nb_sectors *= (iscsilun->block_size / BDRV_SECTOR_SIZE);
> +        n = 0;
> +        ret = iscsi_co_is_allocated(bs, lba, nb_sectors, &n);
> +        if (ret || n != nb_sectors) {
> +            return 0;
> +        }

I would just do lba += n in the for loop, and only exit if n == 0.  The
SCSI spec does not forbid splitting a single allocated area into
multiple descriptors, or only returning part of an allocated area into
the last descriptor.

Otherwise looks good, but you may want to cache the result.  It would
not be 1 anymore after the first write.

Paolo

Re: [Qemu-devel] [PATCH 2/2] iscsi: add intelligent has_zero_init check

[Peter Lieven]

Von meinem iPhone gesendet

Am 21.06.2013 um 22:00 schrieb Paolo Bonzini <pbonzini@redhat.com>:

> Il 20/06/2013 20:20, Peter Lieven ha scritto:
>> +    for (lba = 0; lba < iscsilun->num_blocks; lba += 1 << 26) {
>> +        nb_sectors = 1 << 26;
>> +        if (lba + nb_sectors > iscsilun->num_blocks) {
>> +            nb_sectors = iscsilun->num_blocks - lba;
>> +        }
>> +        nb_sectors *= (iscsilun->block_size / BDRV_SECTOR_SIZE);
>> +        n = 0;
>> +        ret = iscsi_co_is_allocated(bs, lba, nb_sectors, &n);
>> +        if (ret || n != nb_sectors) {
>> +            return 0;
>> +        }
> 
> I would just do lba += n in the for loop, and only exit if n == 0.  The
> SCSI spec does not forbid splitting a single allocated area into
> multiple descriptors, or only returning part of an allocated area into
> the last descriptor.
> 
> Otherwise looks good, but you may want to cache the result.  It would
> not be 1 anymore after the first write.
> 

Of course, but after some discussions with Kevin we might have found a better Solution than extending has_zero_init this far. 

Let you know soon :-)

> Paolo