Re: [Qemu-devel] [PATCH 7/8] iscsi: assert that sectors are aligned to LUN blocksize

[Peter Lieven]

Am 24.06.2013 16:30, schrieb Paolo Bonzini:
> Il 22/06/2013 22:58, Peter Lieven ha scritto:
>> if the blocksize of an iSCSI LUN is bigger than the BDRV_SECTOR_SIZE
>> it is possible that sector_num or nb_sectors are not correctly
>> alligned.
>>
>> for now assert that there is no misalignment to avoid data
>> corruption.
> You should just fail to open the LUN instead.
Ronnie added support for reading fragments of blocksize if the
offset is aligned. I don't know what the idea was, maybe for qemu
reading the boot sector. If the OS does only read multiple of
blocksize at aligned offsets everything should work. I now that
our storages support 4K blocksize. I can check if it is usable.
I could also just fail the operations instead of asserting.

Peter
>
> Paolo
>
>> Signed-off-by: Peter Lieven <address@hidden>
>> ---
>>  block/iscsi.c |    7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index 683692a..2c410b1 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -249,6 +249,7 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int 
>> status,
>>  
>>  static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
>>  {
>> +    assert((sector * BDRV_SECTOR_SIZE) % iscsilun->block_size == 0);
>>      return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
>>  }
>>  
>> @@ -269,6 +270,8 @@ iscsi_aio_writev_acb(IscsiAIOCB *acb)
>>      acb->status     = -EINPROGRESS;
>>      acb->buf        = NULL;
>>  
>> +    assert((acb->nb_sectors * BDRV_SECTOR_SIZE) % acb->iscsilun->block_size 
>> == 0);
>> +
>>      /* this will allow us to get rid of 'buf' completely */
>>      size = acb->nb_sectors * BDRV_SECTOR_SIZE;
>>  
>> @@ -618,6 +621,8 @@ static int iscsi_aio_discard_acb(IscsiAIOCB *acb) {
>>      acb->buf        = NULL;
>>  
>>      list[0].lba = sector_qemu2lun(acb->sector_num, acb->iscsilun);
>> +
>> +    assert((acb->nb_sectors * BDRV_SECTOR_SIZE) % acb->iscsilun->block_size 
>> == 0);
>>      list[0].num = acb->nb_sectors * BDRV_SECTOR_SIZE / 
>> acb->iscsilun->block_size;
>>  
>>      acb->task = iscsi_unmap_task(iscsi, acb->iscsilun->lun,
>> @@ -947,6 +952,8 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, 
>> int64_t sector_num,
>>      iTask.complete = 0;
>>      iTask.bs = bs;
>>  
>> +    assert((nb_sectors * BDRV_SECTOR_SIZE) % iscsilun->block_size == 0);
>> +
>>      list[0].lba = sector_qemu2lun(sector_num, iscsilun);
>>      list[0].num = nb_sectors * BDRV_SECTOR_SIZE / iscsilun->block_size;
>>  
>>