From 9b3fb0ce73341e15d0607adfed09d8d7eaf53724 Mon Sep 17 00:00:00 2001 From: folkert van heusden Date: Thu, 10 Oct 2024 14:29:35 +0200 Subject: [PATCH] set proper iSCSI status for reservation conflicts --- iscsi-pdu.cpp | 18 +++++++++++++----- iscsi-pdu.h | 2 +- scsi.cpp | 10 +++------- scsi.h | 18 +++++++++--------- server.cpp | 2 +- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/iscsi-pdu.cpp b/iscsi-pdu.cpp index 1e4b52e..495ea2a 100644 --- a/iscsi-pdu.cpp +++ b/iscsi-pdu.cpp @@ -435,7 +435,7 @@ std::optional iscsi_pdu_scsi_cmd::get_response(scsi *const s iscsi_response_set response; auto *pdu_scsi_response = new iscsi_pdu_scsi_response(ses) /* 0x21 */; - if (pdu_scsi_response->set(*this, { }, { }) == false) { + if (pdu_scsi_response->set(*this, { }, { }, { }) == false) { DOLOG(logging::ll_info, "iscsi_pdu_scsi_cmd::get_response", ses->get_endpoint_name(), "iscsi_pdu_scsi_response::set returned error"); return { }; @@ -482,7 +482,12 @@ std::optional iscsi_pdu_scsi_cmd::get_response(scsi *const s auto *temp = new iscsi_pdu_scsi_response(ses) /* 0x21 */; DOLOG(logging::ll_debug, "iscsi_pdu_scsi_cmd::get_response", ses->get_endpoint_name(), "sending SCSI response with %zu sense bytes", scsi_reply.value().sense_data.size()); - if (temp->set(*this, scsi_reply.value().sense_data, { }) == false) { + std::optional iscsi_status; + auto & sense_data = scsi_reply.value().sense_data; + if (sense_data == sd->error_reservation_conflict_1()) + iscsi_status = 0x18; // RESERVATION CONFLICT + + if (temp->set(*this, sense_data, { }, iscsi_status) == false) { ok = false; DOLOG(logging::ll_info, "iscsi_pdu_scsi_cmd::get_response", ses->get_endpoint_name(), "iscsi_pdu_scsi_response::set returned error"); } @@ -507,7 +512,7 @@ std::optional iscsi_pdu_scsi_cmd::get_response(scsi *const s else assert(0); - if (temp->set(*this, { }, residual_state) == false) { + if (temp->set(*this, { }, residual_state, { }) == false) { ok = false; DOLOG(logging::ll_info, "iscsi_pdu_scsi_cmd::get_response", ses->get_endpoint_name(), "iscsi_pdu_scsi_response::set returned error"); } @@ -563,7 +568,7 @@ iscsi_pdu_scsi_response::~iscsi_pdu_scsi_response() delete [] pdu_response_data.first; } -bool iscsi_pdu_scsi_response::set(const iscsi_pdu_scsi_cmd & reply_to, const std::vector & scsi_sense_data, std::optional > has_residual) +bool iscsi_pdu_scsi_response::set(const iscsi_pdu_scsi_cmd & reply_to, const std::vector & scsi_sense_data, std::optional > has_residual, const std::optional iscsi_status) { size_t sense_data_size = scsi_sense_data.size(); size_t reply_data_plus_sense_header = sense_data_size > 0 ? 2 + sense_data_size : 0; @@ -599,7 +604,10 @@ bool iscsi_pdu_scsi_response::set(const iscsi_pdu_scsi_cmd & reply_to, const std if (pdu_response_data.second) { DOLOG(logging::ll_warning, "iscsi_pdu_scsi_response::set", ses->get_endpoint_name(), "CHECK CONDITION"); - pdu_response->status = 0x02; // check condition + if (iscsi_status.has_value()) + pdu_response->status = iscsi_status.value(); + else + pdu_response->status = 0x02; // check condition pdu_response->response = 0x01; // target failure pdu_response->ExpDataSN = 0; diff --git a/iscsi-pdu.h b/iscsi-pdu.h index 11d3881..bd12fa0 100644 --- a/iscsi-pdu.h +++ b/iscsi-pdu.h @@ -443,7 +443,7 @@ class iscsi_pdu_scsi_response : public iscsi_pdu_bhs // 0x21 iscsi_pdu_scsi_response(session *const ses); virtual ~iscsi_pdu_scsi_response(); - bool set(const iscsi_pdu_scsi_cmd & reply_to, const std::vector & scsi_sense_data, std::optional > has_residual); + bool set(const iscsi_pdu_scsi_cmd & reply_to, const std::vector & scsi_sense_data, std::optional > has_residual, const std::optional iscsi_status); void set_residual_count(const uint32_t v) { pdu_response->ResidualCt = v; } void set_overflow_flag (const bool state) { set_bits(&pdu_response->b2, 2, 1, state); } diff --git a/scsi.cpp b/scsi.cpp index 7730a55..6f69dc8 100644 --- a/scsi.cpp +++ b/scsi.cpp @@ -122,7 +122,7 @@ std::optional scsi::send(const uint64_t lun, const uint8_t *const else if (opcode == o_mode_sense_6) { // 0x1a if (locking_status() == l_locked_other) { DOLOG(logging::ll_error, "scsi::send", lun_identifier, "MODE SENSE 6 failed due to reservations"); - response.sense_data = error_reservation_conflict_2(); + response.sense_data = error_reservation_conflict_1(); } else { DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "MODE SENSE 6"); @@ -668,12 +668,8 @@ std::optional scsi::send(const uint64_t lun, const uint8_t *const } else if (opcode == o_release_6) { DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "RELEASE 6"); - if (unlock_device()) - response.type = ir_empty_sense; - else { - DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "RELEASE 6 failed"); - response.sense_data = error_reservation_conflict_1(); - } + unlock_device(); + response.type = ir_empty_sense; } else if (opcode == o_unmap) { DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "UNMAP"); diff --git a/scsi.h b/scsi.h index 7d8520a..52fc44c 100644 --- a/scsi.h +++ b/scsi.h @@ -64,15 +64,6 @@ class scsi std::optional > validate_request(const uint64_t lba, const uint32_t n_blocks, const uint8_t *const CDB) const; std::optional > validate_request(const uint64_t lba) const; - std::vector error_reservation_conflict_1() const; - std::vector error_reservation_conflict_2() const; - std::vector error_not_implemented() const; - std::vector error_write_error() const; - std::vector error_compare_and_write_count() const; - std::vector error_out_of_range() const; - std::vector error_miscompare() const; - std::vector error_invalid_field() const; - public: scsi(backend *const b, const int trim_level, io_stats_t *const is); virtual ~scsi(); @@ -135,4 +126,13 @@ class scsi scsi_rw_result cmpwrite(const uint64_t block_nr, const uint32_t n_blocks, const uint8_t *const write_data, const uint8_t *const compare_data); std::optional send(const uint64_t lun, const uint8_t *const CDB, const size_t size, std::pair data); + + std::vector error_reservation_conflict_1() const; + std::vector error_reservation_conflict_2() const; + std::vector error_not_implemented() const; + std::vector error_write_error() const; + std::vector error_compare_and_write_count() const; + std::vector error_out_of_range() const; + std::vector error_miscompare() const; + std::vector error_invalid_field() const; }; diff --git a/server.cpp b/server.cpp index 0059290..62405cd 100644 --- a/server.cpp +++ b/server.cpp @@ -417,7 +417,7 @@ bool server::push_response(com_client *const cc, session *const ses, iscsi_pdu_b else if (scsi_has > iscsi_wants) residual_state = { iSR_OVERFLOW, scsi_has - iscsi_wants }; - if (temp->set(reply_to, { }, residual_state) == false) { + if (temp->set(reply_to, { }, residual_state, { }) == false) { ok = false; DOLOG(logging::ll_info, "server::push_response", cc->get_endpoint_name(), "iscsi_pdu_scsi_response::set returned error"); }