Fix data compression mode page size, update unit tests

cpp/base/primary_device.cpp

@@ -128,8 +128,8 @@ void PrimaryDevice::TestUnitReady()
 
 void PrimaryDevice::Inquiry()
 {
-    // EVPD and page code check
-    if ((GetCdbByte(1) & 0x01) || GetCdbByte(2)) {
+    // EVPD, CMDDT and page code check
+    if ((GetCdbByte(1) & 0x03) || GetCdbByte(2)) {
         throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_cdb);
     }
 

cpp/base/property_handler.cpp

@@ -23,7 +23,7 @@ void PropertyHandler::Init(const string &filenames, const property_map &cmd_prop
 
     property_map properties;
 
-    // Always parse the optional global property file
+    // Parse the optional global property file unless disabled
     if (!ignore_conf && exists(path(CONFIGURATION))) {
         ParsePropertyFile(properties, CONFIGURATION, true);
     }

cpp/devices/host_services.cpp

@@ -278,7 +278,6 @@ void HostServices::AddRealtimeClockPage(map<int, vector<byte>> &pages, bool chan
 
 int HostServices::WriteData(span<const uint8_t> buf, scsi_command command)
 {
-    assert(command == scsi_command::execute_operation);
     if (command != scsi_command::execute_operation) {
         throw scsi_exception(sense_key::aborted_command);
     }

cpp/devices/scsi_hd.cpp

@@ -112,7 +112,7 @@ void ScsiHd::AddFormatPage(map<int, vector<byte>> &pages, bool changeable) const
     vector<byte> buf(24);
 
     if (changeable) {
-        // The sector size is simulated to be changeable in multiples of 4 with a maximum of 4096 bytes per sector.
+        // The sector size is simulated to be changeable in multiples of 4.
         // See the MODE SELECT implementation for details.
         SetInt16(buf, 12, 0x1ffc);
 

cpp/devices/tape.cpp

@@ -327,7 +327,7 @@ void Tape::AddModeBlockDescriptor(map<int, vector<byte>> &pages) const
 
 void Tape::AddDataCompressionPage(map<int, vector<byte>> &pages) const
 {
-    vector<byte> buf(14);
+    vector<byte> buf(16);
 
     pages[15] = buf;
 }

cpp/s2p/s2p_parser.cpp

@@ -34,7 +34,8 @@ void S2pParser::Banner(bool usage) const
             << "                              default is device-specific and usually SCSI-2.\n"
             << "  --name/-n PRODUCT_NAME      Optional product name for SCSI INQUIRY command,\n"
             << "                              format is VENDOR:PRODUCT:REVISION.\n"
-            << "  --block-size/-b BLOCK_SIZE  Optional block (sector) size.\n"
+            << "  --block-size/-b BLOCK_SIZE  Optional block size, 4-4096 bytes\n"
+            << "                              in multiples of 4.\n"
             << "  --caching-mode/-m MODE      Caching mode (piscsi|write-through|linux\n"
             << "                              |linux-optimized), default currently is PiSCSI\n"
             << "                              compatible caching.\n"
@@ -45,8 +46,7 @@ void S2pParser::Banner(bool usage) const
             << "  --property/-c KEY=VALUE     Sets a configuration property.\n"
             << "  --property-files/-C         List of configuration property files.\n"
             << "  --log-level/-L LEVEL        Log level (trace|debug|info|warning|error|\n"
-            << "                              critical|off),\n"
-            << "                              default is 'info'.\n"
+            << "                              critical|off), default is 'info'.\n"
             << "  --log-pattern/-l PATTERN    The spdlog pattern to use for logging.\n"
             << "  --token-file/-P FILE        Access token file.\n"
             << "  --port/-p PORT              s2p server port, default is 6868.\n"

cpp/s2pctl/sp2ctl_core.cpp

@@ -32,8 +32,8 @@ void S2pCtl::Banner(bool usage) const
             << "                                 |unprotect).\n"
             << "  --type/-t TYPE                 Optional device type\n"
             << "                                 (schd|scrm|sccd|scmo|scdp|sclp|schs|sahd).\n"
-            << "  --block-size/-b BLOCK_SIZE     Optional block size\n"
-            << "                                 (256|512|1024|2048|4096).\n"
+            << "  --block-size/-b BLOCK_SIZE     Optional block size, 4-4096 bytes\n"
+            << "                                 in multiples of 4.\n"
             << "  --caching-mode/-m MODE         Caching mode (piscsi|write-through|linux\n"
             << "                                 |linux-optimized), default is PiSCSI\n"
             << "                                 compatible caching.\n"
@@ -77,7 +77,8 @@ void S2pCtl::Banner(bool usage) const
             << "                                 s2p requires authentication.\n"
             << "  --list-settings/-s             List s2p settings.\n"
             << "  --list-statistics/-S           List s2p statistics.\n"
-            << "  --persist                      Save the current configuration to /etc/s2p.conf.\n"
+            << "  --persist                      Save the current configuration to\n"
+            << "                                 /etc/s2p.conf.\n"
             << "  --version/-v                   Display the program version.\n"
             << "  --server-version/-V            Display the s2p server version.\n"
             << "  --shut-down/-X                 Shut down s2p.\n";

cpp/test/controller_factory_test.cpp

@@ -72,7 +72,18 @@ TEST(ControllerFactoryTest, AttachToController)
 
 TEST(ControllerFactoryTest, ProcessOnController)
 {
+    const int VALID_ID = 0;
+    const int INVALID_ID = 1;
+
+    MockBus bus;
     ControllerFactory controller_factory;
 
-    EXPECT_EQ(shutdown_mode::none, controller_factory.ProcessOnController(0));
+    EXPECT_EQ(shutdown_mode::none, controller_factory.ProcessOnController(VALID_ID));
+
+    const auto device = make_shared<MockPrimaryDevice>(0);
+    EXPECT_TRUE(controller_factory.AttachToController(bus, VALID_ID, device));
+
+    EXPECT_EQ(shutdown_mode::none, controller_factory.ProcessOnController(VALID_ID));
+
+    EXPECT_EQ(shutdown_mode::none, controller_factory.ProcessOnController(INVALID_ID));
 }

cpp/test/host_services_test.cpp

@@ -208,6 +208,8 @@ TEST(HostServicesTest, WriteData)
     auto [controller, services] = CreateDevice(SCHS);
     array<uint8_t, 1> buf = { };
 
+    EXPECT_THROW(services->WriteData(buf, scsi_command::test_unit_ready), scsi_exception)<< "Illegal command";
+
     EXPECT_EQ(0, services->WriteData(buf, scsi_command::execute_operation));
 
     controller->SetCdbByte(8, 1);

cpp/test/tape_test.cpp

@@ -24,7 +24,7 @@ static void ValidateModePages(map<int, vector<byte>> &pages)
     EXPECT_EQ(12U, pages[1].size());
     EXPECT_EQ(16U, pages[2].size());
     EXPECT_EQ(8U, pages[10].size());
-    EXPECT_EQ(14U, pages[15].size());
+    EXPECT_EQ(16U, pages[15].size());
     EXPECT_EQ(16U, pages[16].size());
     EXPECT_EQ(8U, pages[17].size());
 }