Patchwork [PATCHv3] iscsi: add iscsi_truncate support

login
register
mail settings
Submitter Peter Lieven
Date Feb. 18, 2013, 12:58 p.m.
Message ID <5122258F.7070201@dlhnet.de>
Download mbox | patch
Permalink /patch/221352/
State New
Headers show

Comments

Peter Lieven - Feb. 18, 2013, 12:58 p.m.
this patch adds iscsi_truncate which effectively allows for
online resizing of iscsi volumes. for this to work you have
to resize the volume on your storage and then call
block_resize command in qemu which will issue a
readcapacity16 to update the capacity.

v3:
  - remove switch statement in iscsi_open
  - create separate patch for brdv_drain_all() in bdrv_truncate()

v2:
  - add a general bdrv_drain_all() before bdrv_truncate() to avoid
    in-flight AIOs while the device is truncated
  - since no AIOs are in flight we can use a sync libiscsi call
    to re-read the capacity
  - factor out the readcapacity16 logic as it is redundant
    to iscsi_open() and iscsi_truncate().

Signed-off-by: Peter Lieven <pl@kamp.de>
---
  block/iscsi.c |   77 +++++++++++++++++++++++++++++++++++++++++----------------
  1 file changed, 56 insertions(+), 21 deletions(-)
Paolo Bonzini - Feb. 18, 2013, 1:01 p.m.
Il 18/02/2013 13:58, Peter Lieven ha scritto:
> 
> -    switch (iscsilun->type) {
> -    case TYPE_DISK:
> -        task = iscsi_readcapacity16_sync(iscsi, iscsilun->lun);
> -        if (task == NULL || task->status != SCSI_STATUS_GOOD) {
> -            error_report("iSCSI: failed to send readcapacity16 command.");
> -            ret = -EINVAL;
> -            goto out;
> -        }
> -        rc16 = scsi_datain_unmarshall(task);
> -        if (rc16 == NULL) {
> -            error_report("iSCSI: Failed to unmarshall readcapacity16
> data.");
> -            ret = -EINVAL;
> +    if (iscsilun->type == TYPE_DISK) {
> +        if ((ret = iscsi_disk_readcapacity16_sync(iscsilun))) {
>              goto out;
>          }
> -        iscsilun->block_size = rc16->block_length;
> -        iscsilun->num_blocks = rc16->returned_lba + 1;
> -        break;
> -    case TYPE_ROM:
> +    }
> +    if (iscsilun->type == TYPE_ROM) {
>          task = iscsi_readcapacity10_sync(iscsi, iscsilun->lun, 0, 0);
>          if (task == NULL || task->status != SCSI_STATUS_GOOD) {
>              error_report("iSCSI: failed to send readcapacity10 command.");
> @@ -964,10 +981,7 @@ static int iscsi_open(BlockDriverState *bs, const
> char *filename, int flags)
>          } else {
>              iscsilun->num_blocks = rc10->lba + 1;
>          }
> -        break;
> -    default:
> -        break;
> -    }
> +    }

I didn't write to *remove* the switch statement, I wrote to *move* it
into a new function and use it from iscsi_truncate.

Paolo
Peter Lieven - Feb. 18, 2013, 1:04 p.m.
On 18.02.2013 14:01, Paolo Bonzini wrote:
> Il 18/02/2013 13:58, Peter Lieven ha scritto:
>>
>> -    switch (iscsilun->type) {
>> -    case TYPE_DISK:
>> -        task = iscsi_readcapacity16_sync(iscsi, iscsilun->lun);
>> -        if (task == NULL || task->status != SCSI_STATUS_GOOD) {
>> -            error_report("iSCSI: failed to send readcapacity16 command.");
>> -            ret = -EINVAL;
>> -            goto out;
>> -        }
>> -        rc16 = scsi_datain_unmarshall(task);
>> -        if (rc16 == NULL) {
>> -            error_report("iSCSI: Failed to unmarshall readcapacity16
>> data.");
>> -            ret = -EINVAL;
>> +    if (iscsilun->type == TYPE_DISK) {
>> +        if ((ret = iscsi_disk_readcapacity16_sync(iscsilun))) {
>>               goto out;
>>           }
>> -        iscsilun->block_size = rc16->block_length;
>> -        iscsilun->num_blocks = rc16->returned_lba + 1;
>> -        break;
>> -    case TYPE_ROM:
>> +    }
>> +    if (iscsilun->type == TYPE_ROM) {
>>           task = iscsi_readcapacity10_sync(iscsi, iscsilun->lun, 0, 0);
>>           if (task == NULL || task->status != SCSI_STATUS_GOOD) {
>>               error_report("iSCSI: failed to send readcapacity10 command.");
>> @@ -964,10 +981,7 @@ static int iscsi_open(BlockDriverState *bs, const
>> char *filename, int flags)
>>           } else {
>>               iscsilun->num_blocks = rc10->lba + 1;
>>           }
>> -        break;
>> -    default:
>> -        break;
>> -    }
>> +    }
>
> I didn't write to *remove* the switch statement, I wrote to *move* it
> into a new function and use it from iscsi_truncate.

Sorry, not enough sleep. However, truncate makes only sense for lun type disk, doesn't it?

Peter


>
> Paolo
>
Paolo Bonzini - Feb. 18, 2013, 1:06 p.m.
Il 18/02/2013 14:04, Peter Lieven ha scritto:
>>>
>>
>> I didn't write to *remove* the switch statement, I wrote to *move* it
>> into a new function and use it from iscsi_truncate.
> 
> Sorry, not enough sleep. However, truncate makes only sense for lun type
> disk, doesn't it?

I think so, mostly because anyway you can use "change" to reset the
capacity on TYPE_ROM, they are removable.  But still the code looks
better that way.

Paolo

Patch

diff --git a/block/iscsi.c b/block/iscsi.c
index deb3b68..105533c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -823,6 +823,35 @@  static void iscsi_nop_timed_event(void *opaque)
  }
  #endif

+static int iscsi_disk_readcapacity16_sync(IscsiLun *iscsilun) {
+    struct scsi_task *task = NULL;
+    struct scsi_readcapacity16 *rc16 = NULL;
+
+    task = iscsi_readcapacity16_sync(iscsilun->iscsi, iscsilun->lun);
+    if (task == NULL) {
+        error_report("iSCSI: failed to send readcapacity16 command.");
+        return -EINVAL;
+    }
+    if (task->status != SCSI_STATUS_GOOD) {
+        error_report("iSCSI: failed to send readcapacity16 command.");
+        scsi_free_scsi_task(task);
+        return -EINVAL;
+    }
+    rc16 = scsi_datain_unmarshall(task);
+    if (rc16 == NULL) {
+        error_report("iSCSI: Failed to unmarshall readcapacity16 data.");
+        scsi_free_scsi_task(task);
+        return -EINVAL;
+    }
+
+    iscsilun->block_size = rc16->block_length;
+    iscsilun->num_blocks = rc16->returned_lba + 1;
+
+    scsi_free_scsi_task(task);
+
+    return 0;
+}
+
  /*
   * We support iscsi url's on the form
   * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
@@ -835,7 +864,6 @@  static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
      struct scsi_task *task = NULL;
      struct scsi_inquiry_standard *inq = NULL;
      struct scsi_readcapacity10 *rc10 = NULL;
-    struct scsi_readcapacity16 *rc16 = NULL;
      char *initiator_name = NULL;
      int ret;

@@ -926,25 +954,14 @@  static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
      iscsilun->type = inq->periperal_device_type;

      scsi_free_scsi_task(task);
+    task = NULL;

-    switch (iscsilun->type) {
-    case TYPE_DISK:
-        task = iscsi_readcapacity16_sync(iscsi, iscsilun->lun);
-        if (task == NULL || task->status != SCSI_STATUS_GOOD) {
-            error_report("iSCSI: failed to send readcapacity16 command.");
-            ret = -EINVAL;
-            goto out;
-        }
-        rc16 = scsi_datain_unmarshall(task);
-        if (rc16 == NULL) {
-            error_report("iSCSI: Failed to unmarshall readcapacity16 data.");
-            ret = -EINVAL;
+    if (iscsilun->type == TYPE_DISK) {
+        if ((ret = iscsi_disk_readcapacity16_sync(iscsilun))) {
              goto out;
          }
-        iscsilun->block_size = rc16->block_length;
-        iscsilun->num_blocks = rc16->returned_lba + 1;
-        break;
-    case TYPE_ROM:
+    }
+    if (iscsilun->type == TYPE_ROM) {
          task = iscsi_readcapacity10_sync(iscsi, iscsilun->lun, 0, 0);
          if (task == NULL || task->status != SCSI_STATUS_GOOD) {
              error_report("iSCSI: failed to send readcapacity10 command.");
@@ -964,10 +981,7 @@  static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
          } else {
              iscsilun->num_blocks = rc10->lba + 1;
          }
-        break;
-    default:
-        break;
-    }
+    }

      bs->total_sectors    = iscsilun->num_blocks *
                             iscsilun->block_size / BDRV_SECTOR_SIZE ;
@@ -1023,6 +1037,26 @@  static void iscsi_close(BlockDriverState *bs)
      memset(iscsilun, 0, sizeof(IscsiLun));
  }

+static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
+{
+    IscsiLun *iscsilun = bs->opaque;
+    int ret = 0;
+
+    if (iscsilun->type != TYPE_DISK) {
+        return -ENOTSUP;
+    }
+
+    if ((ret = iscsi_disk_readcapacity16_sync(iscsilun))) {
+        return ret;
+    }
+
+    if (offset > iscsi_getlength(bs)) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
  static int iscsi_has_zero_init(BlockDriverState *bs)
  {
      return 0;
@@ -1093,6 +1127,7 @@  static BlockDriver bdrv_iscsi = {
      .create_options  = iscsi_create_options,

      .bdrv_getlength  = iscsi_getlength,
+    .bdrv_truncate   = iscsi_truncate,

      .bdrv_aio_readv  = iscsi_aio_readv,
      .bdrv_aio_writev = iscsi_aio_writev,