Skip to content

Commit

Permalink
block/iscsi: only report an iSCSI Failure if we don't handle it grace…
Browse files Browse the repository at this point in the history 
…fully

we currently report an "iSCSI Failure" in iscsi_co_generic_cb if the task
hasn't completed with SCSI_STATUS_GOOD. However, we expect a failure in
some cases and handle it gracefully. This is the case for misaligned UNMAPs
and WRITESAME10/16 calls without UNMAP. In this case a failure in the
logs can be quite misleading.

While we are at it improve the logging to reveal which operation failed
at what LBA.

Signed-off-by: Peter Lieven <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
  • Loading branch information
@plieven @bonzini
plieven authored and bonzini committed Dec 21, 2017
1 parent aef172f commit e38bc23
Showing 1 changed file with 32 additions and 11 deletions.
43 changes: 32 additions & 11 deletions block/iscsi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ typedef struct IscsiTask {
IscsiLun *iscsilun;
QEMUTimer retry_timer;
int err_code;
char *err_str;
} IscsiTask;

typedef struct IscsiAIOCB {
Expand Down Expand Up @@ -265,7 +266,7 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
}
}
iTask->err_code = iscsi_translate_sense(&task->sense);
error_report("iSCSI Failure: %s", iscsi_get_error(iscsi));
iTask->err_str = g_strdup(iscsi_get_error(iscsi));
}

out:
Expand Down Expand Up @@ -629,6 +630,8 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,

if (iTask.status != SCSI_STATUS_GOOD) {
iscsi_allocmap_set_invalid(iscsilun, sector_num, nb_sectors);
error_report("iSCSI WRITE10/16 failed at lba %" PRIu64 ": %s", lba,
iTask.err_str);
r = iTask.err_code;
goto out_unlock;
}
Expand All @@ -637,6 +640,7 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,

out_unlock:
qemu_mutex_unlock(&iscsilun->mutex);
g_free(iTask.err_str);
return r;
}

Expand All @@ -651,10 +655,9 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
struct scsi_get_lba_status *lbas = NULL;
struct scsi_lba_status_descriptor *lbasd = NULL;
struct IscsiTask iTask;
uint64_t lba;
int64_t ret;

iscsi_co_init_iscsitask(iscsilun, &iTask);

if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
ret = -EINVAL;
goto out;
Expand All @@ -670,11 +673,13 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
goto out;
}

lba = sector_qemu2lun(sector_num, iscsilun);

iscsi_co_init_iscsitask(iscsilun, &iTask);
qemu_mutex_lock(&iscsilun->mutex);
retry:
if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
sector_qemu2lun(sector_num, iscsilun),
8 + 16, iscsi_co_generic_cb,
lba, 8 + 16, iscsi_co_generic_cb,
&iTask) == NULL) {
ret = -ENOMEM;
goto out_unlock;
Expand All @@ -701,6 +706,8 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
* because the device is busy or the cmd is not
* supported) we pretend all blocks are allocated
* for backwards compatibility */
error_report("iSCSI GET_LBA_STATUS failed at lba %" PRIu64 ": %s",
lba, iTask.err_str);
goto out_unlock;
}

Expand Down Expand Up @@ -738,6 +745,7 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
}
out_unlock:
qemu_mutex_unlock(&iscsilun->mutex);
g_free(iTask.err_str);
out:
if (iTask.task != NULL) {
scsi_free_scsi_task(iTask.task);
Expand All @@ -756,6 +764,7 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
struct IscsiTask iTask;
uint64_t lba;
uint32_t num_sectors;
int r = 0;

if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
return -EINVAL;
Expand Down Expand Up @@ -853,19 +862,23 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
iTask.complete = 0;
goto retry;
}
qemu_mutex_unlock(&iscsilun->mutex);

if (iTask.status != SCSI_STATUS_GOOD) {
return iTask.err_code;
error_report("iSCSI READ10/16 failed at lba %" PRIu64 ": %s",
lba, iTask.err_str);
r = iTask.err_code;
}

return 0;
qemu_mutex_unlock(&iscsilun->mutex);
g_free(iTask.err_str);
return r;
}

static int coroutine_fn iscsi_co_flush(BlockDriverState *bs)
{
IscsiLun *iscsilun = bs->opaque;
struct IscsiTask iTask;
int r = 0;

iscsi_co_init_iscsitask(iscsilun, &iTask);
qemu_mutex_lock(&iscsilun->mutex);
Expand All @@ -892,13 +905,15 @@ static int coroutine_fn iscsi_co_flush(BlockDriverState *bs)
iTask.complete = 0;
goto retry;
}
qemu_mutex_unlock(&iscsilun->mutex);

if (iTask.status != SCSI_STATUS_GOOD) {
return iTask.err_code;
error_report("iSCSI SYNCHRONIZECACHE10 failed: %s", iTask.err_str);
r = iTask.err_code;
}

return 0;
qemu_mutex_unlock(&iscsilun->mutex);
g_free(iTask.err_str);
return r;
}

#ifdef __linux__
Expand Down Expand Up @@ -1139,12 +1154,15 @@ coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
}

if (iTask.status != SCSI_STATUS_GOOD) {
error_report("iSCSI UNMAP failed at lba %" PRIu64 ": %s",
list.lba, iTask.err_str);
r = iTask.err_code;
goto out_unlock;
}

out_unlock:
qemu_mutex_unlock(&iscsilun->mutex);
g_free(iTask.err_str);
return r;
}

Expand Down Expand Up @@ -1241,6 +1259,8 @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
if (iTask.status != SCSI_STATUS_GOOD) {
iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,
bytes >> BDRV_SECTOR_BITS);
error_report("iSCSI WRITESAME10/16 failed at lba %" PRIu64 ": %s",
lba, iTask.err_str);
r = iTask.err_code;
goto out_unlock;
}
Expand All @@ -1255,6 +1275,7 @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,

out_unlock:
qemu_mutex_unlock(&iscsilun->mutex);
g_free(iTask.err_str);
return r;
}

Expand Down

0 comments on commit e38bc23

Please sign in to comment.