Skip to content

Commit cc36385

Browse files
folkertvanheusdenfolkertvanheusden
committed
process WRITE fragments correctly
1 parent f695a8d commit cc36385

File tree

3 files changed

+55
-27
lines changed

3 files changed

+55
-27
lines changed

‎iscsi-pdu.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -446,14 +446,15 @@ std::optional<iscsi_response_set> iscsi_pdu_scsi_cmd::get_response(scsi *const s
446446
const uint64_t lun = get_LUN_nr();
447447
DOLOG(logging::ll_debug, "iscsi_pdu_scsi_cmd::get_response", ses->get_endpoint_name(), "working on ITT %08x for LUN %" PRIu64, get_Itasktag(), lun);
448448

449-
auto scsi_reply = sd->send(lun, get_CDB(), 16, data);
449+
uint64_t iscsi_expected = get_ExpDatLen();
450+
auto scsi_reply = sd->send(lun, get_CDB(), 16, data);
450451
if (scsi_reply.has_value() == false) {
451452
DOLOG(logging::ll_warning, "iscsi_pdu_scsi_cmd::get_response", ses->get_endpoint_name(), "scsi::send returned nothing");
452453
return { };
453454
}
454455

455456
iscsi_response_set response;
456-
bool ok { true };
457+
bool ok = true;
457458

458459
if (scsi_reply.value().io.is_inline) {
459460
auto pdu_data_in = new iscsi_pdu_scsi_data_in(ses); // 0x25
@@ -499,7 +500,6 @@ std::optional<iscsi_response_set> iscsi_pdu_scsi_cmd::get_response(scsi *const s
499500
else if (scsi_reply.value().type == ir_r2t) {
500501
assert(scsi_reply.value().amount_of_data_expected.has_value());
501502
uint64_t scsi_expected = scsi_reply.value().amount_of_data_expected.value();
502-
uint64_t iscsi_expected = get_ExpDatLen();
503503
bool r2t_would_under_or_overflow = scsi_expected != iscsi_expected;
504504

505505
if (r2t_would_under_or_overflow) {

‎scsi.cpp

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ scsi::~scsi()
8787
#endif
8888
}
8989

90-
std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const CDB, const size_t size, std::pair<uint8_t *, size_t> data)
90+
// data:
91+
// 0: pointer
92+
// 1: size of data
93+
// 2: how much is allowed to be written of it by iSCSI layer
94+
std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const CDB, const size_t size, std::tuple<uint8_t *, size_t> data)
9195
{
9296
assert(size >= 16);
9397

@@ -393,32 +397,53 @@ std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const
393397
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "WRITE_1x parameters invalid");
394398
response.sense_data = vr.value();
395399
}
396-
else if (data.first) {
397-
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "write command includes data (%zu bytes)", data.second);
400+
else if (std::get<0>(data)) {
401+
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "write command includes data (%zu bytes)", std::get<1>(data));
398402

399403
size_t expected_size = transfer_length * backend_block_size;
400-
size_t received_size = data.second;
404+
size_t received_size = std::get<1>(data);
401405
size_t received_blocks = received_size / backend_block_size;
402406
if (received_blocks)
403407
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "WRITE_xx to LBA %" PRIu64 " is %zu in bytes, %zu bytes", lba, lba * backend_block_size, received_size);
404408

405-
bool ok = true;
409+
bool ok = true;
410+
scsi::scsi_rw_result rc = scsi_rw_result::rw_ok;
411+
412+
uint32_t work_n_blocks = std::min(transfer_length, uint32_t(received_blocks));
406413
if (received_blocks > 0) {
407-
auto rc = write(lba, received_blocks, data.first);
414+
rc = write(lba, work_n_blocks, std::get<0>(data));
415+
ok = rc == scsi_rw_result::rw_ok;
416+
}
417+
418+
size_t fragment_size = received_size - work_n_blocks * backend_block_size;
419+
if (ok && fragment_size > 0 && fragment_size < backend_block_size) {
420+
uint8_t *temp_buffer = new uint8_t[backend_block_size];
421+
422+
// received_blocks is rounded down above
423+
rc = read(lba + work_n_blocks, 1, temp_buffer);
424+
if (rc == scsi_rw_result::rw_ok) {
425+
memcpy(temp_buffer, &std::get<0>(data)[work_n_blocks * backend_block_size], fragment_size);
426+
rc = write(lba + received_blocks, 1, temp_buffer);
427+
}
428+
429+
ok = rc == scsi_rw_result::rw_ok;
408430

431+
delete [] temp_buffer;
432+
}
433+
434+
if (ok) {
435+
if (response.fua)
436+
this->sync();
437+
}
438+
else {
409439
if (rc == scsi_rw_result::rw_fail_rw) {
410-
DOLOG(logging::ll_error, "scsi::send", lun_identifier, "WRITE_xx, general write error");
440+
DOLOG(logging::ll_error, "scsi::send", lun_identifier, "WRITE_xx, general i/o error");
411441
response.sense_data = error_write_error();
412-
ok = false;
413442
}
414443
else if (rc == rw_fail_locked) {
415-
DOLOG(logging::ll_error, "scsi::send", lun_identifier, "WRITE_xx, failed writing due to reservations");
444+
DOLOG(logging::ll_error, "scsi::send", lun_identifier, "WRITE_xx, failed i/o due to reservations");
416445
response.sense_data = error_reservation_conflict_1();
417-
ok = false;
418446
}
419-
420-
if (response.fua)
421-
this->sync();
422447
}
423448

