Re: [PATCH v7 10/10] block/iscsi: add persistent reservation in/out driver
On Fri, Jul 05, 2024 at 06:56:14PM +0800, Changqi Lu wrote:
> Add persistent reservation in/out operations for iscsi driver.
> The following methods are implemented: bdrv_co_pr_read_keys,
> bdrv_co_pr_read_reservation, bdrv_co_pr_register, bdrv_co_pr_reserve,
> bdrv_co_pr_release, bdrv_co_pr_clear and bdrv_co_pr_preempt.
>
> Signed-off-by: Changqi Lu
> Signed-off-by: zhenwei pi
> ---
> block/iscsi.c | 431 ++
> 1 file changed, 431 insertions(+)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 2ff14b7472..9a546f48de 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -96,6 +96,7 @@ typedef struct IscsiLun {
> unsigned long *allocmap_valid;
> long allocmap_size;
> int cluster_size;
> +uint8_t pr_cap;
> bool use_16_for_rw;
> bool write_protected;
> bool lbpme;
> @@ -280,6 +281,10 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int
> status,
> iTask->err_code = -error;
> iTask->err_str = g_strdup(iscsi_get_error(iscsi));
> }
> +} else if (status == SCSI_STATUS_RESERVATION_CONFLICT) {
> +iTask->err_code = -EBADE;
> +error_report("iSCSI Persistent Reservation Conflict: %s",
> + iscsi_get_error(iscsi));
> }
> }
> }
> @@ -1792,6 +1797,52 @@ static void iscsi_save_designator(IscsiLun *lun,
> }
> }
>
> +static void iscsi_get_pr_cap_sync(IscsiLun *iscsilun, Error **errp)
> +{
> +struct scsi_task *task = NULL;
> +struct scsi_persistent_reserve_in_report_capabilities *rc = NULL;
> +int retries = ISCSI_CMD_RETRIES;
> +int xferlen = sizeof(struct
> scsi_persistent_reserve_in_report_capabilities);
> +
> +do {
> +if (task != NULL) {
> +scsi_free_scsi_task(task);
> +task = NULL;
> +}
> +
> +task = iscsi_persistent_reserve_in_sync(iscsilun->iscsi,
> + iscsilun->lun, SCSI_PR_IN_REPORT_CAPABILITIES, xferlen);
> +if (task != NULL && task->status == SCSI_STATUS_GOOD) {
> +rc = scsi_datain_unmarshall(task);
> +if (rc == NULL) {
> +error_setg(errp,
> +"iSCSI: Failed to unmarshall report capabilities data.");
> +} else {
> +iscsilun->pr_cap =
> +
> scsi_pr_cap_to_block(rc->persistent_reservation_type_mask);
> +iscsilun->pr_cap |= (rc->ptpl_a) ? BLK_PR_CAP_PTPL : 0;
> +}
> +break;
> +}
> +
> +if (task != NULL && task->status == SCSI_STATUS_CHECK_CONDITION
> +&& task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
> +break;
> +}
> +
> +} while (task != NULL && task->status == SCSI_STATUS_CHECK_CONDITION
> + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION
> + && retries-- > 0);
The if statement is the same condition as the while statement (except
for the retry counter)? It looks like retrying logic won't work in
practice because the if statement breaks early.
> +
> +if (task == NULL || task->status != SCSI_STATUS_GOOD) {
> +error_setg(errp, "iSCSI: failed to send report capabilities
> command");
> +}
Did you test this function against a SCSI target that does not implement
the optional PERSISTENT RESERVE IN operation? iscsi_open() must succeed
when the target does not implement this.
> +
> +if (task) {
> +scsi_free_scsi_task(task);
> +}
> +}
> +
> static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>Error **errp)
> {
> @@ -2024,6 +2075,11 @@ static int iscsi_open(BlockDriverState *bs, QDict
> *options, int flags,
> bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
> }
>
> +iscsi_get_pr_cap_sync(iscsilun, _err);
> +if (local_err != NULL) {
> +error_propagate(errp, local_err);
> +ret = -EINVAL;
> +}
> out:
> qemu_opts_del(opts);
> g_free(initiator_name);
> @@ -2110,6 +2166,8 @@ static void iscsi_refresh_limits(BlockDriverState *bs,
> Error **errp)
> bs->bl.opt_transfer = pow2floor(iscsilun->bl.opt_xfer_len *
> iscsilun->block_size);
> }
> +
> +bs->bl.pr_cap = iscsilun->pr_cap;
> }
>
> /* Note that this will not re-establish a connection with an iSCSI target -
> it
> @@ -2408,6 +2466,371 @@ out_unlock:
> return r;
> }
>
> +static int coroutine_fn
> +iscsi_co_pr_read_keys(BlockDriverState *bs, uint32_t *generation,
> + uint32_t num_keys, uint64_t *keys)
> +{
> +IscsiLun *iscsilun = bs->opaque;
> +QEMUIOVector qiov;
> +struct IscsiTask iTask;
> +int xferlen = sizeof(struct scsi_persistent_reserve_in_read_keys) +
> + sizeof(uint64_t) * num_keys;
> +uint8_t *buf =
[PATCH v7 10/10] block/iscsi: add persistent reservation in/out driver
Add persistent reservation in/out operations for iscsi driver.
The following methods are implemented: bdrv_co_pr_read_keys,
bdrv_co_pr_read_reservation, bdrv_co_pr_register, bdrv_co_pr_reserve,
bdrv_co_pr_release, bdrv_co_pr_clear and bdrv_co_pr_preempt.
Signed-off-by: Changqi Lu
Signed-off-by: zhenwei pi
---
block/iscsi.c | 431 ++
1 file changed, 431 insertions(+)
diff --git a/block/iscsi.c b/block/iscsi.c
index 2ff14b7472..9a546f48de 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -96,6 +96,7 @@ typedef struct IscsiLun {
unsigned long *allocmap_valid;
long allocmap_size;
int cluster_size;
+uint8_t pr_cap;
bool use_16_for_rw;
bool write_protected;
bool lbpme;
@@ -280,6 +281,10 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int
status,
iTask->err_code = -error;
iTask->err_str = g_strdup(iscsi_get_error(iscsi));
}
+} else if (status == SCSI_STATUS_RESERVATION_CONFLICT) {
+iTask->err_code = -EBADE;
+error_report("iSCSI Persistent Reservation Conflict: %s",
+ iscsi_get_error(iscsi));
}
}
}
@@ -1792,6 +1797,52 @@ static void iscsi_save_designator(IscsiLun *lun,
}
}
+static void iscsi_get_pr_cap_sync(IscsiLun *iscsilun, Error **errp)
+{
+struct scsi_task *task = NULL;
+struct scsi_persistent_reserve_in_report_capabilities *rc = NULL;
+int retries = ISCSI_CMD_RETRIES;
+int xferlen = sizeof(struct
scsi_persistent_reserve_in_report_capabilities);
+
+do {
+if (task != NULL) {
+scsi_free_scsi_task(task);
+task = NULL;
+}
+
+task = iscsi_persistent_reserve_in_sync(iscsilun->iscsi,
+ iscsilun->lun, SCSI_PR_IN_REPORT_CAPABILITIES, xferlen);
+if (task != NULL && task->status == SCSI_STATUS_GOOD) {
+rc = scsi_datain_unmarshall(task);
+if (rc == NULL) {
+error_setg(errp,
+"iSCSI: Failed to unmarshall report capabilities data.");
+} else {
+iscsilun->pr_cap =
+scsi_pr_cap_to_block(rc->persistent_reservation_type_mask);
+iscsilun->pr_cap |= (rc->ptpl_a) ? BLK_PR_CAP_PTPL : 0;
+}
+break;
+}
+
+if (task != NULL && task->status == SCSI_STATUS_CHECK_CONDITION
+&& task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
+break;
+}
+
+} while (task != NULL && task->status == SCSI_STATUS_CHECK_CONDITION
+ && task->sense.key == SCSI_SENSE_UNIT_ATTENTION
+ && retries-- > 0);
+
+if (task == NULL || task->status != SCSI_STATUS_GOOD) {
+error_setg(errp, "iSCSI: failed to send report capabilities command");
+}
+
+if (task) {
+scsi_free_scsi_task(task);
+}
+}
+
static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
@@ -2024,6 +2075,11 @@ static int iscsi_open(BlockDriverState *bs, QDict
*options, int flags,
bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
}
+iscsi_get_pr_cap_sync(iscsilun, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+}
out:
qemu_opts_del(opts);
g_free(initiator_name);
@@ -2110,6 +2166,8 @@ static void iscsi_refresh_limits(BlockDriverState *bs,
Error **errp)
bs->bl.opt_transfer = pow2floor(iscsilun->bl.opt_xfer_len *
iscsilun->block_size);
}
+
+bs->bl.pr_cap = iscsilun->pr_cap;
}
/* Note that this will not re-establish a connection with an iSCSI target - it
@@ -2408,6 +2466,371 @@ out_unlock:
return r;
}
+static int coroutine_fn
+iscsi_co_pr_read_keys(BlockDriverState *bs, uint32_t *generation,
+ uint32_t num_keys, uint64_t *keys)
+{
+IscsiLun *iscsilun = bs->opaque;
+QEMUIOVector qiov;
+struct IscsiTask iTask;
+int xferlen = sizeof(struct scsi_persistent_reserve_in_read_keys) +
+ sizeof(uint64_t) * num_keys;
+uint8_t *buf = g_malloc0(xferlen);
+int32_t num_collect_keys = 0;
+int r = 0;
+
+qemu_iovec_init_buf(, buf, xferlen);
+iscsi_co_init_iscsitask(iscsilun, );
+qemu_mutex_lock(>mutex);
+retry:
+iTask.task = iscsi_persistent_reserve_in_task(iscsilun->iscsi,
+ iscsilun->lun, SCSI_PR_IN_READ_KEYS, xferlen,
+ iscsi_co_generic_cb, );
+
+if (iTask.task == NULL) {
+qemu_mutex_unlock(>mutex);
+return -ENOMEM;
+}
+
+scsi_task_set_iov_in(iTask.task, (struct scsi_iovec *)qiov.iov, qiov.niov);
+iscsi_co_wait_for_task(, iscsilun);
+
+if (iTask.task != NULL) {
+scsi_free_scsi_task(iTask.task);
+iTask.task = NULL;
+}
+
+
