Skip to content

Commit

Permalink
set proper iSCSI status for reservation conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
folkertvanheusden committed Oct 10, 2024
1 parent 4aaa1e2 commit 9b3fb0c
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 23 deletions.
18 changes: 13 additions & 5 deletions iscsi-pdu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ std::optional<iscsi_response_set> 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 { };
Expand Down Expand Up @@ -482,7 +482,12 @@ std::optional<iscsi_response_set> 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<uint8_t> 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");
}
Expand All @@ -507,7 +512,7 @@ std::optional<iscsi_response_set> 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");
}
Expand Down Expand Up @@ -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<uint8_t> & scsi_sense_data, std::optional<std::pair<residual, uint32_t> > has_residual)
bool iscsi_pdu_scsi_response::set(const iscsi_pdu_scsi_cmd & reply_to, const std::vector<uint8_t> & scsi_sense_data, std::optional<std::pair<residual, uint32_t> > has_residual, const std::optional<uint8_t> 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;
Expand Down Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion iscsi-pdu.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t> & scsi_sense_data, std::optional<std::pair<residual, uint32_t> > has_residual);
bool set(const iscsi_pdu_scsi_cmd & reply_to, const std::vector<uint8_t> & scsi_sense_data, std::optional<std::pair<residual, uint32_t> > has_residual, const std::optional<uint8_t> 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); }
Expand Down
10 changes: 3 additions & 7 deletions scsi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ std::optional<scsi_response> 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");
Expand Down Expand Up @@ -668,12 +668,8 @@ std::optional<scsi_response> 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");
Expand Down
18 changes: 9 additions & 9 deletions scsi.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,6 @@ class scsi
std::optional<std::vector<uint8_t> > validate_request(const uint64_t lba, const uint32_t n_blocks, const uint8_t *const CDB) const;
std::optional<std::vector<uint8_t> > validate_request(const uint64_t lba) const;

std::vector<uint8_t> error_reservation_conflict_1() const;
std::vector<uint8_t> error_reservation_conflict_2() const;
std::vector<uint8_t> error_not_implemented() const;
std::vector<uint8_t> error_write_error() const;
std::vector<uint8_t> error_compare_and_write_count() const;
std::vector<uint8_t> error_out_of_range() const;
std::vector<uint8_t> error_miscompare() const;
std::vector<uint8_t> error_invalid_field() const;

public:
scsi(backend *const b, const int trim_level, io_stats_t *const is);
virtual ~scsi();
Expand Down Expand Up @@ -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<scsi_response> send(const uint64_t lun, const uint8_t *const CDB, const size_t size, std::pair<uint8_t *, size_t> data);

std::vector<uint8_t> error_reservation_conflict_1() const;
std::vector<uint8_t> error_reservation_conflict_2() const;
std::vector<uint8_t> error_not_implemented() const;
std::vector<uint8_t> error_write_error() const;
std::vector<uint8_t> error_compare_and_write_count() const;
std::vector<uint8_t> error_out_of_range() const;
std::vector<uint8_t> error_miscompare() const;
std::vector<uint8_t> error_invalid_field() const;
};
2 changes: 1 addition & 1 deletion server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down

0 comments on commit 9b3fb0c

Please sign in to comment.