424449
if (ok) {
@@ -429,7 +454,10 @@ std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const
429454
else { // allow R2T packets to come in
430455
response.type = ir_r2t;
431456
response.r2t.buffer_lba = lba;
432-
response.r2t.bytes_left = (transfer_length - received_blocks) * backend_block_size;
457+
if (received_blocks <= transfer_length)
458+
response.r2t.bytes_left = (transfer_length - received_blocks) * backend_block_size;
459+
else
460+
response.r2t.bytes_left = 0;
433461
response.r2t.bytes_done = received_blocks * backend_block_size;
434462
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "starting R2T with %u bytes left (LBA: %" PRIu64 ", offset %u)", response.r2t.bytes_left, response.r2t.buffer_lba, response.r2t.bytes_done);
435463
}
@@ -569,8 +597,8 @@ std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const
569597

570598
auto block_size = b->get_block_size();
571599
auto expected_data_size = block_size * block_count * 2;
572-
if (expected_data_size != data.second)
573-
DOLOG(logging::ll_warning, "scsi::send", lun_identifier, "COMPARE AND WRITE: data count mismatch (%zu versus %zu)", size_t(expected_data_size), data.second);
600+
if (expected_data_size != std::get<1>(data))
601+
DOLOG(logging::ll_warning, "scsi::send", lun_identifier, "COMPARE AND WRITE: data count mismatch (%zu versus %zu)", size_t(expected_data_size), std::get<1>(data));
574602

575603
response.amount_of_data_expected = expected_data_size;
576604

@@ -584,7 +612,7 @@ std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const
584612
response.sense_data = error_compare_and_write_count();
585613
}
586614
else {
587-
auto result = cmpwrite(lba, block_count, &data.first[block_count * block_size], &data.first[0]);
615+
auto result = cmpwrite(lba, block_count, &std::get<0>(data)[block_count * block_size], &std::get<0>(data)[0]);
588616

589617
if (result == scsi_rw_result::rw_ok)
590618
response.type = ir_empty_sense;
@@ -646,9 +674,9 @@ std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const
646674
else if (opcode == o_unmap) {
647675
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "UNMAP");
648676

649-
const uint8_t *const pd = data.first;
677+
const uint8_t *const pd = std::get<0>(data);
650678
scsi_rw_result rc = rw_ok;
651-
for(size_t i=8; i<data.second; i+= 16) {
679+
for(size_t i=8; i<std::get<1>(data); i+= 16) {
652680
uint64_t lba = get_uint64_t(&pd[i]);
653681
uint32_t transfer_length = get_uint32_t(&pd[i + 8]);
654682

@@ -714,10 +742,10 @@ std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const
714742
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "WRITE_SAME parameters invalid");
715743
response.sense_data = vr.value();
716744
}
717-
else if (data.first) {
718-
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "WRITE SAME command includes data (%zu bytes)", data.second);
745+
else if (std::get<0>(data)) {
746+
DOLOG(logging::ll_debug, "scsi::send", lun_identifier, "WRITE SAME command includes data (%zu bytes)", std::get<1>(data));
719747

720-
size_t received_size = data.second;
748+
size_t received_size = std::get<1>(data);
721749
size_t received_blocks = received_size / backend_block_size;
722750

723751
bool ok = true;
@@ -735,7 +763,7 @@ std::optional<scsi_response> scsi::send(const uint64_t lun, const uint8_t *const
735763
for(uint32_t i=0; i<transfer_length; i++) {
736764
rc = response.r2t.write_same_is_unmap ?
737765
trim(lba, 1) :
738-
write(lba, 1, data.first);
766+
write(lba, 1, std::get<0>(data));
739767
if (rc != rw_ok)
740768
break;
741769

‎scsi.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,5 +135,5 @@ class scsi
135135
scsi_rw_result read (const uint64_t block_nr, const uint32_t n_blocks, uint8_t *const data);
136136
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);
137137

138-
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);
138+
std::optional<scsi_response> send(const uint64_t lun, const uint8_t *const CDB, const size_t size, std::tuple<uint8_t *, size_t> data);
139139
};

0 commit comments

Comments
 (0)