Added small fixes on Unmap and Verify test cases
14 views
Skip to first unread message
Mihai Musatoiu
Oct 27, 2016, 5:14:48 AM10/27/16
to libiscsi
Hi,
I have added a pull request with some small fixes on two Unmap and two Verify test cases, after noticing that in specific circumstances (given by target configuration and by LUN size, respectively) those tests were failing even if they were not supposed to.
* The Unmap tests were failing because currently Libiscsi sends out a fixed number of 256 block descriptors, a number that can be bigger than what the target is willing to accept (the parameter Maximum Unmap Block Descriptor Count sent by the target in the Block Limits Page response). In my case, the target had set that parameter to 16 so the test was failing as soon as Unmap was sending a request with 17 block descriptors.
From SBC-4, section 5.30.2, page 193:
If the total number of logical blocks specified in the UNMAP block descriptor data exceeds the value indicated in the MAXIMUM UNMAP LBA COUNT field in the Block Limits VPD page (see 6.6.4) or if the number of UNMAP block descriptors exceeds the value of the MAXIMUM UNMAP BLOCK DESCRIPTOR COUNT field in the Block Limits VPD page, then the device server shall terminate the command with CHECK CONDITION status with the sense key set to ILLEGAL REQUEST and the additional sense code set to INVALID FIELD IN PARAMETER LIST.
* The Verify tests were failing because Libiscsi currently assumes that a reasonably big LBA is 2^31, so it sends a Verify request addressed to that LBA and expects it to fail as that LBA is thought to be outside the LUN boundaries. That is however not true if the LUN is larger than 2^31 blocks (i.e. larger than 1 TB, for a block size of 512 bytes), which was the case when I was running the tests.
The diff report follows hereunder.
Regards,
Mihai Musatoiu
--
diff --git a/test-tool/test_unmap_0blocks.c b/test-tool/test_unmap_0blocks.c
index 2f85cfd..864d574 100644
--- a/test-tool/test_unmap_0blocks.c
+++ b/test-tool/test_unmap_0blocks.c
@@ -30,6 +30,7 @@ void
test_unmap_0blocks(void)
{
int i;
+ int max_nr_bdc = 256;
struct unmap_list list[257];
CHECK_FOR_DATALOSS;
@@ -60,19 +61,23 @@ test_unmap_0blocks(void)
return;
}
- logging(LOG_VERBOSE, "Test UNMAP of 0 blocks at LBA:0-255 with one descriptor per block");
- for (i = 0; i < 256; i++) {
+ if (inq_bl->max_unmap_bdc > 0 && max_nr_bdc > (int)inq_bl->max_unmap_bdc) {
+ max_nr_bdc = (int)inq_bl->max_unmap_bdc;
+ }
+
+ logging(LOG_VERBOSE, "Test UNMAP of 0 blocks at LBA:0-%d with one descriptor per block", max_nr_bdc - 1);
+ for (i = 0; i < max_nr_bdc; i++) {
list[i].lba = i;
list[i].num = 0;
UNMAP(sd, 0, list, i + 1,
EXPECT_STATUS_GOOD);
}
- logging(LOG_VERBOSE, "Test UNMAP of 0 blocks at LBA:0-255 with one descriptor per block, possibly \"overlapping\".");
- for (i = 0; i < 256; i++) {
+ logging(LOG_VERBOSE, "Test UNMAP of 0 blocks at LBA:0-%d with one descriptor per block, possibly \"overlapping\".", max_nr_bdc - 1);
+ for (i = 0; i < max_nr_bdc; i++) {
list[i].lba = i/2;
list[i].num = 0;
}
- UNMAP(sd, 0, list, 256,
+ UNMAP(sd, 0, list, max_nr_bdc,
EXPECT_STATUS_GOOD);
}
diff --git a/test-tool/test_unmap_simple.c b/test-tool/test_unmap_simple.c
index ce86c9f..835ac49 100644
--- a/test-tool/test_unmap_simple.c
+++ b/test-tool/test_unmap_simple.c
@@ -30,6 +30,7 @@ void
test_unmap_simple(void)
{
int i;
+ int max_nr_bdc = 256;
struct unmap_list list[257];
logging(LOG_VERBOSE, LOG_BLANK_LINE);
@@ -68,23 +69,27 @@ test_unmap_simple(void)
}
}
- logging(LOG_VERBOSE, "Test UNMAP of 1-256 blocks at the start of the "
- "LUN with one descriptor per block");
+ if (inq_bl->max_unmap_bdc > 0 && max_nr_bdc > (int)inq_bl->max_unmap_bdc) {
+ max_nr_bdc = (int)inq_bl->max_unmap_bdc;
+ }
- logging(LOG_VERBOSE, "Write 'a' to the first 256 LBAs");
- memset(scratch, 'a', 256 * block_size);
- WRITE10(sd, 0, 256 * block_size,
+ logging(LOG_VERBOSE, "Test UNMAP of 1-%d blocks at the start of the "
+ "LUN with one descriptor per block", max_nr_bdc);
+
+ logging(LOG_VERBOSE, "Write 'a' to the first %d LBAs", max_nr_bdc);
+ memset(scratch, 'a', max_nr_bdc * block_size);
+ WRITE10(sd, 0, max_nr_bdc * block_size,
block_size, 0, 0, 0, 0, 0, scratch,
EXPECT_STATUS_GOOD);
- for (i = 0; i < 256; i++) {
+ for (i = 0; i < max_nr_bdc; i++) {
list[i].lba = i;
list[i].num = 1;
UNMAP(sd, 0, list, i + 1,
EXPECT_STATUS_GOOD);
logging(LOG_VERBOSE, "Read blocks 0-%d", i);
- READ10(sd, NULL, 0, i * block_size,
+ READ10(sd, NULL, 0, (i + 1) * block_size,
block_size, 0, 0, 0, 0, 0, scratch,
EXPECT_STATUS_GOOD);
diff --git a/test-tool/test_verify10_0blocks.c b/test-tool/test_verify10_0blocks.c
index 49b6220..ca3b018 100644
--- a/test-tool/test_verify10_0blocks.c
+++ b/test-tool/test_verify10_0blocks.c
@@ -34,9 +34,11 @@ test_verify10_0blocks(void)
VERIFY10(sd, num_blocks + 1, 0, block_size, 0, 0, 1, NULL,
EXPECT_LBA_OOB);
- logging(LOG_VERBOSE, "Test VERIFY10 0-blocks at LBA==2^31");
- VERIFY10(sd, 0x80000000, 0, block_size, 0, 0, 1, NULL,
- EXPECT_LBA_OOB);
+ if (num_blocks - 1 < 0x80000000) {
+ logging(LOG_VERBOSE, "Test VERIFY10 0-blocks at LBA==2^31");
+ VERIFY10(sd, 0x80000000, 0, block_size, 0, 0, 1, NULL,
+ EXPECT_LBA_OOB);
+ }
logging(LOG_VERBOSE, "Test VERIFY10 0-blocks at LBA==-1");
VERIFY10(sd, -1, 0, block_size, 0, 0, 1, NULL,
diff --git a/test-tool/test_verify12_0blocks.c b/test-tool/test_verify12_0blocks.c
index a5bd481..1489d36 100644
--- a/test-tool/test_verify12_0blocks.c
+++ b/test-tool/test_verify12_0blocks.c
@@ -34,9 +34,11 @@ test_verify12_0blocks(void)
VERIFY12(sd, num_blocks + 1, 0, block_size, 0, 0, 1, NULL,
EXPECT_LBA_OOB);
- logging(LOG_VERBOSE, "Test VERIFY12 0-blocks at LBA==2^31");
- VERIFY12(sd, 0x80000000, 0, block_size, 0, 0, 1, NULL,
- EXPECT_LBA_OOB);
+ if (num_blocks - 1 < 0x80000000) {
+ logging(LOG_VERBOSE, "Test VERIFY12 0-blocks at LBA==2^31");
+ VERIFY12(sd, 0x80000000, 0, block_size, 0, 0, 1, NULL,
+ EXPECT_LBA_OOB);
+ }
logging(LOG_VERBOSE, "Test VERIFY12 0-blocks at LBA==-1");
VERIFY12(sd, -1, 0, block_size, 0, 0, 1, NULL,
Reply all
Reply to author
Forward
0 new messages