Issues 1179 and 1182 (#1232)

* Update logging

* Remove duplicate code

* Update unit tests

* Clean up includes

* Merge ProtobufSerializer into protobuf_util namespace

* Precompile regex

* Add const

* Add Split() convenience method, update log level/ID parsing

* Move log.h to legacy folder

* Elimininate gotos

* Fixes for gcc 13

* Update compiler flags

* Update default folder handling

* Use references instead of pointers

* Move code for better encapsulation

* Move code

* Remove unused method argument

* Move device logger

* Remove redundant to_string

* Rename for consistency

* Update handling of protobuf pointers

* Simplify protobuf usage

* Memory handling update

* Add hasher

cpp/piscsi/command_context.cpp

@@ -3,24 +3,50 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
-#include "shared/log.h"
-#include "generated/piscsi_interface.pb.h"
+#include "shared/piscsi_exceptions.h"
+#include "shared/protobuf_util.h"
 #include "command_context.h"
+#include <spdlog/spdlog.h>
 #include <iostream>
 
 using namespace std;
 using namespace piscsi_interface;
+using namespace protobuf_util;
 
-void CommandContext::Cleanup()
+bool CommandContext::ReadCommand()
 {
-	if (fd != -1) {
-		close(fd);
-		fd = -1;
+	// Read magic string
+	array<byte, 6> magic;
+	if (const size_t bytes_read = ReadBytes(fd, magic); bytes_read) {
+		if (bytes_read != magic.size() || memcmp(magic.data(), "RASCSI", magic.size())) {
+			throw io_exception("Invalid magic");
+		}
+
+		// Fetch the command
+		DeserializeMessage(fd, command);
+
+		return true;
 	}
+
+	return false;
+}
+
+void CommandContext::WriteResult(const PbResult& result) const
+{
+	// The descriptor is -1 when devices are not attached via the remote interface but by the piscsi tool
+	if (fd != -1) {
+		SerializeMessage(fd, result);
+	}
+}
+
+void CommandContext::WriteSuccessResult(PbResult& result) const
+{
+	result.set_status(true);
+	WriteResult(result);
 }
 
 bool CommandContext::ReturnLocalizedError(LocalizationKey key, const string& arg1, const string& arg2,
@@ -33,7 +59,7 @@ bool CommandContext::ReturnLocalizedError(LocalizationKey key, PbErrorCode error
 		const string& arg2, const string& arg3) const
 {
 	// For the logfile always use English
-	LOGERROR("%s", localizer.Localize(key, "en", arg1, arg2, arg3).c_str())
+	spdlog::error(localizer.Localize(key, "en", arg1, arg2, arg3));
 
 	return ReturnStatus(false, localizer.Localize(key, locale, arg1, arg2, arg3), error_code, false);
 }
@@ -42,17 +68,12 @@ bool CommandContext::ReturnStatus(bool status, const string& msg, PbErrorCode er
 {
 	// Do not log twice if logging has already been done in the localized error handling above
 	if (log && !status && !msg.empty()) {
-		LOGERROR("%s", msg.c_str())
+		spdlog::error(msg);
 	}
 
 	if (fd == -1) {
 		if (!msg.empty()) {
-			if (status) {
-				cerr << "Error: " << msg << endl;
-			}
-			else {
-				cout << msg << endl;
-			}
+			cerr << "Error: " << msg << endl;
 		}
 	}
 	else {
@@ -60,8 +81,18 @@ bool CommandContext::ReturnStatus(bool status, const string& msg, PbErrorCode er
 		result.set_status(status);
 		result.set_error_code(error_code);
 		result.set_msg(msg);
-		serializer.SerializeMessage(fd, result);
+		WriteResult(result);
 	}
 
 	return status;
 }
+
+bool CommandContext::ReturnSuccessStatus() const
+{
+	return ReturnStatus(true, "", PbErrorCode::NO_ERROR_CODE, true);
+}
+
+bool CommandContext::ReturnErrorStatus(const string& msg) const
+{
+	return ReturnStatus(false, msg, PbErrorCode::NO_ERROR_CODE, true);
+}

cpp/piscsi/command_context.h

@@ -3,15 +3,14 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
 #pragma once
 
-#include "generated/piscsi_interface.pb.h"
 #include "localizer.h"
-#include "shared/protobuf_serializer.h"
+#include "generated/piscsi_interface.pb.h"
 #include <string>
 
 using namespace std;
@@ -19,27 +18,36 @@ using namespace piscsi_interface;
 
 class CommandContext
 {
-	const ProtobufSerializer serializer;
-
-	const Localizer localizer;
-
-	string locale;
-
-	int fd;
 
 public:
 
-	CommandContext(const std::string& s = "", int f = -1) : locale(s), fd(f) {}
+	CommandContext(const PbCommand& cmd, string_view f, string_view l) : command(cmd), default_folder(f), locale(l) {}
+	explicit CommandContext(int f) : fd(f) {}
 	~CommandContext() = default;
 
-	void Cleanup();
-
-	const ProtobufSerializer& GetSerializer() const { return serializer; }
-	int GetFd() const { return fd; }
-	void SetFd(int f) { fd = f; }
-	bool IsValid() const { return fd != -1; }
+	string GetDefaultFolder() const { return default_folder; }
+	void SetDefaultFolder(string_view f) { default_folder = f; }
+	bool ReadCommand();
+	void WriteResult(const PbResult&) const;
+	void WriteSuccessResult(PbResult&) const;
+	const PbCommand& GetCommand() const { return command; }
 
 	bool ReturnLocalizedError(LocalizationKey, const string& = "", const string& = "", const string& = "") const;
 	bool ReturnLocalizedError(LocalizationKey, PbErrorCode, const string& = "", const string& = "", const string& = "") const;
-	bool ReturnStatus(bool = true, const string& = "", PbErrorCode = PbErrorCode::NO_ERROR_CODE, bool = true) const;
+	bool ReturnSuccessStatus() const;
+	bool ReturnErrorStatus(const string&) const;
+
+private:
+
+	bool ReturnStatus(bool, const string&, PbErrorCode, bool) const;
+
+	const Localizer localizer;
+
+	PbCommand command;
+
+	string default_folder;
+
+	string locale;
+
+	int fd = -1;
 };

cpp/piscsi/localizer.cpp

@@ -3,15 +3,13 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
 #include "localizer.h"
 #include <cassert>
-#include <string>
 #include <algorithm>
-#include <regex>
 
 using namespace std;
 
@@ -31,12 +29,12 @@ Localizer::Localizer()
 	Add(LocalizationKey::ERROR_OPERATION, "es", "Operación desconocida");
 	Add(LocalizationKey::ERROR_OPERATION, "zh", "未知操作");
 
-	Add(LocalizationKey::ERROR_LOG_LEVEL, "en", "Invalid log level %1");
-	Add(LocalizationKey::ERROR_LOG_LEVEL, "de", "Ungültiger Log-Level %1");
-	Add(LocalizationKey::ERROR_LOG_LEVEL, "sv", "Ogiltig loggnivå %1");
-	Add(LocalizationKey::ERROR_LOG_LEVEL, "fr", "Niveau de journalisation invalide %1");
-	Add(LocalizationKey::ERROR_LOG_LEVEL, "es", "Nivel de registro %1 no válido");
-	Add(LocalizationKey::ERROR_LOG_LEVEL, "zh", "无效的日志级别 %1");
+	Add(LocalizationKey::ERROR_LOG_LEVEL, "en", "Invalid log level '%1'");
+	Add(LocalizationKey::ERROR_LOG_LEVEL, "de", "Ungültiger Log-Level '%1'");
+	Add(LocalizationKey::ERROR_LOG_LEVEL, "sv", "Ogiltig loggnivå '%1'");
+	Add(LocalizationKey::ERROR_LOG_LEVEL, "fr", "Niveau de journalisation invalide '%1'");
+	Add(LocalizationKey::ERROR_LOG_LEVEL, "es", "Nivel de registro '%1' no válido");
+	Add(LocalizationKey::ERROR_LOG_LEVEL, "zh", "无效的日志级别 '%1'");
 
 	Add(LocalizationKey::ERROR_MISSING_DEVICE_ID, "en", "Missing device ID");
 	Add(LocalizationKey::ERROR_MISSING_DEVICE_ID, "de", "Fehlende Geräte-ID");
@@ -51,26 +49,26 @@ Localizer::Localizer()
 	Add(LocalizationKey::ERROR_MISSING_FILENAME, "fr", "Nom de fichier manquant");
 	Add(LocalizationKey::ERROR_MISSING_FILENAME, "es", "Falta el nombre del archivo");
 	Add(LocalizationKey::ERROR_MISSING_FILENAME, "zh", "缺少文件名");
-	
+
 	Add(LocalizationKey::ERROR_DEVICE_MISSING_FILENAME, "en", "Device type %1 requires a filename");
 	Add(LocalizationKey::ERROR_DEVICE_MISSING_FILENAME, "de", "Gerätetyp %1 erfordert einen Dateinamen");
 	Add(LocalizationKey::ERROR_DEVICE_MISSING_FILENAME, "sv", "Enhetstypen %1 kräver ett filnamn");
 	Add(LocalizationKey::ERROR_DEVICE_MISSING_FILENAME, "es", "El tipo de dispositivo %1 requiere un nombre de archivo");
 	Add(LocalizationKey::ERROR_DEVICE_MISSING_FILENAME, "zh", "设备类型 %1 需要一个文件名");
-	
-	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "en", "Image file '%1' is already being used by ID %2, unit %3");
-	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "de", "Image-Datei '%1' wird bereits von ID %2, Einheit %3 benutzt");
-	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "sv", "Skivbildsfilen '%1' används redan av id %2, enhetsnummer %3");
-	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "fr", "Le fichier d'image '%1' est déjà utilisé par l'ID %2, unité %3");
-	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "es", "El archivo de imagen '%1' ya está siendo utilizado por el ID %2, unidad %3");
-	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "zh", "图像文件%1已被 ID %2、单元 %3 使用");
-	
+
+	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "en", "Image file '%1' is already being used by device %2");
+	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "de", "Image-Datei '%1' wird bereits von Gerät %2 benutzt");
+	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "sv", "Skivbildsfilen '%1' används redan av nhet %2");
+	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "fr", "Le fichier d'image '%1' est déjà utilisé par périphérique %2");
+	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "es", "El archivo de imagen '%1' ya está siendo utilizado por dispositivo %2");
+	Add(LocalizationKey::ERROR_IMAGE_IN_USE, "zh", "图像文件%1已被 ID %2");
+
 	Add(LocalizationKey::ERROR_IMAGE_FILE_INFO, "en", "Can't create image file info for '%1'");
 	Add(LocalizationKey::ERROR_IMAGE_FILE_INFO, "de", "Image-Datei-Information für '%1' kann nicht erzeugt werden");
 	Add(LocalizationKey::ERROR_IMAGE_FILE_INFO, "sv", "Kunde ej skapa skivbildsfilsinfo för '%1'");
 	Add(LocalizationKey::ERROR_IMAGE_FILE_INFO, "es", "No se puede crear información de archivo de imagen para '%1'");
 	Add(LocalizationKey::ERROR_IMAGE_FILE_INFO, "zh", "无法为'%1'创建图像文件信息");
-	
+
 	Add(LocalizationKey::ERROR_RESERVED_ID, "en", "Device ID %1 is reserved");
 	Add(LocalizationKey::ERROR_RESERVED_ID, "de", "Geräte-ID %1 ist reserviert");
 	Add(LocalizationKey::ERROR_RESERVED_ID, "sv", "Enhets-id %1 är reserverat");
@@ -84,21 +82,21 @@ Localizer::Localizer()
 	Add(LocalizationKey::ERROR_NON_EXISTING_DEVICE, "fr", "Commande pour ID %1 non-existant");
 	Add(LocalizationKey::ERROR_NON_EXISTING_DEVICE, "es", "Comando para ID %1 no existente");
 	Add(LocalizationKey::ERROR_NON_EXISTING_DEVICE, "zh", "不存在的 ID %1 的指令");
-	
+
 	Add(LocalizationKey::ERROR_NON_EXISTING_UNIT, "en", "Command for non-existing ID %1, unit %2");
 	Add(LocalizationKey::ERROR_NON_EXISTING_UNIT, "de", "Kommando für nicht existente ID %1, Einheit %2");
 	Add(LocalizationKey::ERROR_NON_EXISTING_UNIT, "sv", "Kommando för id %1, enhetsnummer %2 som ej existerar");
 	Add(LocalizationKey::ERROR_NON_EXISTING_UNIT, "fr", "Command pour ID %1, unité %2 non-existant");
 	Add(LocalizationKey::ERROR_NON_EXISTING_UNIT, "es", "Comando para ID %1 inexistente, unidad %2");
 	Add(LocalizationKey::ERROR_NON_EXISTING_UNIT, "zh", "不存在的 ID %1, 单元 %2 的指令");
-	
+
 	Add(LocalizationKey::ERROR_UNKNOWN_DEVICE_TYPE, "en", "Unknown device type %1");
 	Add(LocalizationKey::ERROR_UNKNOWN_DEVICE_TYPE, "de", "Unbekannter Gerätetyp %1");
 	Add(LocalizationKey::ERROR_UNKNOWN_DEVICE_TYPE, "sv", "Obekant enhetstyp: %1");
 	Add(LocalizationKey::ERROR_UNKNOWN_DEVICE_TYPE, "fr", "Type de périphérique inconnu %1");
 	Add(LocalizationKey::ERROR_UNKNOWN_DEVICE_TYPE, "es", "Tipo de dispositivo desconocido %1");
 	Add(LocalizationKey::ERROR_UNKNOWN_DEVICE_TYPE, "zh", "未知设备类型 %1");
-	
+
 	Add(LocalizationKey::ERROR_MISSING_DEVICE_TYPE, "en", "Device type required for unknown extension of file '%1'");
 	Add(LocalizationKey::ERROR_MISSING_DEVICE_TYPE, "de", "Gerätetyp erforderlich für unbekannte Extension der Datei '%1'");
 	Add(LocalizationKey::ERROR_MISSING_DEVICE_TYPE, "sv", "Man måste ange enhetstyp för obekant filändelse '%1'");
@@ -199,11 +197,11 @@ Localizer::Localizer()
 	Add(LocalizationKey::ERROR_LUN0, "es", "El LUN 0 no se puede desconectar mientras haya otro LUN");
 	Add(LocalizationKey::ERROR_LUN0, "zh", "LUN 0 无法卸载，因为当前仍有另一个 LUN。");
 
-	Add(LocalizationKey::ERROR_INITIALIZATION, "en", "Initialization of %1 device, ID %2, LUN %3 failed");
-	Add(LocalizationKey::ERROR_INITIALIZATION, "de", "Initialisierung von %1-Gerät, ID %2, LUN %3 fehlgeschlagen");
-	Add(LocalizationKey::ERROR_INITIALIZATION, "sv", "Kunde ej initialisera enheten %1 med id %2 och enhetsnummer %3");
-	Add(LocalizationKey::ERROR_INITIALIZATION, "es", "La inicialización del dispositivo %1, ID %2, LUN %3 falló");
-	Add(LocalizationKey::ERROR_INITIALIZATION, "zh", "%1 设备、ID %2、LUN %3 的初始化失败");
+	Add(LocalizationKey::ERROR_INITIALIZATION, "en", "Initialization of %1 failed");
+	Add(LocalizationKey::ERROR_INITIALIZATION, "de", "Initialisierung von %1 fehlgeschlagen");
+	Add(LocalizationKey::ERROR_INITIALIZATION, "sv", "Kunde ej initialisera %1 ");
+	Add(LocalizationKey::ERROR_INITIALIZATION, "es", "La inicialización del %1 falló");
+	Add(LocalizationKey::ERROR_INITIALIZATION, "zh", "%1 的初始化失败");
 
 	Add(LocalizationKey::ERROR_OPERATION_DENIED_STOPPABLE, "en", "%1 operation denied, %2 isn't stoppable");
 	Add(LocalizationKey::ERROR_OPERATION_DENIED_STOPPABLE, "de", "%1-Operation verweigert, %2 ist nicht stopbar");
@@ -235,17 +233,16 @@ void Localizer::Add(LocalizationKey key, const string& locale, string_view value
 	// Safeguards against empty messages, duplicate entries and unsupported locales
 	assert(locale.size());
 	assert(value.size());
-	assert(supported_languages.find(locale) != supported_languages.end());
+	assert(supported_languages.contains(locale));
 	assert(localized_messages[locale][key].empty());
-
 	localized_messages[locale][key] = value;
 }
 
 string Localizer::Localize(LocalizationKey key, const string& locale, const string& arg1, const string& arg2,
 		const string &arg3) const
 {
-	string locale_lower = locale;
-	transform(locale_lower.begin(), locale_lower.end(), locale_lower.begin(), ::tolower);
+	string locale_lower;
+	ranges::transform(locale, back_inserter(locale_lower), ::tolower);
 
 	auto it = localized_messages.find(locale_lower);
 	if (it == localized_messages.end()) {
@@ -268,9 +265,9 @@ string Localizer::Localize(LocalizationKey key, const string& locale, const stri
 	}
 
 	string message = m->second;
-	message = regex_replace(message, regex("%1"), arg1);
-	message = regex_replace(message, regex("%2"), arg2);
-	message = regex_replace(message, regex("%3"), arg3);
+	message = regex_replace(message, regex1, arg1);
+	message = regex_replace(message, regex2, arg2);
+	message = regex_replace(message, regex3, arg3);
 
 	return message;
 }

cpp/piscsi/localizer.h

@@ -3,7 +3,7 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 // Message localization support. Currently only for messages with up to 3 string parameters.
 //
@@ -11,9 +11,11 @@
 
 #pragma once
 
+#include "shared/piscsi_util.h"
 #include <string>
 #include <unordered_set>
 #include <unordered_map>
+#include <regex>
 
 using namespace std;
 
@@ -64,8 +66,12 @@ public:
 private:
 
 	void Add(LocalizationKey, const string&, string_view);
-	unordered_map<string, unordered_map<LocalizationKey, string>> localized_messages;
+	unordered_map<string, unordered_map<LocalizationKey, string>, piscsi_util::StringHash, equal_to<>> localized_messages;
 
 	// Supported locales, always lower case
-	unordered_set<string> supported_languages = { "en", "de", "sv", "fr", "es", "zh" };
+	unordered_set<string, piscsi_util::StringHash, equal_to<>> supported_languages = { "en", "de", "sv", "fr", "es", "zh" };
+
+	const regex regex1 = regex("%1");
+	const regex regex2 = regex("%2");
+	const regex regex3 = regex("%3");
 };

cpp/piscsi/piscsi_core.cpp

@@ -1,57 +1,52 @@
 //---------------------------------------------------------------------------
 //
-//	SCSI Target Emulator PiSCSI
-//	for Raspberry Pi
+// SCSI Target Emulator PiSCSI
+// for Raspberry Pi
 //
-//	Powered by XM6 TypeG Technology.
-//	Copyright (C) 2016-2020 GIMONS
-//	Copyright (C) 2020-2023 Contributors to the PiSCSI project
+// Powered by XM6 TypeG Technology.
+// Copyright (C) 2016-2020 GIMONS
+// Copyright (C) 2020-2023 Contributors to the PiSCSI project
+// Copyright (C) 2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
 #include "shared/config.h"
-#include "shared/log.h"
 #include "shared/piscsi_util.h"
-#include "shared/protobuf_serializer.h"
 #include "shared/protobuf_util.h"
 #include "shared/piscsi_exceptions.h"
 #include "shared/piscsi_version.h"
-#include "controllers/controller_manager.h"
 #include "controllers/scsi_controller.h"
+#include "devices/device_logger.h"
 #include "devices/device_factory.h"
 #include "devices/storage_device.h"
 #include "hal/gpiobus_factory.h"
 #include "hal/gpiobus.h"
 #include "hal/systimer.h"
-#include "piscsi/piscsi_executor.h"
 #include "piscsi/piscsi_core.h"
-#include "spdlog/sinks/stdout_color_sinks.h"
+#include <spdlog/spdlog.h>
 #include <netinet/in.h>
 #include <csignal>
-#include <string>
 #include <sstream>
 #include <iostream>
 #include <fstream>
-#include <list>
-#include <filesystem>
+#include <vector>
 
 using namespace std;
 using namespace filesystem;
-using namespace spdlog;
 using namespace piscsi_interface;
 using namespace piscsi_util;
 using namespace protobuf_util;
 using namespace scsi_defs;
 
-void Piscsi::Banner(const vector<char *>& args) const
+void Piscsi::Banner(span<char *> args) const
 {
 	cout << piscsi_util::Banner("(Backend Service)");
 	cout << "Connection type: " << CONNECT_DESC << '\n' << flush;
 
 	if ((args.size() > 1 && strcmp(args[1], "-h") == 0) || (args.size() > 1 && strcmp(args[1], "--help") == 0)){
 		cout << "\nUsage: " << args[0] << " [-idID[:LUN] FILE] ...\n\n";
-		cout << " ID is SCSI device ID (0-7).\n";
-		cout << " LUN is the optional logical unit (0-31).\n";
+		cout << " ID is SCSI device ID (0-" << (ControllerManager::GetScsiIdMax() - 1) << ").\n";
+		cout << " LUN is the optional logical unit (0-" << (ControllerManager::GetScsiLunMax() - 1) <<").\n";
 		cout << " FILE is a disk image file, \"daynaport\", \"bridge\", \"printer\" or \"services\".\n\n";
 		cout << " Image type is detected based on file extension if no explicit type is specified.\n";
 		cout << "  hd1 : SCSI-1 HD image (Non-removable generic SCSI-1 HD image)\n";
@@ -69,59 +64,57 @@ void Piscsi::Banner(const vector<char *>& args) const
 	}
 }
 
-bool Piscsi::InitBus() const
+bool Piscsi::InitBus()
 {
 	bus = GPIOBUS_Factory::Create(BUS::mode_e::TARGET);
 	if (bus == nullptr) {
 		return false;
 	}
 
-	auto b = bus;
-	controller_manager = make_shared<ControllerManager>(*b);
-	auto c = controller_manager;
-	executor = make_shared<PiscsiExecutor>(piscsi_image, *c);
+	executor = make_unique<PiscsiExecutor>(piscsi_image, *bus, controller_manager);
 
 	return true;
 }
 
-void Piscsi::Cleanup()
+void Piscsi::CleanUp()
 {
-	executor->DetachAll();
+	if (service.IsRunning()) {
+		service.Stop();
+	}
 
-	service.Cleanup();
+	executor->DetachAll();
 
 	bus->Cleanup();
 }
 
-void Piscsi::ReadAccessToken(const string& filename) const
+void Piscsi::ReadAccessToken(const path& filename)
 {
-	struct stat st;
-	if (stat(filename.c_str(), &st) || !S_ISREG(st.st_mode)) {
-		throw parser_exception("Can't access token file '" + filename + "'");
+	if (error_code error; !is_regular_file(filename, error)) {
+		throw parser_exception("Access token file '" + filename.string() + "' must be a regular file");
 	}
 
-	if (st.st_uid || st.st_gid) {
-		throw parser_exception("Access token file '" + filename + "' must be owned by root");
+	if (struct stat st; stat(filename.c_str(), &st) || st.st_uid || st.st_gid) {
+		throw parser_exception("Access token file '" + filename.string() + "' must be owned by root");
 	}
 
 	if (const auto perms = filesystem::status(filename).permissions();
 		(perms & perms::group_read) != perms::none || (perms & perms::others_read) != perms::none ||
 			(perms & perms::group_write) != perms::none || (perms & perms::others_write) != perms::none) {
-		throw parser_exception("Access token file '" + filename + "' must be readable by root only");
+		throw parser_exception("Access token file '" + filename.string() + "' must be readable by root only");
 	}
 
 	ifstream token_file(filename);
 	if (token_file.fail()) {
-		throw parser_exception("Can't open access token file '" + filename + "'");
+		throw parser_exception("Can't open access token file '" + filename.string() + "'");
 	}
 
 	getline(token_file, access_token);
 	if (token_file.fail()) {
-		throw parser_exception("Can't read access token file '" + filename + "'");
+		throw parser_exception("Can't read access token file '" + filename.string() + "'");
 	}
 
 	if (access_token.empty()) {
-		throw parser_exception("Access token file '" + filename + "' must not be empty");
+		throw parser_exception("Access token file '" + filename.string() + "' must not be empty");
 	}
 }
 
@@ -131,59 +124,74 @@ void Piscsi::LogDevices(string_view devices) const
 	string line;
 
 	while (getline(ss, line, '\n')) {
-		LOGINFO("%s", line.c_str())
+		spdlog::info(line);
 	}
 }
 
-PbDeviceType Piscsi::ParseDeviceType(const string& value) const
-{
-	string t = value;
-	PbDeviceType type;
-	transform(t.begin(), t.end(), t.begin(), ::toupper);
-	if (!PbDeviceType_Parse(t, &type)) {
-		throw parser_exception("Illegal device type '" + value + "'");
-	}
-
-	return type;
-}
-
 void Piscsi::TerminationHandler(int)
 {
-	Cleanup();
+	instance->CleanUp();
 
 	// Process will terminate automatically
 }
 
-Piscsi::optargs_type Piscsi::ParseArguments(const vector<char *>& args, int& port) const
+string Piscsi::ParseArguments(span<char *> args, PbCommand& command, int& port, string& reserved_ids)
 {
-	optargs_type optargs;
+	string log_level = "info";
+	PbDeviceType type = UNDEFINED;
+	int block_size = 0;
 	string name;
+	string id_and_lun;
+
+	string locale = GetLocale();
+
+	// Avoid duplicate messages while parsing
+	set_level(level::off);
 
 	opterr = 1;
 	int opt;
 	while ((opt = getopt(static_cast<int>(args.size()), args.data(), "-Iib:d:n:p:r:t:z:D:F:L:P:R:C:v")) != -1) {
 		switch (opt) {
-			// The following options can not be processed until AFTER
-			// the 'bus' object is created and configured
+			// The two options below are kind of a compound option with two letters
 			case 'i':
 			case 'I':
-			case 'b':
+				continue;
+
 			case 'd':
 			case 'D':
-			case 'R':
-			case 'n':
-			case 'r':
-			case 't':
-			case 'F':
-			case 'z':
-			{
-				const string optarg_str = optarg == nullptr ? "" : optarg;
-				optargs.emplace_back(opt, optarg_str);
+				id_and_lun = optarg;
+				continue;
+
+			case 'b':
+				if (!GetAsUnsignedInt(optarg, block_size)) {
+					throw parser_exception("Invalid block size " + string(optarg));
+				}
+				continue;
+
+			case 'z':
+				locale = optarg;
+				continue;
+
+			case 'F':
+				if (const string error = piscsi_image.SetDefaultFolder(optarg); !error.empty()) {
+					throw parser_exception(error);
+				}
 				continue;
-			}
 
 			case 'L':
-				current_log_level = optarg;
+				log_level = optarg;
+				continue;
+
+			case 'R':
+				int depth;
+				if (!GetAsUnsignedInt(optarg, depth)) {
+					throw parser_exception("Invalid image file scan depth " + string(optarg));
+				}
+				piscsi_image.SetDepth(depth);
+				continue;
+
+			case 'n':
+				name = optarg;
 				continue;
 
 			case 'p':
@@ -196,92 +204,12 @@ Piscsi::optargs_type Piscsi::ParseArguments(const vector<char *>& args, int& por
 				ReadAccessToken(optarg);
 				continue;
 
-			case 'v':
-				cout << piscsi_get_version_string() << endl;
-				exit(0);
-
-			case 1:
-			{
-				// Encountered filename
-				const string optarg_str = (optarg == nullptr) ? "" : string(optarg);
-				optargs.emplace_back(opt, optarg_str);
-				continue;
-			}
-
-			default:
-				throw parser_exception("Parser error");
-		}
-
-		if (optopt) {
-			throw parser_exception("Parser error");
-		}
-	}
-
-	return optargs;
-}
-
-void Piscsi::CreateInitialDevices(const optargs_type& optargs) const
-{
-	PbCommand command;
-	PbDeviceType type = UNDEFINED;
-	int block_size = 0;
-	string name;
-	string log_level;
-	string id_and_lun;
-
-	const char *locale = setlocale(LC_MESSAGES, "");
-	if (locale == nullptr || !strcmp(locale, "C")) {
-		locale = "en";
-	}
-
-	opterr = 1;
-	for (const auto& [option, value] : optargs) {
-		switch (option) {
-			case 'i':
-			case 'I':
-				continue;
-
-			case 'd':
-			case 'D':
-				id_and_lun = value;
-				continue;
-
-			case 'b':
-				if (!GetAsUnsignedInt(value, block_size)) {
-					throw parser_exception("Invalid block size " + value);
-				}
-				continue;
-
-			case 'z':
-				locale = value.c_str();
-				continue;
-
-			case 'F':
-				if (const string error = piscsi_image.SetDefaultFolder(value); !error.empty()) {
-					throw parser_exception(error);
-				}
-				continue;
-
-			case 'R':
-				int depth;
-				if (!GetAsUnsignedInt(value, depth)) {
-					throw parser_exception("Invalid image file scan depth " + value);
-				}
-				piscsi_image.SetDepth(depth);
-				continue;
-
-			case 'n':
-				name = value;
-				continue;
-
 			case 'r':
-				if (const string error = executor->SetReservedIds(value); !error.empty()) {
-					throw parser_exception(error);
-				}
+				reserved_ids = optarg;
 				continue;
 
 			case 't':
-				type = ParseDeviceType(value);
+				type = ParseDeviceType(optarg);
 				continue;
 
 			case 1:
@@ -292,10 +220,16 @@ void Piscsi::CreateInitialDevices(const optargs_type& optargs) const
 				throw parser_exception("Parser error");
 		}
 
-		PbDeviceDefinition *device = command.add_devices();
+		if (optopt) {
+			throw parser_exception("Parser error");
+		}
+
+		// Set up the device data
+
+		auto device = command.add_devices();
 
 		if (!id_and_lun.empty()) {
-			if (const string error = SetIdAndLun(*device, id_and_lun, ScsiController::LUN_MAX); !error.empty()) {
+			if (const string error = SetIdAndLun(*device, id_and_lun); !error.empty()) {
 				throw parser_exception(error);
 			}
 		}
@@ -303,7 +237,7 @@ void Piscsi::CreateInitialDevices(const optargs_type& optargs) const
 		device->set_type(type);
 		device->set_block_size(block_size);
 
-		ParseParameters(*device, value);
+		ParseParameters(*device, optarg);
 
 		SetProductData(*device, name);
 
@@ -313,182 +247,231 @@ void Piscsi::CreateInitialDevices(const optargs_type& optargs) const
 		id_and_lun = "";
 	}
 
-	// Attach all specified devices
-	command.set_operation(ATTACH);
-
-	if (CommandContext context(locale); !executor->ProcessCmd(context, command)) {
-		throw parser_exception("Can't execute " + PbOperation_Name(command.operation()));
+	if (!SetLogLevel(log_level)) {
+		throw parser_exception("Invalid log level '" + log_level + "'");
 	}
 
-	// Display and log the device list
-	PbServerInfo server_info;
-	piscsi_response.GetDevices(controller_manager->GetAllDevices(), server_info, piscsi_image.GetDefaultFolder());
-	const list<PbDevice>& devices = { server_info.devices_info().devices().begin(), server_info.devices_info().devices().end() };
-	const string device_list = ListDevices(devices);
-	LogDevices(device_list);
-	cout << device_list << flush;
+	return locale;
 }
 
-bool Piscsi::ExecuteCommand(const CommandContext& context, const PbCommand& command)
+PbDeviceType Piscsi::ParseDeviceType(const string& value)
 {
+	string t;
+	ranges::transform(value, back_inserter(t), ::toupper);
+	if (PbDeviceType type; PbDeviceType_Parse(t, &type)) {
+		return type;
+	}
+
+	throw parser_exception("Illegal device type '" + value + "'");
+}
+
+bool Piscsi::SetLogLevel(const string& log_level) const
+{
+	int id = -1;
+	int lun = -1;
+	string level = log_level;
+
+	if (const auto& components = Split(log_level, COMPONENT_SEPARATOR, 2); !components.empty()) {
+		level = components[0];
+
+		if (components.size() > 1) {
+			if (const string error = ProcessId(components[1], id, lun); !error.empty()) {
+				spdlog::warn("Error setting log level: " + error);
+				return false;
+			}
+		}
+	}
+
+	const level::level_enum l = level::from_str(level);
+	// Compensate for spdlog using 'off' for unknown levels
+	if (to_string_view(l) != level) {
+		spdlog::warn("Invalid log level '" + level + "'");
+		return false;
+	}
+
+	set_level(l);
+	DeviceLogger::SetLogIdAndLun(id, lun);
+
+	if (id != -1) {
+		if (lun == -1) {
+			spdlog::info("Set log level for device " + to_string(id) + " to '" + level + "'");
+		}
+		else {
+			spdlog::info("Set log level for device " + to_string(id) + ":" + to_string(lun) + " to '" + level + "'");
+		}
+	}
+	else {
+		spdlog::info("Set log level to '" + level + "'");
+	}
+
+	return true;
+}
+
+bool Piscsi::ExecuteCommand(CommandContext& context)
+{
+	context.SetDefaultFolder(piscsi_image.GetDefaultFolder());
+
+	const PbCommand& command = context.GetCommand();
+
 	if (!access_token.empty() && access_token != GetParam(command, "token")) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_AUTHENTICATION, UNAUTHORIZED);
 	}
 
 	if (!PbOperation_IsValid(command.operation())) {
-		LOGERROR("Received unknown command with operation opcode %d", command.operation())
+		spdlog::error("Received unknown command with operation opcode " + to_string(command.operation()));
 
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION, UNKNOWN_OPERATION);
 	}
 
-	LOGTRACE("Received %s command", PbOperation_Name(command.operation()).c_str())
+	spdlog::trace("Received " + PbOperation_Name(command.operation()) + " command");
 
 	PbResult result;
-	ProtobufSerializer serializer;
 
 	switch(command.operation()) {
-		case LOG_LEVEL: {
-			const string log_level = GetParam(command, "level");
-			if (const bool status = executor->SetLogLevel(log_level); !status) {
+		case LOG_LEVEL:
+			if (const string log_level = GetParam(command, "level"); !SetLogLevel(log_level)) {
 				context.ReturnLocalizedError(LocalizationKey::ERROR_LOG_LEVEL, log_level);
 			}
 			else {
-				current_log_level = log_level;
-
-				context.ReturnStatus();
+				context.ReturnSuccessStatus();
 			}
 			break;
-		}
 
-		case DEFAULT_FOLDER: {
-			if (const string status = piscsi_image.SetDefaultFolder(GetParam(command, "folder")); !status.empty()) {
-				context.ReturnStatus(false, status);
+		case DEFAULT_FOLDER:
+			if (const string error = piscsi_image.SetDefaultFolder(GetParam(command, "folder")); !error.empty()) {
+				context.ReturnErrorStatus(error);
 			}
 			else {
-				context.ReturnStatus();
+				context.ReturnSuccessStatus();
 			}
 			break;
-		}
 
-		case DEVICES_INFO: {
-			piscsi_response.GetDevicesInfo(controller_manager->GetAllDevices(), result, command,
-					piscsi_image.GetDefaultFolder());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case DEVICES_INFO:
+			response.GetDevicesInfo(controller_manager.GetAllDevices(), result, command, piscsi_image.GetDefaultFolder());
+			context.WriteResult(result);
 			break;
-		}
 
-		case DEVICE_TYPES_INFO: {
-			result.set_allocated_device_types_info(piscsi_response.GetDeviceTypesInfo(result).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case DEVICE_TYPES_INFO:
+			response.GetDeviceTypesInfo(*result.mutable_device_types_info());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case SERVER_INFO: {
-			result.set_allocated_server_info(piscsi_response.GetServerInfo(controller_manager->GetAllDevices(),
-					result, executor->GetReservedIds(), current_log_level, piscsi_image.GetDefaultFolder(),
-					GetParam(command, "folder_pattern"), GetParam(command, "file_pattern"),
-					piscsi_image.GetDepth()).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case SERVER_INFO:
+			response.GetServerInfo(*result.mutable_server_info(), controller_manager.GetAllDevices(),
+					executor->GetReservedIds(), piscsi_image.GetDefaultFolder(),
+					GetParam(command, "folder_pattern"), GetParam(command, "file_pattern"), piscsi_image.GetDepth());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case VERSION_INFO: {
-			result.set_allocated_version_info(piscsi_response.GetVersionInfo(result).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case VERSION_INFO:
+			response.GetVersionInfo(*result.mutable_version_info());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case LOG_LEVEL_INFO: {
-			result.set_allocated_log_level_info(piscsi_response.GetLogLevelInfo(result, current_log_level).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case LOG_LEVEL_INFO:
+			response.GetLogLevelInfo(*result.mutable_log_level_info());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case DEFAULT_IMAGE_FILES_INFO: {
-			result.set_allocated_image_files_info(piscsi_response.GetAvailableImages(result,
-					piscsi_image.GetDefaultFolder(), GetParam(command, "folder_pattern"),
-					GetParam(command, "file_pattern"), piscsi_image.GetDepth()).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case DEFAULT_IMAGE_FILES_INFO:
+			response.GetImageFilesInfo(*result.mutable_image_files_info(), piscsi_image.GetDefaultFolder(),
+					GetParam(command, "folder_pattern"), GetParam(command, "file_pattern"), piscsi_image.GetDepth());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case IMAGE_FILE_INFO: {
+		case IMAGE_FILE_INFO:
 			if (string filename = GetParam(command, "file"); filename.empty()) {
 				context.ReturnLocalizedError( LocalizationKey::ERROR_MISSING_FILENAME);
 			}
 			else {
 				auto image_file = make_unique<PbImageFile>();
-				const bool status = piscsi_response.GetImageFile(*image_file.get(), piscsi_image.GetDefaultFolder(), filename);
+				const bool status = response.GetImageFile(*image_file.get(), piscsi_image.GetDefaultFolder(),
+						filename);
 				if (status) {
-					result.set_status(true);
 					result.set_allocated_image_file_info(image_file.get());
-					serializer.SerializeMessage(context.GetFd(), result);
+					result.set_status(true);
+					context.WriteResult(result);
 				}
 				else {
 					context.ReturnLocalizedError(LocalizationKey::ERROR_IMAGE_FILE_INFO);
 				}
 			}
 			break;
-		}
 
-		case NETWORK_INTERFACES_INFO: {
-			result.set_allocated_network_interfaces_info(piscsi_response.GetNetworkInterfacesInfo(result).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case NETWORK_INTERFACES_INFO:
+			response.GetNetworkInterfacesInfo(*result.mutable_network_interfaces_info());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case MAPPING_INFO: {
-			result.set_allocated_mapping_info(piscsi_response.GetMappingInfo(result).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case MAPPING_INFO:
+			response.GetMappingInfo(*result.mutable_mapping_info());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case OPERATION_INFO: {
-			result.set_allocated_operation_info(piscsi_response.GetOperationInfo(result,
-					piscsi_image.GetDepth()).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case OPERATION_INFO:
+			response.GetOperationInfo(*result.mutable_operation_info(), piscsi_image.GetDepth());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case RESERVED_IDS_INFO: {
-			result.set_allocated_reserved_ids_info(piscsi_response.GetReservedIds(result,
-					executor->GetReservedIds()).release());
-			serializer.SerializeMessage(context.GetFd(), result);
+		case RESERVED_IDS_INFO:
+			response.GetReservedIds(*result.mutable_reserved_ids_info(), executor->GetReservedIds());
+			context.WriteSuccessResult(result);
 			break;
-		}
 
-		case SHUT_DOWN: {
+		case SHUT_DOWN:
 			if (executor->ShutDown(context, GetParam(command, "mode"))) {
 				TerminationHandler(0);
 			}
 			break;
-		}
 
-		default: {
-			// Wait until we become idle
+		case NO_OPERATION:
+			context.ReturnSuccessStatus();
+			break;
+
+		// TODO The image operations below can most likely directly be executed without calling the executor,
+		// because they do not require the target to be idle
+		case CREATE_IMAGE:
+		case DELETE_IMAGE:
+		case RENAME_IMAGE:
+		case COPY_IMAGE:
+		case PROTECT_IMAGE:
+		case UNPROTECT_IMAGE:
+		case RESERVE_IDS:
+			return executor->ProcessCmd(context);
+
+		// The remaining commands can only be executed when the target is idle
+		// TODO What happens when the target becomes active while the command is still being executed?
+		// A field 'mutex locker' can probably avoid SCSI commands and ProcessCmd() being executed at the same time
+		default:
+			// TODO Find a better way to wait
 			const timespec ts = { .tv_sec = 0, .tv_nsec = 500'000'000};
-			while (active) {
+			while (target_is_active) {
 				nanosleep(&ts, nullptr);
 			}
-
-			executor->ProcessCmd(context, command);
-			break;
-		}
+			return executor->ProcessCmd(context);
 	}
 
 	return true;
 }
 
-int Piscsi::run(const vector<char *>& args)
+int Piscsi::run(span<char *> args)
 {
 	GOOGLE_PROTOBUF_VERIFY_VERSION;
 
 	Banner(args);
 
+	// The -v option shall result in no other action except displaying the version
+	if (ranges::find_if(args, [] (const char *arg) { return !strcasecmp(arg, "-v"); } ) != args.end()) {
+		cout << piscsi_get_version_string() << '\n';
+		return EXIT_SUCCESS;
+	}
+
+	PbCommand command;
+	string locale;
+	string reserved_ids;
 	int port = DEFAULT_PORT;
-	optargs_type optargs;
 	try {
-		optargs = ParseArguments(args, port);
+		locale = ParseArguments(args, command, port, reserved_ids);
 	}
 	catch(const parser_exception& e) {
 		cerr << "Error: " << e.what() << endl;
@@ -496,35 +479,51 @@ int Piscsi::run(const vector<char *>& args)
 		return EXIT_FAILURE;
 	}
 
-	// current_log_level may have been updated by ParseArguments()
-	executor->SetLogLevel(current_log_level);
-
-	// Create a thread-safe stdout logger to process the log messages
-	const auto logger = stdout_color_mt("piscsi stdout logger");
-
 	if (!InitBus()) {
 		cerr << "Error: Can't initialize bus" << endl;
 
 		return EXIT_FAILURE;
 	}
 
-	// We need to wait to create the devices until after the bus/controller/etc objects have been created
-	// TODO Try to resolve dependencies so that this work-around can be removed
-	try {
-		CreateInitialDevices(optargs);
-	}
-	catch(const parser_exception& e) {
-		cerr << "Error: " << e.what() << endl;
+	if (const string error = service.Init([this] (CommandContext& context) { return ExecuteCommand(context); }, port);
+		!error.empty()) {
+		cerr << "Error: " << error << endl;
 
-		Cleanup();
+		CleanUp();
 
 		return EXIT_FAILURE;
 	}
 
-	if (!service.Init(&ExecuteCommand, port)) {
+	if (const string error = executor->SetReservedIds(reserved_ids); !error.empty()) {
+		cerr << "Error: " << error << endl;
+
+		CleanUp();
+
 		return EXIT_FAILURE;
 	}
 
+	if (command.devices_size()) {
+		// Attach all specified devices
+		command.set_operation(ATTACH);
+
+		if (const CommandContext context(command, piscsi_image.GetDefaultFolder(), locale); !executor->ProcessCmd(context)) {
+			cerr << "Error: Can't attach devices" << endl;
+
+			CleanUp();
+
+			return EXIT_FAILURE;
+		}
+	}
+
+	// Display and log the device list
+	PbServerInfo server_info;
+	response.GetDevices(controller_manager.GetAllDevices(), server_info, piscsi_image.GetDefaultFolder());
+	const vector<PbDevice>& devices = { server_info.devices_info().devices().begin(), server_info.devices_info().devices().end() };
+	const string device_list = ListDevices(devices);
+	LogDevices(device_list);
+	cout << device_list << flush;
+
+	instance = this;
 	// Signal handler to detach all devices on a KILL or TERM signal
 	struct sigaction termination_handler;
 	termination_handler.sa_handler = TerminationHandler;
@@ -532,22 +531,30 @@ int Piscsi::run(const vector<char *>& args)
 	termination_handler.sa_flags = 0;
 	sigaction(SIGINT, &termination_handler, nullptr);
 	sigaction(SIGTERM, &termination_handler, nullptr);
+	signal(SIGPIPE, SIG_IGN);
 
     // Set the affinity to a specific processor core
 	FixCpu(3);
 
-	sched_param schparam;
+	service.Start();
+
+	Process();
+
+	return EXIT_SUCCESS;
+}
+
+void Piscsi::Process()
+{
 #ifdef USE_SEL_EVENT_ENABLE
 	// Scheduling policy setting (highest priority)
+	// TODO Check whether this results in any performance gain
+	sched_param schparam;
 	schparam.sched_priority = sched_get_priority_max(SCHED_FIFO);
 	sched_setscheduler(0, SCHED_FIFO, &schparam);
 #else
 	cout << "Note: No PiSCSI hardware support, only client interface calls are supported" << endl;
 #endif
 
-	// Start execution
-	service.SetRunning(true);
-
 	// Main Loop
 	while (service.IsRunning()) {
 #ifdef USE_SEL_EVENT_ENABLE
@@ -571,81 +578,55 @@ int Piscsi::run(const vector<char *>& args)
 		}
 #endif
 
-        // Wait until BSY is released as there is a possibility for the
-        // initiator to assert it while setting the ID (for up to 3 seconds)
-		WaitForNotBusy();
+		// Only process the SCSI command if the bus is not busy and no other device responded
+		if (IsNotBusy() && bus->GetSEL()) {
+			target_is_active = true;
 
-		// Stop because the bus is busy or another device responded
-		if (bus->GetBSY() || !bus->GetSEL()) {
-			continue;
-		}
-
-		int initiator_id = AbstractController::UNKNOWN_INITIATOR_ID;
-
-		// The initiator and target ID
-		const uint8_t id_data = bus->GetDAT();
-
-		phase_t phase = phase_t::busfree;
-
-		// Identify the responsible controller
-		auto controller = controller_manager->IdentifyController(id_data);
-		if (controller != nullptr) {
-			device_logger.SetIdAndLun(controller->GetTargetId(), -1);
-
-			initiator_id = controller->ExtractInitiatorId(id_data);
-
-			if (initiator_id != AbstractController::UNKNOWN_INITIATOR_ID) {
-				device_logger.Trace("++++ Starting processing for initiator ID " + to_string(initiator_id));
-			}
-			else {
-				device_logger.Trace("++++ Starting processing for unknown initiator ID");
+			// Process command on the responsible controller based on the current initiator and target ID
+			if (const auto shutdown_mode = controller_manager.ProcessOnController(bus->GetDAT());
+				shutdown_mode != AbstractController::piscsi_shutdown_mode::NONE) {
+				// When the bus is free PiSCSI or the Pi may be shut down.
+				ShutDown(shutdown_mode);
 			}
 
-			if (controller->Process(initiator_id) == phase_t::selection) {
-				phase = phase_t::selection;
-			}
+			target_is_active = false;
 		}
-
-		// Return to bus monitoring if the selection phase has not started
-		if (phase != phase_t::selection) {
-			continue;
-		}
-
-		// Start target device
-		active = true;
-
-#if !defined(USE_SEL_EVENT_ENABLE) && defined(__linux__)
-		// Scheduling policy setting (highest priority)
-		schparam.sched_priority = sched_get_priority_max(SCHED_FIFO);
-		sched_setscheduler(0, SCHED_FIFO, &schparam);
-#endif
-
-		// Loop until the bus is free
-		while (service.IsRunning()) {
-			// Target drive
-			phase = controller->Process(initiator_id);
-
-			// End when the bus is free
-			if (phase == phase_t::busfree) {
-				break;
-			}
-		}
-
-#if !defined(USE_SEL_EVENT_ENABLE) && defined(__linux__)
-		// Set the scheduling priority back to normal
-		schparam.sched_priority = 0;
-		sched_setscheduler(0, SCHED_OTHER, &schparam);
-#endif
-
-		// End the target travel
-		active = false;
 	}
-
-	return EXIT_SUCCESS;
 }
 
-void Piscsi::WaitForNotBusy() const
+void Piscsi::ShutDown(AbstractController::piscsi_shutdown_mode shutdown_mode)
 {
+	CleanUp();
+
+	switch(shutdown_mode) {
+	case AbstractController::piscsi_shutdown_mode::STOP_PISCSI:
+		spdlog::info("PiSCSI shutdown requested");
+		break;
+
+	case AbstractController::piscsi_shutdown_mode::STOP_PI:
+		spdlog::info("Raspberry Pi shutdown requested");
+		if (system("init 0") == -1) {
+			spdlog::error("Raspberry Pi shutdown failed");
+		}
+		break;
+
+	case AbstractController::piscsi_shutdown_mode::RESTART_PI:
+		spdlog::info("Raspberry Pi restart requested");
+		if (system("init 6") == -1) {
+			spdlog::error("Raspberry Pi restart failed");
+		}
+		break;
+
+	case AbstractController::piscsi_shutdown_mode::NONE:
+		assert(false);
+		break;
+	}
+}
+
+bool Piscsi::IsNotBusy() const
+{
+    // Wait until BSY is released as there is a possibility for the
+	// initiator to assert it while setting the ID (for up to 3 seconds)
 	if (bus->GetBSY()) {
 		const uint32_t now = SysTimer::GetTimerLow();
 
@@ -654,8 +635,12 @@ void Piscsi::WaitForNotBusy() const
 			bus->Acquire();
 
 			if (!bus->GetBSY()) {
-				break;
+				return true;
 			}
 		}
+
+		return false;
 	}
+
+	return true;
 }

cpp/piscsi/piscsi_core.h

@@ -3,31 +3,31 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2022 Uwe Seimet
+// Copyright (C) 2022-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
 #pragma once
 
-#include "devices/device_logger.h"
+#include "controllers/controller_manager.h"
+#include "controllers/abstract_controller.h"
 #include "piscsi/command_context.h"
 #include "piscsi/piscsi_service.h"
 #include "piscsi/piscsi_image.h"
 #include "piscsi/piscsi_response.h"
+#include "piscsi/piscsi_executor.h"
 #include "generated/piscsi_interface.pb.h"
-#include <vector>
+#include "spdlog/sinks/stdout_color_sinks.h"
+#include <span>
 #include <string>
+#include <atomic>
 
 using namespace std;
 
 class BUS;
-class ControllerManager;
-class PiscsiExecutor;
 
 class Piscsi
 {
-	using optargs_type = vector<pair<int, string>>;
-
 	static const int DEFAULT_PORT = 6868;
 
 public:
@@ -35,46 +35,47 @@ public:
 	Piscsi() = default;
 	~Piscsi() = default;
 
-	int run(const vector<char *>&);
+	int run(span<char *>);
 
 private:
 
-	void Banner(const vector<char *>&) const;
-	bool InitBus() const;
-	static void Cleanup();
-	void ReadAccessToken(const string&) const;
+	void Banner(span<char *>) const;
+	bool InitBus();
+	void CleanUp();
+	void ReadAccessToken(const path&);
 	void LogDevices(string_view) const;
-	PbDeviceType ParseDeviceType(const string&) const;
 	static void TerminationHandler(int);
-	optargs_type ParseArguments(const vector<char *>&, int&) const;
-	void CreateInitialDevices(const optargs_type&) const;
-	void WaitForNotBusy() const;
+	string ParseArguments(span<char *>, PbCommand&, int&, string&);
+	void Process();
+	bool IsNotBusy() const;
 
-	// TODO Should not be static and should be moved to PiscsiService
-	static bool ExecuteCommand(const CommandContext&, const PbCommand&);
+	void ShutDown(AbstractController::piscsi_shutdown_mode);
 
-	DeviceLogger device_logger;
+	bool ExecuteCommand(CommandContext&);
 
-	// A static instance is needed because of the signal handler
-	static inline shared_ptr<BUS> bus;
+	bool SetLogLevel(const string&) const;
 
-	// TODO These fields should not be static
+	const shared_ptr<spdlog::logger> logger = spdlog::stdout_color_mt("piscsi stdout logger");
 
-	static inline PiscsiService service;
-
-	static inline PiscsiImage piscsi_image;
-
-	const static inline PiscsiResponse piscsi_response;
-
-	static inline shared_ptr<ControllerManager> controller_manager;
-
-	static inline shared_ptr<PiscsiExecutor> executor;
+	static PbDeviceType ParseDeviceType(const string&);
 
 	// Processing flag
-	static inline volatile bool active;
+	atomic_bool target_is_active;
 
-	// Some versions of spdlog do not support get_log_level(), so we have to remember the level
-	static inline string current_log_level = "info";
+	string access_token;
 
-	static inline string access_token;
+	PiscsiImage piscsi_image;
+
+	PiscsiResponse response;
+
+	PiscsiService service;
+
+	unique_ptr<PiscsiExecutor> executor;
+
+	ControllerManager controller_manager;
+
+	unique_ptr<BUS> bus;
+
+	// Required for the termination handler
+	static inline Piscsi *instance;
 };

cpp/piscsi/piscsi_executor.cpp

@@ -3,35 +3,31 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
-#include "shared/log.h"
 #include "shared/piscsi_util.h"
 #include "shared/protobuf_util.h"
 #include "shared/piscsi_exceptions.h"
-#include "controllers/controller_manager.h"
 #include "controllers/scsi_controller.h"
-#include "devices/device_logger.h"
 #include "devices/device_factory.h"
 #include "devices/primary_device.h"
 #include "devices/disk.h"
-#include "piscsi_service.h"
 #include "piscsi_image.h"
 #include "localizer.h"
 #include "command_context.h"
 #include "piscsi_executor.h"
+#include <spdlog/spdlog.h>
 #include <sstream>
 
 using namespace spdlog;
 using namespace protobuf_util;
 using namespace piscsi_util;
 
-bool PiscsiExecutor::ProcessDeviceCmd(const CommandContext& context, const PbDeviceDefinition& pb_device,
-		const PbCommand& command, bool dryRun)
+bool PiscsiExecutor::ProcessDeviceCmd(const CommandContext& context, const PbDeviceDefinition& pb_device, bool dryRun)
 {
-	PrintCommand(command, pb_device, dryRun);
+	spdlog::info((dryRun ? "Validating: " : "Executing: ") + PrintCommand(context.GetCommand(), pb_device));
 
 	const int id = pb_device.id();
 	const int lun = pb_device.unit();
@@ -40,14 +36,14 @@ bool PiscsiExecutor::ProcessDeviceCmd(const CommandContext& context, const PbDev
 		return false;
 	}
 
-	const PbOperation operation = command.operation();
+	const PbOperation operation = context.GetCommand().operation();
 
 	// For all commands except ATTACH the device and LUN must exist
 	if (operation != ATTACH && !VerifyExistingIdAndLun(context, id, lun)) {
 		return false;
 	}
 
-	auto device = controller_manager.GetDeviceByIdAndLun(id, lun);
+	auto device = controller_manager.GetDeviceForIdAndLun(id, lun);
 
 	if (!ValidateOperationAgainstDevice(context, *device, operation)) {
 		return false;
@@ -64,7 +60,7 @@ bool PiscsiExecutor::ProcessDeviceCmd(const CommandContext& context, const PbDev
 			return Attach(context, pb_device, dryRun);
 
 		case DETACH:
-			return Detach(context, device, dryRun);
+			return Detach(context, *device, dryRun);
 
 		case INSERT:
 			return Insert(context, pb_device, device, dryRun);
@@ -82,7 +78,7 @@ bool PiscsiExecutor::ProcessDeviceCmd(const CommandContext& context, const PbDev
 		case CHECK_AUTHENTICATION:
 		case NO_OPERATION:
 			// Do nothing, just log
-			LOGTRACE("Received %s command", PbOperation_Name(operation).c_str())
+			spdlog::trace("Received " + PbOperation_Name(operation) + " command");
 			break;
 
 		default:
@@ -92,136 +88,83 @@ bool PiscsiExecutor::ProcessDeviceCmd(const CommandContext& context, const PbDev
 	return true;
 }
 
-bool PiscsiExecutor::ProcessCmd(const CommandContext& context, const PbCommand& command)
+bool PiscsiExecutor::ProcessCmd(const CommandContext& context)
 {
+	const PbCommand& command = context.GetCommand();
+
 	switch (command.operation()) {
 		case DETACH_ALL:
 			DetachAll();
-			return context.ReturnStatus();
+			return context.ReturnSuccessStatus();
 
 		case RESERVE_IDS: {
 			const string ids = GetParam(command, "ids");
 			if (const string error = SetReservedIds(ids); !error.empty()) {
-				return context.ReturnStatus(false, error);
+				return context.ReturnErrorStatus(error);
 			}
 
-			return context.ReturnStatus();
+			return context.ReturnSuccessStatus();
 		}
 
 		case CREATE_IMAGE:
-			return piscsi_image.CreateImage(context, command);
+			return piscsi_image.CreateImage(context);
 
 		case DELETE_IMAGE:
-			return piscsi_image.DeleteImage(context, command);
+			return piscsi_image.DeleteImage(context);
 
 		case RENAME_IMAGE:
-			return piscsi_image.RenameImage(context, command);
+			return piscsi_image.RenameImage(context);
 
 		case COPY_IMAGE:
-			return piscsi_image.CopyImage(context, command);
+			return piscsi_image.CopyImage(context);
 
 		case PROTECT_IMAGE:
 		case UNPROTECT_IMAGE:
-			return piscsi_image.SetImagePermissions(context, command);
+			return piscsi_image.SetImagePermissions(context);
 
 		default:
 			// This is a device-specific command handled below
 			break;
 	}
 
-	// Remember the list of reserved files, than run the dry run
+	// Remember the list of reserved files during the dry run
 	const auto& reserved_files = StorageDevice::GetReservedFiles();
-	for (const auto& device : command.devices()) {
-		if (!ProcessDeviceCmd(context, device, command, true)) {
-			// Dry run failed, restore the file list
-			StorageDevice::SetReservedFiles(reserved_files);
-			return false;
-		}
-	}
-
-	// Restore the list of reserved files before proceeding
+	const bool reserved = ranges::find_if_not(context.GetCommand().devices(), [&] (const auto& device)
+			{ return ProcessDeviceCmd(context, device, true); }) != command.devices().end();
 	StorageDevice::SetReservedFiles(reserved_files);
-
-	if (const string result = ValidateLunSetup(command); !result.empty()) {
-		return context.ReturnStatus(false, result);
-	}
-
-	for (const auto& device : command.devices()) {
-		if (!ProcessDeviceCmd(context, device, command, false)) {
-			return false;
-		}
-	}
-
-	// ATTACH and DETACH return the device list
-	if (context.IsValid() && (command.operation() == ATTACH || command.operation() == DETACH)) {
-		// A new command with an empty device list is required here in order to return data for all devices
-		PbCommand cmd;
-		PbResult result;
-		piscsi_response.GetDevicesInfo(controller_manager.GetAllDevices(), result, cmd,
-				piscsi_image.GetDefaultFolder());
-		serializer.SerializeMessage(context.GetFd(), result);
-		return true;
-	}
-
-	return context.ReturnStatus();
-}
-
-bool PiscsiExecutor::SetLogLevel(const string& log_level) const
-{
-	int id = -1;
-	int lun = -1;
-	string level = log_level;
-
-	if (size_t separator_pos = log_level.find(COMPONENT_SEPARATOR); separator_pos != string::npos) {
-		level = log_level.substr(0, separator_pos);
-
-		const string l = log_level.substr(separator_pos + 1);
-		separator_pos = l.find(COMPONENT_SEPARATOR);
-		if (separator_pos != string::npos) {
-			const string error = ProcessId(l, ScsiController::LUN_MAX, id, lun);
-			if (!error.empty()) {
-				LOGWARN("Invalid device ID/LUN specifier '%s'", l.c_str())
-				return false;
-			}
-		}
-		else if (!GetAsUnsignedInt(l, id)) {
-			LOGWARN("Invalid device ID specifier '%s'", l.c_str())
-			return false;
-		}
-	}
-
-	if (const auto& it = log_level_mapping.find(level); it != log_level_mapping.end()) {
-		set_level(it->second);
-	}
-	else {
-		LOGWARN("Invalid log level '%s'", log_level.c_str())
+	if (reserved) {
 		return false;
 	}
 
-	DeviceLogger::SetLogIdAndLun(id, lun);
-
-	if (id != -1) {
-		if (lun == -1) {
-			LOGINFO("Set log level for device ID %d to '%s'", id, level.c_str())
-		}
-		else {
-			LOGINFO("Set log level for device ID %d, LUN %d to '%s'", id, lun, level.c_str())
-		}
-	}
-	else {
-		LOGINFO("Set log level to '%s'", level.c_str())
+	if (const string error = EnsureLun0(command); !error.empty()) {
+		return context.ReturnErrorStatus(error);
 	}
 
-	return true;
+	if (ranges::find_if_not(command.devices(), [&] (const auto& device)
+			{ return ProcessDeviceCmd(context, device, false); } ) != command.devices().end()) {
+		return false;
+	}
+
+	// ATTACH and DETACH return the device list
+	if (command.operation() == ATTACH || command.operation() == DETACH) {
+		// A new command with an empty device list is required here in order to return data for all devices
+		PbCommand cmd;
+		PbResult result;
+		piscsi_response.GetDevicesInfo(controller_manager.GetAllDevices(), result, cmd, context.GetDefaultFolder());
+		context.WriteResult(result);
+		return true;
+	}
+
+	return context.ReturnSuccessStatus();
 }
 
 bool PiscsiExecutor::Start(PrimaryDevice& device, bool dryRun) const
 {
 	if (!dryRun) {
-		LOGINFO("Start requested for %s ID %d, unit %d", device.GetTypeString(), device.GetId(), device.GetLun())
+		spdlog::info("Start requested for " + device.GetIdentifier());
 
 		if (!device.Start()) {
-			LOGWARN("Starting %s ID %d, unit %d failed", device.GetTypeString(), device.GetId(), device.GetLun())
+			spdlog::warn("Starting " + device.GetIdentifier() + " failed");
 		}
 	}
 
@@ -231,7 +174,7 @@ bool PiscsiExecutor::Start(PrimaryDevice& device, bool dryRun) const
 bool PiscsiExecutor::Stop(PrimaryDevice& device, bool dryRun) const
 {
 	if (!dryRun) {
-		LOGINFO("Stop requested for %s ID %d, unit %d", device.GetTypeString(), device.GetId(), device.GetLun())
+		spdlog::info("Stop requested for " + device.GetIdentifier());
 
 		device.Stop();
 	}
@@ -242,10 +185,10 @@ bool PiscsiExecutor::Stop(PrimaryDevice& device, bool dryRun) const
 bool PiscsiExecutor::Eject(PrimaryDevice& device, bool dryRun) const
 {
 	if (!dryRun) {
-		LOGINFO("Eject requested for %s ID %d, unit %d", device.GetTypeString(), device.GetId(), device.GetLun())
+		spdlog::info("Eject requested for " + device.GetIdentifier());
 
 		if (!device.Eject(true)) {
-			LOGWARN("Ejecting %s ID %d, unit %d failed", device.GetTypeString(), device.GetId(), device.GetLun())
+			spdlog::warn("Ejecting " + device.GetIdentifier() + " failed");
 		}
 	}
 
@@ -255,8 +198,7 @@ bool PiscsiExecutor::Eject(PrimaryDevice& device, bool dryRun) const
 bool PiscsiExecutor::Protect(PrimaryDevice& device, bool dryRun) const
 {
 	if (!dryRun) {
-		LOGINFO("Write protection requested for %s ID %d, unit %d", device.GetTypeString(), device.GetId(),
-				device.GetLun())
+		spdlog::info("Write protection requested for " + device.GetIdentifier());
 
 		device.SetProtected(true);
 	}
@@ -267,8 +209,7 @@ bool PiscsiExecutor::Protect(PrimaryDevice& device, bool dryRun) const
 bool PiscsiExecutor::Unprotect(PrimaryDevice& device, bool dryRun) const
 {
 	if (!dryRun) {
-		LOGINFO("Write unprotection requested for %s ID %d, unit %d", device.GetTypeString(), device.GetId(),
-				device.GetLun())
+		spdlog::info("Write unprotection requested for " + device.GetIdentifier());
 
 		device.SetProtected(false);
 	}
@@ -282,15 +223,16 @@ bool PiscsiExecutor::Attach(const CommandContext& context, const PbDeviceDefinit
 	const int lun = pb_device.unit();
 	const PbDeviceType type = pb_device.type();
 
-	if (lun >= ScsiController::LUN_MAX) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_INVALID_LUN, to_string(lun), to_string(ScsiController::LUN_MAX));
+	if (lun >= ControllerManager::GetScsiLunMax()) {
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_INVALID_LUN, to_string(lun),
+				to_string(ControllerManager::GetScsiLunMax()));
 	}
 
-	if (controller_manager.GetDeviceByIdAndLun(id, lun) != nullptr) {
+	if (controller_manager.HasDeviceForIdAndLun(id, lun)) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_DUPLICATE_ID, to_string(id), to_string(lun));
 	}
 
-	if (reserved_ids.find(id) != reserved_ids.end()) {
+	if (reserved_ids.contains(id)) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_RESERVED_ID, to_string(id));
 	}
 
@@ -302,8 +244,7 @@ bool PiscsiExecutor::Attach(const CommandContext& context, const PbDeviceDefinit
 	}
 
 	// If no filename was provided the medium is considered not inserted
-	auto storage_device = dynamic_pointer_cast<StorageDevice>(device);
-	device->SetRemoved(storage_device != nullptr ? filename.empty() : false);
+	device->SetRemoved(device->SupportsFile() ? filename.empty() : false);
 
 	if (!SetProductData(context, pb_device, *device)) {
 		return false;
@@ -313,14 +254,14 @@ bool PiscsiExecutor::Attach(const CommandContext& context, const PbDeviceDefinit
 		return false;
 	}
 
-	string full_path;
+	const auto storage_device = dynamic_pointer_cast<StorageDevice>(device);
 	if (device->SupportsFile()) {
 		// Only with removable media drives, CD and MO the medium (=file) may be inserted later
 		if (!device->IsRemovable() && filename.empty()) {
 			return context.ReturnLocalizedError(LocalizationKey::ERROR_MISSING_FILENAME, PbDeviceType_Name(type));
 		}
 
-		if (!ValidateImageFile(context, *storage_device, filename, full_path)) {
+		if (!ValidateImageFile(context, *storage_device, filename)) {
 			return false;
 		}
 	}
@@ -336,23 +277,22 @@ bool PiscsiExecutor::Attach(const CommandContext& context, const PbDeviceDefinit
 		return true;
 	}
 
-	unordered_map<string, string> params = { pb_device.params().begin(), pb_device.params().end() };
+	param_map params = { pb_device.params().begin(), pb_device.params().end() };
 	if (!device->SupportsFile()) {
 		// Clients like scsictl might have sent both "file" and "interfaces"
 		params.erase("file");
 	}
 
 	if (!device->Init(params)) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_INITIALIZATION, PbDeviceType_Name(device->GetType()),
-				to_string(id), to_string(lun));
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_INITIALIZATION, device->GetIdentifier());
+	}
+
+	if (!controller_manager.AttachToController(bus, id, device)) {
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_SCSI_CONTROLLER);
 	}
 
 	if (storage_device != nullptr) {
-		storage_device->ReserveFile(full_path, id, lun);
-	}
-
-	if (!controller_manager.AttachToScsiController(id, device)) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_SCSI_CONTROLLER);
+		storage_device->ReserveFile();
 	}
 
 	string msg = "Attached ";
@@ -362,8 +302,8 @@ bool PiscsiExecutor::Attach(const CommandContext& context, const PbDeviceDefinit
 	else if (device->IsProtectable() && device->IsProtected()) {
 		msg += "protected ";
 	}
-	msg += string(device->GetTypeString()) + " device, ID " + to_string(id) + ", unit " + to_string(lun);
-	LOGINFO("%s", msg.c_str())
+	msg += device->GetIdentifier();
+	spdlog::info(msg);
 
 	return true;
 }
@@ -371,12 +311,11 @@ bool PiscsiExecutor::Attach(const CommandContext& context, const PbDeviceDefinit
 bool PiscsiExecutor::Insert(const CommandContext& context, const PbDeviceDefinition& pb_device,
 		const shared_ptr<PrimaryDevice>& device, bool dryRun) const
 {
-	auto storage_device = dynamic_pointer_cast<StorageDevice>(device);
-	if (storage_device == nullptr) {
+	if (!device->SupportsFile()) {
 		return false;
 	}
 
-	if (!storage_device->IsRemoved()) {
+	if (!device->IsRemoved()) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_EJECT_REQUIRED);
 	}
 
@@ -394,56 +333,52 @@ bool PiscsiExecutor::Insert(const CommandContext& context, const PbDeviceDefinit
 		return true;
 	}
 
-	LOGINFO("Insert %sfile '%s' requested into %s ID %d, unit %d", pb_device.protected_() ? "protected " : "",
-			filename.c_str(), storage_device->GetTypeString(), pb_device.id(), pb_device.unit())
+	spdlog::info("Insert " + string(pb_device.protected_() ? "protected " : "") + "file '" + filename +
+			"' requested into " + device->GetIdentifier());
 
+	// TODO It may be better to add PrimaryDevice::Insert for all device-specific insert operations
+	auto storage_device = dynamic_pointer_cast<StorageDevice>(device);
 	if (!SetSectorSize(context, storage_device, pb_device.block_size())) {
 		return false;
 	}
 
-	string full_path;
-	if (!ValidateImageFile(context, *storage_device, filename, full_path)) {
+	if (!ValidateImageFile(context, *storage_device, filename)) {
 		return false;
 	}
 
 	storage_device->SetProtected(pb_device.protected_());
-	storage_device->ReserveFile(full_path, storage_device->GetId(), storage_device->GetLun());
+	storage_device->ReserveFile();
 	storage_device->SetMediumChanged(true);
 
 	return true;
 }
 
-bool PiscsiExecutor::Detach(const CommandContext& context, const shared_ptr<PrimaryDevice>& device, bool dryRun) const
+bool PiscsiExecutor::Detach(const CommandContext& context, PrimaryDevice& device, bool dryRun)
 {
-	auto controller = controller_manager.FindController(device->GetId());
+	auto controller = controller_manager.FindController(device.GetId());
 	if (controller == nullptr) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_DETACH);
 	}
 
 	// LUN 0 can only be detached if there is no other LUN anymore
-	if (!device->GetLun() && controller->GetLunCount() > 1) {
+	if (!device.GetLun() && controller->GetLunCount() > 1) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_LUN0);
 	}
 
 	if (!dryRun) {
-		// Remember the ID before it gets invalid when removing the device
-		const int id = device->GetId();
+		// Remember the device identifier for the log message before the device data become invalid on removal
+		const string identifier = device.GetIdentifier();
 
 		if (!controller->RemoveDevice(device)) {
 			return context.ReturnLocalizedError(LocalizationKey::ERROR_DETACH);
 		}
 
 		// If no LUN is left also delete the controller
-		if (!controller->GetLunCount() && !controller_manager.DeleteController(controller)) {
+		if (!controller->GetLunCount() && !controller_manager.DeleteController(*controller)) {
 			return context.ReturnLocalizedError(LocalizationKey::ERROR_DETACH);
 		}
 
-		if (auto storage_device = dynamic_pointer_cast<StorageDevice>(device); storage_device != nullptr) {
-			storage_device->UnreserveFile();
-		}
-
-		LOGINFO("%s", ("Detached " + string(device->GetTypeString()) + " device with ID " + to_string(id)
-				+ ", unit " + to_string(device->GetLun())).c_str())
+		spdlog::info("Detached " + identifier);
 	}
 
 	return true;
@@ -452,9 +387,8 @@ bool PiscsiExecutor::Detach(const CommandContext& context, const shared_ptr<Prim
 void PiscsiExecutor::DetachAll()
 {
 	controller_manager.DeleteAllControllers();
-	StorageDevice::UnreserveAll();
 
-	LOGINFO("Detached all devices")
+	spdlog::info("Detached all devices");
 }
 
 bool PiscsiExecutor::ShutDown(const CommandContext& context, const string& mode) {
@@ -467,9 +401,9 @@ bool PiscsiExecutor::ShutDown(const CommandContext& context, const string& mode)
 
 	// The PiSCSI shutdown mode is "rascsi" instead of "piscsi" for backwards compatibility
 	if (mode == "rascsi") {
-		LOGINFO("PiSCSI shutdown requested")
+		spdlog::info("PiSCSI shutdown requested");
 
-		serializer.SerializeMessage(context.GetFd(), result);
+		context.WriteResult(result);
 
 		return true;
 	}
@@ -484,145 +418,116 @@ bool PiscsiExecutor::ShutDown(const CommandContext& context, const string& mode)
 	}
 
 	if (mode == "system") {
-		LOGINFO("System shutdown requested")
-
-		serializer.SerializeMessage(context.GetFd(), result);
+		spdlog::info("System shutdown requested");
 
 		DetachAll();
 
+		context.WriteResult(result);
+
 		if (system("init 0") == -1) {
-			LOGERROR("System shutdown failed: %s", strerror(errno))
+			spdlog::error("System shutdown failed");
 		}
 	}
 	else if (mode == "reboot") {
-		LOGINFO("System reboot requested")
-
-		serializer.SerializeMessage(context.GetFd(), result);
+		spdlog::info("System reboot requested");
 
 		DetachAll();
 
+		context.WriteResult(result);
+
 		if (system("init 6") == -1) {
-			LOGERROR("System reboot failed: %s", strerror(errno))
+			spdlog::error("System reboot failed");
 		}
 	}
-	else {
-		assert(false);
-	}
 
 	return false;
 }
 
 string PiscsiExecutor::SetReservedIds(string_view ids)
 {
-	list<string> ids_to_reserve;
+	set<int> ids_to_reserve;
 	stringstream ss(ids.data());
     string id;
     while (getline(ss, id, ',')) {
-    	if (!id.empty()) {
-    		ids_to_reserve.push_back(id);
-    	}
-    }
-
-    set<int> reserved;
-    for (const string& id_to_reserve : ids_to_reserve) {
     	int res_id;
- 		if (!GetAsUnsignedInt(id_to_reserve, res_id) || res_id > 7) {
- 			return "Invalid ID " + id_to_reserve;
- 		}
-
- 		if (controller_manager.FindController(res_id) != nullptr) {
- 			return "ID " + id_to_reserve + " is currently in use";
- 		}
-
- 		reserved.insert(res_id);
-    }
-
-    reserved_ids = { reserved.begin(), reserved.end() };
-
-    if (!reserved_ids.empty()) {
-    	string s;
-    	bool isFirst = true;
-    	for (const auto& reserved_id : reserved) {
-    		if (!isFirst) {
-    			s += ", ";
-    		}
-    		isFirst = false;
-    		s += to_string(reserved_id);
+    	if (!GetAsUnsignedInt(id, res_id) || res_id > 7) {
+    		return "Invalid ID " + id;
     	}
 
-    	LOGINFO("Reserved ID(s) set to %s", s.c_str())
+    	if (controller_manager.HasController(res_id)) {
+    		return "ID " + id + " is currently in use";
+    	}
+
+    	ids_to_reserve.insert(res_id);
+    }
+
+    reserved_ids = { ids_to_reserve.begin(), ids_to_reserve.end() };
+
+    if (!ids_to_reserve.empty()) {
+    	spdlog::info("Reserved ID(s) set to " + Join(ids_to_reserve));
     }
     else {
-    	LOGINFO("Cleared reserved ID(s)")
+    	spdlog::info("Cleared reserved ID(s)");
     }
 
 	return "";
 }
 
 bool PiscsiExecutor::ValidateImageFile(const CommandContext& context, StorageDevice& storage_device,
-		const string& filename, string& full_path) const
+		const string& filename) const
 {
 	if (filename.empty()) {
 		return true;
 	}
 
-	if (const auto [id1, lun1] = StorageDevice::GetIdsForReservedFile(filename); id1 != -1 || lun1 != -1) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_IMAGE_IN_USE, filename,
-				to_string(id1), to_string(lun1));
+	if (!CheckForReservedFile(context, filename)) {
+		return false;
 	}
 
-	string effective_filename = filename;
+	storage_device.SetFilename(filename);
 
 	if (!StorageDevice::FileExists(filename)) {
 		// If the file does not exist search for it in the default image folder
-		effective_filename = piscsi_image.GetDefaultFolder() + "/" + filename;
+		const string effective_filename = context.GetDefaultFolder() + "/" + filename;
 
-		if (const auto [id2, lun2] = StorageDevice::GetIdsForReservedFile(effective_filename); id2 != -1 || lun2 != -1) {
-			return context.ReturnLocalizedError(LocalizationKey::ERROR_IMAGE_IN_USE, filename,
-					to_string(id2), to_string(lun2));
+		if (!CheckForReservedFile(context, effective_filename)) {
+			return false;
 		}
 
-		if (!StorageDevice::FileExists(effective_filename)) {
-			return context.ReturnLocalizedError(LocalizationKey::ERROR_FILE_OPEN, effective_filename);
-		}
-	}
-
-	storage_device.SetFilename(effective_filename);
-
-	if (storage_device.IsReadOnlyFile()) {
-		// Permanently write-protected
-		storage_device.SetReadOnly(true);
-		storage_device.SetProtectable(false);
-	}
-	else {
-		storage_device.SetReadOnly(false);
-		storage_device.SetProtectable(true);
+		storage_device.SetFilename(effective_filename);
 	}
 
 	try {
 		storage_device.Open();
 	}
 	catch(const io_exception&) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_FILE_OPEN, effective_filename);
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_FILE_OPEN, storage_device.GetFilename());
 	}
 
-	full_path = effective_filename;
-
 	return true;
 }
 
-void PiscsiExecutor::PrintCommand(const PbCommand& command, const PbDeviceDefinition& pb_device, bool dryRun) const
+bool PiscsiExecutor::CheckForReservedFile(const CommandContext& context, const string& filename)
+{
+	if (const auto [id, lun] = StorageDevice::GetIdsForReservedFile(filename); id != -1) {
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_IMAGE_IN_USE, filename,
+				to_string(id) + ":" + to_string(lun));
+	}
+
+	return true;
+}
+
+string PiscsiExecutor::PrintCommand(const PbCommand& command, const PbDeviceDefinition& pb_device) const
 {
 	const map<string, string, less<>> params = { command.params().begin(), command.params().end() };
 
 	ostringstream s;
-	s << (dryRun ? "Validating" : "Executing");
-	s << ": operation=" << PbOperation_Name(command.operation());
+	s << "operation=" << PbOperation_Name(command.operation());
 
 	if (!params.empty()) {
 		s << ", command params=";
 		bool isFirst = true;
-		for (const auto& [key, value]: params) {
+		for (const auto& [key, value] : params) {
 			if (!isFirst) {
 				s << ", ";
 			}
@@ -632,8 +537,7 @@ void PiscsiExecutor::PrintCommand(const PbCommand& command, const PbDeviceDefini
 		}
 	}
 
-	s << ", device id=" << pb_device.id() << ", lun=" << pb_device.unit() << ", type="
-			<< PbDeviceType_Name(pb_device.type());
+	s << ", device=" << pb_device.id() << ":" << pb_device.unit() << ", type=" << PbDeviceType_Name(pb_device.type());
 
 	if (pb_device.params_size()) {
 		s << ", device params=";
@@ -649,13 +553,14 @@ void PiscsiExecutor::PrintCommand(const PbCommand& command, const PbDeviceDefini
 
 	s << ", vendor='" << pb_device.vendor() << "', product='" << pb_device.product()
 		<< "', revision='" << pb_device.revision() << "', block size=" << pb_device.block_size();
-	LOGINFO("%s", s.str().c_str())
+
+	return s.str();
 }
 
-string PiscsiExecutor::ValidateLunSetup(const PbCommand& command) const
+string PiscsiExecutor::EnsureLun0(const PbCommand& command) const
 {
 	// Mapping of available LUNs (bit vector) to devices
-	unordered_map<uint32_t, uint32_t> luns;
+	unordered_map<int32_t, int32_t> luns;
 
 	// Collect LUN bit vectors of new devices
 	for (const auto& device : command.devices()) {
@@ -667,23 +572,17 @@ string PiscsiExecutor::ValidateLunSetup(const PbCommand& command) const
 		luns[device->GetId()] |= 1 << device->GetLun();
 	}
 
-	// LUN 0 must exist for all devices
-	for (const auto& [id, lun]: luns) {
-		if (!(lun & 0x01)) {
-			return "LUN 0 is missing for device ID " + to_string(id);
-		}
-	}
-
-	return "";
+	const auto& it = ranges::find_if_not(luns, [] (const auto& l) { return l.second & 0x01; } );
+	return it == luns.end() ? "" : "LUN 0 is missing for device ID " + to_string((*it).first);
 }
 
 bool PiscsiExecutor::VerifyExistingIdAndLun(const CommandContext& context, int id, int lun) const
 {
-	if (controller_manager.FindController(id) == nullptr) {
+	if (!controller_manager.HasController(id)) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_NON_EXISTING_DEVICE, to_string(id));
 	}
 
-	if (controller_manager.GetDeviceByIdAndLun(id, lun) == nullptr) {
+	if (!controller_manager.HasDeviceForIdAndLun(id, lun)) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_NON_EXISTING_UNIT, to_string(id), to_string(lun));
 	}
 
@@ -706,13 +605,13 @@ shared_ptr<PrimaryDevice> PiscsiExecutor::CreateDevice(const CommandContext& con
 	return device;
 }
 
-bool PiscsiExecutor::SetSectorSize(const CommandContext& context, shared_ptr<PrimaryDevice> device, int block_size) const
+bool PiscsiExecutor::SetSectorSize(const CommandContext& context, shared_ptr<PrimaryDevice> device, int size) const
 {
-	if (block_size) {
-		auto disk = dynamic_pointer_cast<Disk>(device);
+	if (size) {
+		const auto disk = dynamic_pointer_cast<Disk>(device);
 		if (disk != nullptr && disk->IsSectorSizeConfigurable()) {
-			if (!disk->SetConfiguredSectorSize(device_factory, block_size)) {
-				return context.ReturnLocalizedError(LocalizationKey::ERROR_BLOCK_SIZE, to_string(block_size));
+			if (!disk->SetConfiguredSectorSize(device_factory, size)) {
+				return context.ReturnLocalizedError(LocalizationKey::ERROR_BLOCK_SIZE, to_string(size));
 			}
 		}
 		else {
@@ -725,22 +624,26 @@ bool PiscsiExecutor::SetSectorSize(const CommandContext& context, shared_ptr<Pri
 }
 
 bool PiscsiExecutor::ValidateOperationAgainstDevice(const CommandContext& context, const PrimaryDevice& device,
-		const PbOperation& operation)
+		PbOperation operation)
 {
 	if ((operation == START || operation == STOP) && !device.IsStoppable()) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_STOPPABLE, device.GetTypeString());
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_STOPPABLE, PbOperation_Name(operation),
+				device.GetTypeString());
 	}
 
 	if ((operation == INSERT || operation == EJECT) && !device.IsRemovable()) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_REMOVABLE, device.GetTypeString());
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_REMOVABLE, PbOperation_Name(operation),
+				device.GetTypeString());
 	}
 
 	if ((operation == PROTECT || operation == UNPROTECT) && !device.IsProtectable()) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_PROTECTABLE, device.GetTypeString());
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_PROTECTABLE, PbOperation_Name(operation),
+				device.GetTypeString());
 	}
 
 	if ((operation == PROTECT || operation == UNPROTECT) && !device.IsReady()) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_READY, device.GetTypeString());
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_READY, PbOperation_Name(operation),
+				device.GetTypeString());
 	}
 
 	return true;
@@ -748,15 +651,16 @@ bool PiscsiExecutor::ValidateOperationAgainstDevice(const CommandContext& contex
 
 bool PiscsiExecutor::ValidateIdAndLun(const CommandContext& context, int id, int lun)
 {
-	// Validate the device ID and LUN
 	if (id < 0) {
 		return context.ReturnLocalizedError(LocalizationKey::ERROR_MISSING_DEVICE_ID);
 	}
-	if (id >= ControllerManager::DEVICE_MAX) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_INVALID_ID, to_string(id), to_string(ControllerManager::DEVICE_MAX - 1));
+	if (id >= ControllerManager::GetScsiIdMax()) {
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_INVALID_ID, to_string(id),
+				to_string(ControllerManager::GetScsiIdMax() - 1));
 	}
-	if (lun < 0 || lun >= ScsiController::LUN_MAX) {
-		return context.ReturnLocalizedError(LocalizationKey::ERROR_INVALID_LUN, to_string(lun), to_string(ScsiController::LUN_MAX - 1));
+	if (lun < 0 || lun >= ControllerManager::GetScsiLunMax()) {
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_INVALID_LUN, to_string(lun),
+				to_string(ControllerManager::GetScsiLunMax() - 1));
 	}
 
 	return true;
@@ -777,7 +681,7 @@ bool PiscsiExecutor::SetProductData(const CommandContext& context, const PbDevic
 		}
 	}
 	catch(const invalid_argument& e) {
-		return context.ReturnStatus(false, e.what());
+		return context.ReturnErrorStatus(e.what());
 	}
 
 	return true;

cpp/piscsi/piscsi_executor.h

@@ -3,22 +3,21 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
 #pragma once
 
-#include "spdlog/spdlog.h"
-#include "shared/protobuf_serializer.h"
+#include "hal/bus.h"
+#include "controllers/controller_manager.h"
 #include "piscsi/piscsi_response.h"
 #include <unordered_set>
-#include <unordered_map>
 
 class PiscsiImage;
 class DeviceFactory;
-class ControllerManager;
 class PrimaryDevice;
+class StorageDevice;
 class CommandContext;
 
 using namespace spdlog;
@@ -27,15 +26,14 @@ class PiscsiExecutor
 {
 public:
 
-	PiscsiExecutor(PiscsiImage& piscsi_image, ControllerManager& controller_manager)
-		: piscsi_image(piscsi_image), controller_manager(controller_manager) {}
+	PiscsiExecutor(PiscsiImage& piscsi_image, BUS& bus, ControllerManager& controller_manager)
+		: piscsi_image(piscsi_image), bus(bus), controller_manager(controller_manager) {}
 	~PiscsiExecutor() = default;
 
-	unordered_set<int> GetReservedIds() const { return reserved_ids; }
+	auto GetReservedIds() const { return reserved_ids; }
 
-	bool ProcessDeviceCmd(const CommandContext&, const PbDeviceDefinition&, const PbCommand&, bool);
-	bool ProcessCmd(const CommandContext&, const PbCommand&);
-	bool SetLogLevel(const string&) const;
+	bool ProcessDeviceCmd(const CommandContext&, const PbDeviceDefinition&, bool);
+	bool ProcessCmd(const CommandContext&);
 	bool Start(PrimaryDevice&, bool) const;
 	bool Stop(PrimaryDevice&, bool) const;
 	bool Eject(PrimaryDevice&, bool) const;
@@ -43,41 +41,34 @@ public:
 	bool Unprotect(PrimaryDevice&, bool) const;
 	bool Attach(const CommandContext&, const PbDeviceDefinition&, bool);
 	bool Insert(const CommandContext&, const PbDeviceDefinition&, const shared_ptr<PrimaryDevice>&, bool) const;
-	bool Detach(const CommandContext&, const shared_ptr<PrimaryDevice>&, bool) const;
+	bool Detach(const CommandContext&, PrimaryDevice&, bool);
 	void DetachAll();
 	bool ShutDown(const CommandContext&, const string&);
 	string SetReservedIds(string_view);
-	bool ValidateImageFile(const CommandContext&, StorageDevice&, const string&, string&) const;
-	void PrintCommand(const PbCommand&, const PbDeviceDefinition&, bool) const;
-	string ValidateLunSetup(const PbCommand&) const;
+	bool ValidateImageFile(const CommandContext&, StorageDevice&, const string&) const;
+	string PrintCommand(const PbCommand&, const PbDeviceDefinition&) const;
+	string EnsureLun0(const PbCommand&) const;
 	bool VerifyExistingIdAndLun(const CommandContext&, int, int) const;
 	shared_ptr<PrimaryDevice> CreateDevice(const CommandContext&, const PbDeviceType, int, const string&) const;
 	bool SetSectorSize(const CommandContext&, shared_ptr<PrimaryDevice>, int) const;
 
-	static bool ValidateOperationAgainstDevice(const CommandContext&, const PrimaryDevice&, const PbOperation&);
+	static bool ValidateOperationAgainstDevice(const CommandContext&, const PrimaryDevice&, PbOperation);
 	static bool ValidateIdAndLun(const CommandContext&, int, int);
 	static bool SetProductData(const CommandContext&, const PbDeviceDefinition&, PrimaryDevice&);
 
 private:
 
+	static bool CheckForReservedFile(const CommandContext&, const string&);
+
 	const PiscsiResponse piscsi_response;
 
 	PiscsiImage& piscsi_image;
 
+	BUS& bus;
+
 	ControllerManager& controller_manager;
 
 	const DeviceFactory device_factory;
 
-	const ProtobufSerializer serializer;
-
 	unordered_set<int> reserved_ids;
-
-	static inline const unordered_map<string, level::level_enum> log_level_mapping = {
-			{ "trace", level::trace },
-			{ "debug", level::debug },
-			{ "info", level::info },
-			{ "warn", level::warn },
-			{ "err", level::err },
-			{ "off", level::off }
-	};
 };

cpp/piscsi/piscsi_image.cpp

@@ -3,21 +3,18 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
-#include "shared/log.h"
-#include "shared/protobuf_util.h"
 #include "devices/disk.h"
-#include "command_context.h"
 #include "piscsi_image.h"
+#include "shared/protobuf_util.h"
+#include <spdlog/spdlog.h>
 #include <unistd.h>
 #include <pwd.h>
 #include <fstream>
-#include <string>
 #include <array>
-#include <filesystem>
 
 using namespace std;
 using namespace filesystem;
@@ -32,14 +29,12 @@ PiscsiImage::PiscsiImage()
 
 bool PiscsiImage::CheckDepth(string_view filename) const
 {
-	return count(filename.begin(), filename.end(), '/') <= depth;
+	return ranges::count(filename, '/') <= depth;
 }
 
-bool PiscsiImage::CreateImageFolder(const CommandContext& context, const string& filename) const
+bool PiscsiImage::CreateImageFolder(const CommandContext& context, string_view filename) const
 {
-	if (const size_t filename_start = filename.rfind('/'); filename_start != string::npos) {
-		const auto folder = path(filename.substr(0, filename_start));
-
+	if (const auto folder = path(filename).parent_path(); !folder.string().empty()) {
 		// Checking for existence first prevents an error if the top-level folder is a softlink
 		if (error_code error; exists(folder, error)) {
 			return true;
@@ -51,67 +46,64 @@ bool PiscsiImage::CreateImageFolder(const CommandContext& context, const string&
 			return ChangeOwner(context, folder, false);
 		}
 		catch(const filesystem_error& e) {
-			return context.ReturnStatus(false, "Can't create image folder '" + string(folder) + "': " + e.what());
+			return context.ReturnErrorStatus("Can't create image folder '" + folder.string() + "': " + e.what());
 		}
 	}
 
 	return true;
 }
 
-string PiscsiImage::SetDefaultFolder(const string& f)
+string PiscsiImage::SetDefaultFolder(string_view f)
 {
 	if (f.empty()) {
 		return "Can't set default image folder: Missing folder name";
 	}
 
-	string folder = f;
-
 	// If a relative path is specified, the path is assumed to be relative to the user's home directory
-	if (folder[0] != '/') {
-		folder = GetHomeDir() + "/" + folder;
+	path folder(f);
+	if (folder.is_relative()) {
+		folder = path(GetHomeDir() + "/" + folder.string());
 	}
-	else {
-		if (folder.find("/home/") != 0) {
-			return "Default image folder must be located in '/home/'";
-		}
+
+	if (path home_root = path(GetHomeDir()).parent_path(); !folder.string().starts_with(home_root.string())) {
+		return "Default image folder must be located in '" + home_root.string() + "'";
 	}
 
 	// Resolve a potential symlink
-	auto p = path(folder);
-	if (error_code error; is_symlink(p, error)) {
-		p = read_symlink(p);
+	if (error_code error; is_symlink(folder, error)) {
+		folder = read_symlink(folder);
 	}
 
-	if (error_code error; !is_directory(p, error)) {
-		return "'" + string(p) + "' is not a valid folder";
+	if (error_code error; !is_directory(folder)) {
+		return string("'") + folder.string() + "' is not a valid image folder";
 	}
 
-	default_folder = string(p);
+	default_folder = folder.string();
 
-	LOGINFO("Default image folder set to '%s'", default_folder.c_str())
+	spdlog::info("Default image folder set to '" + default_folder + "'");
 
 	return "";
 }
 
-bool PiscsiImage::CreateImage(const CommandContext& context, const PbCommand& command) const
+bool PiscsiImage::CreateImage(const CommandContext& context) const
 {
-	const string filename = GetParam(command, "file");
+	const string filename = GetParam(context.GetCommand(), "file");
 	if (filename.empty()) {
-		return context.ReturnStatus(false, "Can't create image file: Missing image filename");
+		return context.ReturnErrorStatus("Missing image filename");
 	}
 
 	if (!CheckDepth(filename)) {
-		return context.ReturnStatus(false, ("Invalid folder hierarchy depth '" + filename + "'").c_str());
+		return context.ReturnErrorStatus(("Invalid folder hierarchy depth '" + filename + "'").c_str());
 	}
 
 	const string full_filename = GetFullName(filename);
 	if (!IsValidDstFilename(full_filename)) {
-		return context.ReturnStatus(false, "Can't create image file: '" + full_filename + "': File already exists");
+		return context.ReturnErrorStatus("Can't create image file: '" + full_filename + "': File already exists");
 	}
 
-	const string size = GetParam(command, "size");
+	const string size = GetParam(context.GetCommand(), "size");
 	if (size.empty()) {
-		return context.ReturnStatus(false, "Can't create image file '" + full_filename + "': Missing file size");
+		return context.ReturnErrorStatus("Can't create image file '" + full_filename + "': Missing file size");
 	}
 
 	off_t len;
@@ -119,20 +111,20 @@ bool PiscsiImage::CreateImage(const CommandContext& context, const PbCommand& co
 		len = stoull(size);
 	}
 	catch(const invalid_argument&) {
-		return context.ReturnStatus(false, "Can't create image file '" + full_filename + "': Invalid file size " + size);
+		return context.ReturnErrorStatus("Can't create image file '" + full_filename + "': Invalid file size " + size);
 	}
 	catch(const out_of_range&) {
-		return context.ReturnStatus(false, "Can't create image file '" + full_filename + "': Invalid file size " + size);
+		return context.ReturnErrorStatus("Can't create image file '" + full_filename + "': Invalid file size " + size);
 	}
 	if (len < 512 || (len & 0x1ff)) {
-		return context.ReturnStatus(false, "Invalid image file size " + to_string(len) + " (not a multiple of 512)");
+		return context.ReturnErrorStatus("Invalid image file size " + to_string(len) + " (not a multiple of 512)");
 	}
 
 	if (!CreateImageFolder(context, full_filename)) {
 		return false;
 	}
 
-	const bool read_only = GetParam(command, "read_only") == "true";
+	const bool read_only = GetParam(context.GetCommand(), "read_only") == "true";
 
 	error_code error;
 	path file(full_filename);
@@ -149,40 +141,37 @@ bool PiscsiImage::CreateImage(const CommandContext& context, const PbCommand& co
 	catch(const filesystem_error& e) {
 		remove(file, error);
 
-		return context.ReturnStatus(false, "Can't create image file '" + full_filename + "': " + e.what());
+		return context.ReturnErrorStatus("Can't create image file '" + full_filename + "': " + e.what());
 	}
 
-	LOGINFO("%s", string("Created " + string(read_only ? "read-only " : "") + "image file '" + full_filename +
-			"' with a size of " + to_string(len) + " bytes").c_str())
+	spdlog::info("Created " + string(read_only ? "read-only " : "") + "image file '" + full_filename +
+			"' with a size of " + to_string(len) + " bytes");
 
-	return context.ReturnStatus();
+	return context.ReturnSuccessStatus();
 }
 
-bool PiscsiImage::DeleteImage(const CommandContext& context, const PbCommand& command) const
+bool PiscsiImage::DeleteImage(const CommandContext& context) const
 {
-	const string filename = GetParam(command, "file");
+	const string filename = GetParam(context.GetCommand(), "file");
 	if (filename.empty()) {
-		return context.ReturnStatus(false, "Missing image filename");
+		return context.ReturnErrorStatus("Missing image filename");
 	}
 
 	if (!CheckDepth(filename)) {
-		return context.ReturnStatus(false, ("Invalid folder hierarchy depth '" + filename + "'").c_str());
+		return context.ReturnErrorStatus("Invalid folder hierarchy depth '" + filename + "'");
 	}
 
 	const auto full_filename = path(GetFullName(filename));
-
 	if (!exists(full_filename)) {
-		return context.ReturnStatus(false, "Image file '" + string(full_filename) + "' does not exist");
+		return context.ReturnErrorStatus("Image file '" + full_filename.string() + "' does not exist");
 	}
 
-	const auto [id, lun] = StorageDevice::GetIdsForReservedFile(full_filename);
-	if (id != -1 || lun != -1) {
-		return context.ReturnStatus(false, "Can't delete image file '" + string(full_filename) +
-				"', it is currently being used by device ID " + to_string(id) + ", LUN " + to_string(lun));
+	if (!IsReservedFile(context, full_filename, "delete")) {
+		return false;
 	}
 
 	if (error_code error; !remove(full_filename, error)) {
-		return context.ReturnStatus(false, "Can't delete image file '" + string(full_filename) + "'");
+		return context.ReturnErrorStatus("Can't delete image file '" + full_filename.string() + "'");
 	}
 
 	// Delete empty subfolders
@@ -196,32 +185,22 @@ bool PiscsiImage::DeleteImage(const CommandContext& context, const PbCommand& co
 		}
 
 		if (error_code error; !remove(full_folder)) {
-			return context.ReturnStatus(false, "Can't delete empty image folder '" + string(full_folder) +  "'");
+			return context.ReturnErrorStatus("Can't delete empty image folder '" + full_folder.string() +  "'");
 		}
 
 		last_slash = folder.rfind('/');
 	}
 
-	LOGINFO("Deleted image file '%s'", full_filename.c_str())
+	spdlog::info("Deleted image file '" + full_filename.string() + "'");
 
-	return context.ReturnStatus();
+	return context.ReturnSuccessStatus();
 }
 
-bool PiscsiImage::RenameImage(const CommandContext& context, const PbCommand& command) const
+bool PiscsiImage::RenameImage(const CommandContext& context) const
 {
 	string from;
 	string to;
-	if (!ValidateParams(context, command, "rename/move", from, to)) {
-		return false;
-	}
-
-	const auto [id, lun] = StorageDevice::GetIdsForReservedFile(from);
-	if (id != -1 || lun != -1) {
-		return context.ReturnStatus(false, "Can't rename/move image file '" + from +
-				"', it is currently being used by device ID " + to_string(id) + ", LUN " + to_string(lun));
-	}
-
-	if (!CreateImageFolder(context, to)) {
+	if (!ValidateParams(context, "rename/move", from, to)) {
 		return false;
 	}
 
@@ -229,33 +208,19 @@ bool PiscsiImage::RenameImage(const CommandContext& context, const PbCommand& co
 		rename(path(from), path(to));
 	}
 	catch(const filesystem_error& e) {
-		return context.ReturnStatus(false, "Can't rename/move image file '" + from + "' to '" + to + "': " + e.what());
+		return context.ReturnErrorStatus("Can't rename/move image file '" + from + "': " + e.what());
 	}
 
-	LOGINFO("Renamed/Moved image file '%s' to '%s'", from.c_str(), to.c_str())
+	spdlog::info("Renamed/Moved image file '" + from + "' to '" + to + "'");
 
-	return context.ReturnStatus();
+	return context.ReturnSuccessStatus();
 }
 
-bool PiscsiImage::CopyImage(const CommandContext& context, const PbCommand& command) const
+bool PiscsiImage::CopyImage(const CommandContext& context) const
 {
 	string from;
 	string to;
-	if (!ValidateParams(context, command, "copy", from, to)) {
-		return false;
-	}
-
-	if (access(from.c_str(), R_OK)) {
-    	return context.ReturnStatus(false, "Can't read source image file '" + from + "'");
-    }
-
-	const auto [id, lun] = StorageDevice::GetIdsForReservedFile(from);
-	if (id != -1 || lun != -1) {
-		return context.ReturnStatus(false, "Can't copy image file '" + from +
-				"', it is currently being used by device ID " + to_string(id) + ", LUN " + to_string(lun));
-	}
-
-	if (!CreateImageFolder(context, to)) {
+	if (!ValidateParams(context, "copy", from, to)) {
 		return false;
 	}
 
@@ -268,119 +233,134 @@ bool PiscsiImage::CopyImage(const CommandContext& context, const PbCommand& comm
 			copy_symlink(f, t);
 		}
 		catch(const filesystem_error& e) {
-	    	return context.ReturnStatus(false, "Can't copy image file symlink '" + from + "': " + e.what());
+	    	return context.ReturnErrorStatus("Can't copy image file symlink '" + from + "': " + e.what());
 		}
 
-		LOGINFO("Copied image file symlink '%s' to '%s'", from.c_str(), to.c_str())
+		spdlog::info("Copied image file symlink '" + from + "' to '" + to + "'");
 
-		return context.ReturnStatus();
+		return context.ReturnSuccessStatus();
 	}
 
 	try {
 		copy_file(f, t);
 
-		permissions(t, GetParam(command, "read_only") == "true" ?
+		permissions(t, GetParam(context.GetCommand(), "read_only") == "true" ?
 				perms::owner_read | perms::group_read | perms::others_read :
 				perms::owner_read | perms::group_read | perms::others_read |
 				perms::owner_write | perms::group_write);
 	}
 	catch(const filesystem_error& e) {
-        return context.ReturnStatus(false, "Can't copy image file '" + from + "' to '" + to + "': " + e.what());
+        return context.ReturnErrorStatus("Can't copy image file '" + from + "': " + e.what());
 	}
 
-	LOGINFO("Copied image file '%s' to '%s'", from.c_str(), to.c_str())
+	spdlog::info("Copied image file '" + from + "' to '" + to + "'");
 
-	return context.ReturnStatus();
+	return context.ReturnSuccessStatus();
 }
 
-bool PiscsiImage::SetImagePermissions(const CommandContext& context, const PbCommand& command) const
+bool PiscsiImage::SetImagePermissions(const CommandContext& context) const
 {
-	string filename = GetParam(command, "file");
+	const string filename = GetParam(context.GetCommand(), "file");
 	if (filename.empty()) {
-		return context.ReturnStatus(false, "Missing image filename");
+		return context.ReturnErrorStatus("Missing image filename");
 	}
 
 	if (!CheckDepth(filename)) {
-		return context.ReturnStatus(false, ("Invalid folder hierarchy depth '" + filename + "'").c_str());
+		return context.ReturnErrorStatus("Invalid folder hierarchy depth '" + filename + "'");
 	}
 
-	filename = GetFullName(filename);
-	if (!IsValidSrcFilename(filename)) {
-		return context.ReturnStatus(false, "Can't modify image file '" + filename + "': Invalid name or type");
+	const string full_filename = GetFullName(filename);
+	if (!IsValidSrcFilename(full_filename)) {
+		return context.ReturnErrorStatus("Can't modify image file '" + full_filename + "': Invalid name or type");
 	}
 
-	const bool protect = command.operation() == PROTECT_IMAGE;
+	const bool protect = context.GetCommand().operation() == PROTECT_IMAGE;
 
 	try {
-		permissions(path(filename), protect ?
+		permissions(path(full_filename), protect ?
 				perms::owner_read | perms::group_read | perms::others_read :
 				perms::owner_read | perms::group_read | perms::others_read |
 				perms::owner_write | perms::group_write);
 	}
 	catch(const filesystem_error& e) {
-		return context.ReturnStatus(false, "Can't " + string(protect ? "protect" : "unprotect") + " image file '" +
-				filename + "': " + e.what());
+		return context.ReturnErrorStatus("Can't " + string(protect ? "protect" : "unprotect") + " image file '" +
+				full_filename + "': " + e.what());
 	}
 
-	if (protect) {
-		LOGINFO("Protected image file '%s'", filename.c_str())
-	}
-	else {
-		LOGINFO("Unprotected image file '%s'", filename.c_str())
-	}
+	spdlog::info((protect ? "Protected" : "Unprotected") + string(" image file '") + full_filename + "'");
 
-	return context.ReturnStatus();
+	return context.ReturnSuccessStatus();
 }
 
-bool PiscsiImage::ValidateParams(const CommandContext& context, const PbCommand& command, const string& operation,
-		string& from, string& to) const
+bool PiscsiImage::IsReservedFile(const CommandContext& context, const string& file, const string& op)
 {
-	from = GetParam(command, "from");
-	if (from.empty()) {
-		return context.ReturnStatus(false, "Can't " + operation + " image file: Missing source filename");
-	}
-
-	if (!CheckDepth(from)) {
-		return context.ReturnStatus(false, ("Invalid folder hierarchy depth '" + from + "'").c_str());
-	}
-
-	from = GetFullName(from);
-	if (!IsValidSrcFilename(from)) {
-		return context.ReturnStatus(false, "Can't " + operation + " image file: '" + from + "': Invalid name or type");
-	}
-
-	to = GetParam(command, "to");
-	if (to.empty()) {
-		return context.ReturnStatus(false, "Can't " + operation + " image file '" + from + "': Missing destination filename");
-	}
-
-	if (!CheckDepth(to)) {
-		return context.ReturnStatus(false, ("Invalid folder hierarchy depth '" + to + "'").c_str());
-	}
-
-	to = GetFullName(to);
-	if (!IsValidDstFilename(to)) {
-		return context.ReturnStatus(false, "Can't " + operation + " image file '" + from + "' to '" + to + "': File already exists");
+	const auto [id, lun] = StorageDevice::GetIdsForReservedFile(file);
+	if (id != -1) {
+		return context.ReturnErrorStatus("Can't " + op + " image file '" + file +
+				"', it is currently being used by device " + to_string(id) + ":" + to_string(lun));
 	}
 
 	return true;
 }
 
-bool PiscsiImage::IsValidSrcFilename(const string& filename)
+bool PiscsiImage::ValidateParams(const CommandContext& context, const string& op, string& from, string& to) const
+{
+	from = GetParam(context.GetCommand(), "from");
+	if (from.empty()) {
+		return context.ReturnErrorStatus("Can't " + op + " image file: Missing source filename");
+	}
+
+	if (!CheckDepth(from)) {
+		return context.ReturnErrorStatus("Invalid folder hierarchy depth '" + from + "'");
+	}
+
+	to = GetParam(context.GetCommand(), "to");
+	if (to.empty()) {
+		return context.ReturnErrorStatus("Can't " + op + " image file '" + from + "': Missing destination filename");
+	}
+
+	if (!CheckDepth(to)) {
+		return context.ReturnErrorStatus("Invalid folder hierarchy depth '" + to + "'");
+	}
+
+	from = GetFullName(from);
+	if (!IsValidSrcFilename(from)) {
+		return context.ReturnErrorStatus("Can't " + op + " image file: '" + from + "': Invalid name or type");
+	}
+
+	to = GetFullName(to);
+	if (!IsValidDstFilename(to)) {
+		return context.ReturnErrorStatus("Can't " + op + " image file '" + from + "' to '" + to + "': File already exists");
+	}
+
+	if (!IsReservedFile(context, from, op)) {
+		return false;
+	}
+
+	if (!CreateImageFolder(context, to)) {
+		return false;
+	}
+
+	return true;
+}
+
+bool PiscsiImage::IsValidSrcFilename(string_view filename)
 {
 	// Source file must exist and must be a regular file or a symlink
 	path file(filename);
-	return is_regular_file(file) || is_symlink(file);
+
+	error_code error;
+	return is_regular_file(file, error) || is_symlink(file, error);
 }
 
-bool PiscsiImage::IsValidDstFilename(const string& filename)
+bool PiscsiImage::IsValidDstFilename(string_view filename)
 {
 	// Destination file must not yet exist
 	try {
 		return !exists(path(filename));
 	}
 	catch(const filesystem_error&) {
-		return true;
+		return false;
 	}
 }
 
@@ -394,7 +374,7 @@ bool PiscsiImage::ChangeOwner(const CommandContext& context, const path& filenam
 		error_code error;
 		remove(filename, error);
 
-		return context.ReturnStatus(false, "Can't change ownership of '" + string(filename) + "': " + strerror(e));
+		return context.ReturnErrorStatus("Can't change ownership of '" + filename.string() + "': " + strerror(e));
 	}
 
 	permissions(filename, read_only ?
@@ -437,5 +417,5 @@ pair<int, int> PiscsiImage::GetUidAndGid()
 		gid = pwd.pw_gid;
 	}
 
-	return make_pair(uid, gid);
+	return { uid, gid };
 }

cpp/piscsi/piscsi_image.h

@@ -3,7 +3,7 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -28,22 +28,23 @@ public:
 	void SetDepth(int d) { depth = d; }
 	int GetDepth() const { return depth; }
 	string GetDefaultFolder() const { return default_folder; }
-	string SetDefaultFolder(const string&);
-	bool CreateImage(const CommandContext&, const PbCommand&) const;
-	bool DeleteImage(const CommandContext&, const PbCommand&) const;
-	bool RenameImage(const CommandContext&, const PbCommand&) const;
-	bool CopyImage(const CommandContext&, const PbCommand&) const;
-	bool SetImagePermissions(const CommandContext&, const PbCommand&) const;
+	string SetDefaultFolder(string_view);
+	bool CreateImage(const CommandContext&) const;
+	bool DeleteImage(const CommandContext&) const;
+	bool RenameImage(const CommandContext&) const;
+	bool CopyImage(const CommandContext&) const;
+	bool SetImagePermissions(const CommandContext&) const;
 
 private:
 
 	bool CheckDepth(string_view) const;
 	string GetFullName(const string& filename) const { return default_folder + "/" + filename; }
-	bool CreateImageFolder(const CommandContext&, const string&) const;
-	bool ValidateParams(const CommandContext&, const PbCommand&, const string&, string&, string&) const;
+	bool CreateImageFolder(const CommandContext&, string_view) const;
+	static bool IsReservedFile(const CommandContext&, const string&, const string&);
+	bool ValidateParams(const CommandContext&, const string&, string&, string&) const;
 
-	static bool IsValidSrcFilename(const string&);
-	static bool IsValidDstFilename(const string&);
+	static bool IsValidSrcFilename(string_view);
+	static bool IsValidDstFilename(string_view);
 	static bool ChangeOwner(const CommandContext&, const path&, bool);
 	static string GetHomeDir();
 	static pair<int, int> GetUidAndGid();

cpp/piscsi/piscsi_response.cpp

@@ -3,50 +3,48 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
-#include "shared/log.h"
+#include "controllers/controller_manager.h"
 #include "shared/protobuf_util.h"
+#include "shared/network_util.h"
+#include "shared/piscsi_util.h"
 #include "shared/piscsi_version.h"
 #include "devices/disk.h"
-#include "devices/device_factory.h"
-#include "generated/piscsi_interface.pb.h"
 #include "piscsi_response.h"
+#include <spdlog/spdlog.h>
 #include <filesystem>
 
 using namespace std;
 using namespace filesystem;
 using namespace piscsi_interface;
+using namespace piscsi_util;
+using namespace network_util;
 using namespace protobuf_util;
 
-unique_ptr<PbDeviceProperties> PiscsiResponse::GetDeviceProperties(const Device& device) const
+void PiscsiResponse::GetDeviceProperties(const Device& device, PbDeviceProperties& properties) const
 {
-	auto properties = make_unique<PbDeviceProperties>();
-
-	// Currently there is only a SCSI controller, i.e. there can always be 32 LUNs
-	properties->set_luns(32);
-	properties->set_read_only(device.IsReadOnly());
-	properties->set_protectable(device.IsProtectable());
-	properties->set_stoppable(device.IsStoppable());
-	properties->set_removable(device.IsRemovable());
-	properties->set_lockable(device.IsLockable());
-	properties->set_supports_file(device.SupportsFile());
-	properties->set_supports_params(device.SupportsParams());
+	properties.set_luns(ControllerManager::GetScsiLunMax());
+	properties.set_read_only(device.IsReadOnly());
+	properties.set_protectable(device.IsProtectable());
+	properties.set_stoppable(device.IsStoppable());
+	properties.set_removable(device.IsRemovable());
+	properties.set_lockable(device.IsLockable());
+	properties.set_supports_file(device.SupportsFile());
+	properties.set_supports_params(device.SupportsParams());
 
 	if (device.SupportsParams()) {
 		for (const auto& [key, value] : device_factory.GetDefaultParams(device.GetType())) {
-			auto& map = *properties->mutable_default_params();
+			auto& map = *properties.mutable_default_params();
 			map[key] = value;
 		}
 	}
 
 	for (const auto& block_size : device_factory.GetSectorSizes(device.GetType())) {
-		properties->add_block_sizes(block_size);
+		properties.add_block_sizes(block_size);
 	}
-
-	return properties;
 }
 
 void PiscsiResponse::GetDeviceTypeProperties(PbDeviceTypesInfo& device_types_info, PbDeviceType type) const
@@ -54,10 +52,10 @@ void PiscsiResponse::GetDeviceTypeProperties(PbDeviceTypesInfo& device_types_inf
 	auto type_properties = device_types_info.add_properties();
 	type_properties->set_type(type);
 	const auto device = device_factory.CreateDevice(type, 0, "");
-	type_properties->set_allocated_properties(GetDeviceProperties(*device).release());
-} //NOSONAR The allocated memory is managed by protobuf
+	GetDeviceProperties(*device, *type_properties->mutable_properties());
+}
 
-void PiscsiResponse::GetAllDeviceTypeProperties(PbDeviceTypesInfo& device_types_info) const
+void PiscsiResponse::GetDeviceTypesInfo(PbDeviceTypesInfo& device_types_info) const
 {
 	// Start with 2 instead of 1. 1 was the removed SASI drive type.
 	int ordinal = 2;
@@ -78,16 +76,15 @@ void PiscsiResponse::GetDevice(const Device& device, PbDevice& pb_device, const
 	pb_device.set_revision(device.GetRevision());
 	pb_device.set_type(device.GetType());
 
-    pb_device.set_allocated_properties(GetDeviceProperties(device).release());
+    GetDeviceProperties(device, *pb_device.mutable_properties());
 
-    auto status = make_unique<PbDeviceStatus>().release(); //NOSONAR The allocated memory is managed by protobuf
-	pb_device.set_allocated_status(status);
+    auto status = pb_device.mutable_status();
 	status->set_protected_(device.IsProtected());
 	status->set_stopped(device.IsStopped());
 	status->set_removed(device.IsRemoved());
 	status->set_locked(device.IsLocked());
 
-	if (device.SupportsParams()) { //NOSONAR The allocated memory is managed by protobuf
+	if (device.SupportsParams()) {
 		for (const auto& [key, value] : device.GetParams()) {
 			SetParam(pb_device, key, value);
 		}
@@ -100,11 +97,9 @@ void PiscsiResponse::GetDevice(const Device& device, PbDevice& pb_device, const
 
     const auto storage_device = dynamic_cast<const StorageDevice *>(&device);
 	if (storage_device != nullptr) {
-		auto image_file = make_unique<PbImageFile>().release();
-		GetImageFile(*image_file, default_folder, device.IsReady() ? storage_device->GetFilename() : "");
-		pb_device.set_allocated_file(image_file);
+		GetImageFile(*pb_device.mutable_file(), default_folder, device.IsReady() ? storage_device->GetFilename() : "");
 	}
-} //NOSONAR The allocated memory is managed by protobuf
+}
 
 bool PiscsiResponse::GetImageFile(PbImageFile& image_file, const string& default_folder, const string& filename) const
 {
@@ -112,13 +107,13 @@ bool PiscsiResponse::GetImageFile(PbImageFile& image_file, const string& default
 		image_file.set_name(filename);
 		image_file.set_type(device_factory.GetTypeForFile(filename));
 
-		const string f = filename[0] == '/' ? filename : default_folder + "/" + filename;
+		const path p(filename[0] == '/' ? filename : default_folder + "/" + filename);
 
-		image_file.set_read_only(access(f.c_str(), W_OK));
+		image_file.set_read_only(access(p.c_str(), W_OK));
 
-		// filesystem::file_size cannot be used here because gcc < 10.3.0 cannot handle files of more than 2 GiB
-		if (struct stat st; !stat(f.c_str(), &st) && !S_ISDIR(st.st_mode)) {
-			image_file.set_size(st.st_size);
+		error_code error;
+		if (is_regular_file(p, error) || (is_symlink(p, error) && !is_block_file(p, error))) {
+			image_file.set_size(file_size(p));
 			return true;
 		}
 	}
@@ -127,92 +122,68 @@ bool PiscsiResponse::GetImageFile(PbImageFile& image_file, const string& default
 }
 
 void PiscsiResponse::GetAvailableImages(PbImageFilesInfo& image_files_info, const string& default_folder,
-		const string& folder, const string& folder_pattern, const string& file_pattern, int scan_depth) const
+		const string& folder_pattern, const string& file_pattern, int scan_depth) const
 {
-	if (scan_depth-- < 0) {
+	const path default_path(default_folder);
+	if (!is_directory(default_path)) {
 		return;
 	}
 
-	string folder_pattern_lower = folder_pattern;
-	transform(folder_pattern_lower.begin(), folder_pattern_lower.end(), folder_pattern_lower.begin(), ::tolower);
+	string folder_pattern_lower;
+	ranges::transform(folder_pattern, back_inserter(folder_pattern_lower), ::tolower);
 
-	string file_pattern_lower = file_pattern;
-	transform(file_pattern_lower.begin(), file_pattern_lower.end(), file_pattern_lower.begin(), ::tolower);
+	string file_pattern_lower;
+	ranges::transform(file_pattern, back_inserter(file_pattern_lower), ::tolower);
 
-	DIR *d = opendir(folder.c_str());
-	if (d == nullptr) {
-		return;
-	}
-
-	// C++ filesystem cannot be used here because gcc < 10.3.0 cannot handle files of more than 2 GiB
-	const dirent *dir;
-	while ((dir = readdir(d))) {
-		string filename = GetNextImageFile(dir, folder);
-		if (filename.empty()) {
+	for (auto it = recursive_directory_iterator(default_path, directory_options::follow_directory_symlink);
+		it != recursive_directory_iterator(); it++) {
+		if (it.depth() > scan_depth) {
+			it.disable_recursion_pending();
 			continue;
 		}
 
-		string name_lower = dir->d_name;
-		if (!file_pattern.empty()) {
-			transform(name_lower.begin(), name_lower.end(), name_lower.begin(), ::tolower);
-		}
+		const string parent = it->path().parent_path().string();
 
-		if (dir->d_type == DT_DIR) {
-			if (folder_pattern_lower.empty() || name_lower.find(folder_pattern_lower) != string::npos) {
-				GetAvailableImages(image_files_info, default_folder, filename, folder_pattern,
-						file_pattern, scan_depth);
-			}
+		const string folder = parent.size() > default_folder.size() ? parent.substr(default_folder.size() + 1) : "";
 
+		if (!FilterMatches(folder, folder_pattern_lower) || !FilterMatches(it->path().filename().string(), file_pattern_lower)) {
 			continue;
 		}
 
-		if (file_pattern_lower.empty() || name_lower.find(file_pattern_lower) != string::npos) {
-			if (auto image_file = make_unique<PbImageFile>(); GetImageFile(*image_file.get(), default_folder, filename)) {
-				GetImageFile(*image_files_info.add_image_files(), default_folder,
-						filename.substr(default_folder.length() + 1));
-			}
+		if (!ValidateImageFile(it->path())) {
+			continue;
+		}
+
+		const string filename = folder.empty() ?
+				it->path().filename().string() : folder + "/" + it->path().filename().string();
+		if (PbImageFile image_file; GetImageFile(image_file, default_folder, filename)) {
+			GetImageFile(*image_files_info.add_image_files(), default_folder, filename);
 		}
 	}
-
-	closedir(d);
 }
 
-unique_ptr<PbImageFilesInfo> PiscsiResponse::GetAvailableImages(PbResult& result, const string& default_folder,
+void PiscsiResponse::GetImageFilesInfo(PbImageFilesInfo& image_files_info, const string& default_folder,
 		const string& folder_pattern, const string& file_pattern, int scan_depth) const
 {
-	auto image_files_info = make_unique<PbImageFilesInfo>();
+	image_files_info.set_default_image_folder(default_folder);
+	image_files_info.set_depth(scan_depth);
 
-	image_files_info->set_default_image_folder(default_folder);
-	image_files_info->set_depth(scan_depth);
-
-	GetAvailableImages(*image_files_info, default_folder, default_folder, folder_pattern,
-			file_pattern, scan_depth);
-
-	result.set_status(true);
-
-	return image_files_info;
+	GetAvailableImages(image_files_info, default_folder, folder_pattern, file_pattern, scan_depth);
 }
 
-void PiscsiResponse::GetAvailableImages(PbResult& result, PbServerInfo& server_info, const string& default_folder,
+void PiscsiResponse::GetAvailableImages(PbServerInfo& server_info, const string& default_folder,
 		const string& folder_pattern, const string& file_pattern, int scan_depth) const
 {
-	auto image_files_info = GetAvailableImages(result, default_folder, folder_pattern, file_pattern, scan_depth);
-	image_files_info->set_default_image_folder(default_folder);
-	server_info.set_allocated_image_files_info(image_files_info.release());
+	server_info.mutable_image_files_info()->set_default_image_folder(default_folder);
 
-	result.set_status(true); //NOSONAR The allocated memory is managed by protobuf
+	GetImageFilesInfo(*server_info.mutable_image_files_info(), default_folder, folder_pattern, file_pattern, scan_depth);
 }
 
-unique_ptr<PbReservedIdsInfo> PiscsiResponse::GetReservedIds(PbResult& result, const unordered_set<int>& ids) const
+void PiscsiResponse::GetReservedIds(PbReservedIdsInfo& reserved_ids_info, const unordered_set<int>& ids) const
 {
-	auto reserved_ids_info = make_unique<PbReservedIdsInfo>();
 	for (const int id : ids) {
-		reserved_ids_info->add_ids(id);
+		reserved_ids_info.add_ids(id);
 	}
-
-	result.set_status(true);
-
-	return reserved_ids_info;
 }
 
 void PiscsiResponse::GetDevices(const unordered_set<shared_ptr<PrimaryDevice>>& devices, PbServerInfo& server_info,
@@ -232,7 +203,7 @@ void PiscsiResponse::GetDevicesInfo(const unordered_set<shared_ptr<PrimaryDevice
 	// If no device list was provided in the command get information on all devices
 	if (!command.devices_size()) {
 		for (const auto& device : devices) {
-			id_sets.insert(make_pair(device->GetId(), device->GetLun()));
+			id_sets.insert({ device->GetId(), device->GetLun() });
 		}
 	}
 	// Otherwise get information on the devices provided in the command
@@ -243,8 +214,7 @@ void PiscsiResponse::GetDevicesInfo(const unordered_set<shared_ptr<PrimaryDevice
 		}
 	}
 
-	auto devices_info = make_unique<PbDevicesInfo>();
-
+	auto devices_info = result.mutable_devices_info();
 	for (const auto& [id, lun] : id_sets) {
 		for (const auto& d : devices) {
 			if (d->GetId() == id && d->GetLun() == lun) {
@@ -254,244 +224,179 @@ void PiscsiResponse::GetDevicesInfo(const unordered_set<shared_ptr<PrimaryDevice
 		}
 	}
 
-	result.set_allocated_devices_info(devices_info.release());
 	result.set_status(true);
 }
 
-unique_ptr<PbDeviceTypesInfo> PiscsiResponse::GetDeviceTypesInfo(PbResult& result) const
+void PiscsiResponse::GetServerInfo(PbServerInfo& server_info, const unordered_set<shared_ptr<PrimaryDevice>>& devices,
+		const unordered_set<int>& reserved_ids, const string& default_folder, const string& folder_pattern,
+		const string& file_pattern, int scan_depth) const
 {
-	auto device_types_info = make_unique<PbDeviceTypesInfo>();
-
-	GetAllDeviceTypeProperties(*device_types_info);
-
-	result.set_status(true);
-
-	return device_types_info;
+	GetVersionInfo(*server_info.mutable_version_info());
+	GetLogLevelInfo(*server_info.mutable_log_level_info());
+	GetDeviceTypesInfo(*server_info.mutable_device_types_info());
+	GetAvailableImages(server_info, default_folder, folder_pattern, file_pattern, scan_depth);
+	GetNetworkInterfacesInfo(*server_info.mutable_network_interfaces_info());
+	GetMappingInfo(*server_info.mutable_mapping_info());
+	GetDevices(devices, server_info, default_folder);
+	GetReservedIds(*server_info.mutable_reserved_ids_info(), reserved_ids);
+	GetOperationInfo(*server_info.mutable_operation_info(), scan_depth);
 }
 
-unique_ptr<PbServerInfo> PiscsiResponse::GetServerInfo(const unordered_set<shared_ptr<PrimaryDevice>>& devices,
-		PbResult& result, const unordered_set<int>& reserved_ids, const string& current_log_level,
-		const string& default_folder, const string& folder_pattern, const string& file_pattern, int scan_depth) const
+void PiscsiResponse::GetVersionInfo(PbVersionInfo& version_info) const
 {
-	auto server_info = make_unique<PbServerInfo>();
-
-	server_info->set_allocated_version_info(GetVersionInfo(result).release());
-	server_info->set_allocated_log_level_info(GetLogLevelInfo(result, current_log_level).release()); //NOSONAR The allocated memory is managed by protobuf
-	GetAllDeviceTypeProperties(*server_info->mutable_device_types_info()); //NOSONAR The allocated memory is managed by protobuf
-	GetAvailableImages(result, *server_info, default_folder, folder_pattern, file_pattern, scan_depth);
-	server_info->set_allocated_network_interfaces_info(GetNetworkInterfacesInfo(result).release());
-	server_info->set_allocated_mapping_info(GetMappingInfo(result).release()); //NOSONAR The allocated memory is managed by protobuf
-	GetDevices(devices, *server_info, default_folder); //NOSONAR The allocated memory is managed by protobuf
-	server_info->set_allocated_reserved_ids_info(GetReservedIds(result, reserved_ids).release());
-	server_info->set_allocated_operation_info(GetOperationInfo(result, scan_depth).release()); //NOSONAR The allocated memory is managed by protobuf
-
-	result.set_status(true);
-
-	return server_info;
+	version_info.set_major_version(piscsi_major_version);
+	version_info.set_minor_version(piscsi_minor_version);
+	version_info.set_patch_version(piscsi_patch_version);
 }
 
-unique_ptr<PbVersionInfo> PiscsiResponse::GetVersionInfo(PbResult& result) const
+void PiscsiResponse::GetLogLevelInfo(PbLogLevelInfo& log_level_info) const
 {
-	auto version_info = make_unique<PbVersionInfo>();
-
-	version_info->set_major_version(piscsi_major_version);
-	version_info->set_minor_version(piscsi_minor_version);
-	version_info->set_patch_version(piscsi_patch_version);
-
-	result.set_status(true);
-
-	return version_info;
-}
-
-unique_ptr<PbLogLevelInfo> PiscsiResponse::GetLogLevelInfo(PbResult& result, const string& current_log_level) const
-{
-	auto log_level_info = make_unique<PbLogLevelInfo>();
-
-	for (const auto& log_level : log_levels) {
-		log_level_info->add_log_levels(log_level);
+	for (const auto& log_level : spdlog::level::level_string_views) {
+		log_level_info.add_log_levels(log_level.data());
 	}
 
-	log_level_info->set_current_log_level(current_log_level);
-
-	result.set_status(true);
-
-	return log_level_info;
+	log_level_info.set_current_log_level(spdlog::level::level_string_views[spdlog::get_level()].data());
 }
 
-unique_ptr<PbNetworkInterfacesInfo> PiscsiResponse::GetNetworkInterfacesInfo(PbResult& result) const
+void PiscsiResponse::GetNetworkInterfacesInfo(PbNetworkInterfacesInfo& network_interfaces_info) const
 {
-	auto network_interfaces_info = make_unique<PbNetworkInterfacesInfo>();
-
-	for (const auto& network_interface : device_factory.GetNetworkInterfaces()) {
-		network_interfaces_info->add_name(network_interface);
+	for (const auto& network_interface : GetNetworkInterfaces()) {
+		network_interfaces_info.add_name(network_interface);
 	}
-
-	result.set_status(true);
-
-	return network_interfaces_info;
 }
 
-unique_ptr<PbMappingInfo> PiscsiResponse::GetMappingInfo(PbResult& result) const
+void PiscsiResponse::GetMappingInfo(PbMappingInfo& mapping_info) const
 {
-	auto mapping_info = make_unique<PbMappingInfo>();
-
 	for (const auto& [name, type] : device_factory.GetExtensionMapping()) {
-		(*mapping_info->mutable_mapping())[name] = type;
+		(*mapping_info.mutable_mapping())[name] = type;
 	}
-
-	result.set_status(true);
-
-	return mapping_info;
 }
 
-unique_ptr<PbOperationInfo> PiscsiResponse::GetOperationInfo(PbResult& result, int depth) const
+void PiscsiResponse::GetOperationInfo(PbOperationInfo& operation_info, int depth) const
 {
-	auto operation_info = make_unique<PbOperationInfo>();
+	auto operation = CreateOperation(operation_info, ATTACH, "Attach device, device-specific parameters are required");
+	AddOperationParameter(*operation, "name", "Image file name in case of a mass storage device");
+	AddOperationParameter(*operation, "interface", "Comma-separated prioritized network interface list");
+	AddOperationParameter(*operation, "inet", "IP address and netmask of the network bridge");
+	AddOperationParameter(*operation, "cmd", "Print command for the printer device");
 
-	auto operation = CreateOperation(*operation_info, ATTACH, "Attach device, device-specific parameters are required");
-	AddOperationParameter(*operation, "name", "Image file name in case of a mass storage device").release();
-	AddOperationParameter(*operation, "interface", "Comma-separated prioritized network interface list").release();
-	AddOperationParameter(*operation, "inet", "IP address and netmask of the network bridge").release();
-	AddOperationParameter(*operation, "cmd", "Print command for the printer device").release();
-	operation.release();
+	CreateOperation(operation_info, DETACH, "Detach device, device-specific parameters are required");
 
-	CreateOperation(*operation_info, DETACH, "Detach device, device-specific parameters are required").release();
+	CreateOperation(operation_info, DETACH_ALL, "Detach all devices");
 
-	CreateOperation(*operation_info, DETACH_ALL, "Detach all devices").release();
+	CreateOperation(operation_info, START, "Start device, device-specific parameters are required");
 
-	CreateOperation(*operation_info, START, "Start device, device-specific parameters are required").release();
+	CreateOperation(operation_info, STOP, "Stop device, device-specific parameters are required");
 
-	CreateOperation(*operation_info, STOP, "Stop device, device-specific parameters are required").release();
+	operation = CreateOperation(operation_info, INSERT, "Insert medium, device-specific parameters are required");
+	AddOperationParameter(*operation, "file", "Image file name", "", true);
 
-	operation = CreateOperation(*operation_info, INSERT, "Insert medium, device-specific parameters are required");
-	AddOperationParameter(*operation, "file", "Image file name", "", true).release();
-	operation.release();
+	CreateOperation(operation_info, EJECT, "Eject medium, device-specific parameters are required");
 
-	CreateOperation(*operation_info, EJECT, "Eject medium, device-specific parameters are required").release();
+	CreateOperation(operation_info, PROTECT, "Protect medium, device-specific parameters are required");
 
-	CreateOperation(*operation_info, PROTECT, "Protect medium, device-specific parameters are required").release();
+	CreateOperation(operation_info, UNPROTECT, "Unprotect medium, device-specific parameters are required");
 
-	CreateOperation(*operation_info, UNPROTECT, "Unprotect medium, device-specific parameters are required").release();
-
-	operation = CreateOperation(*operation_info, SERVER_INFO, "Get piscsi server information");
+	operation = CreateOperation(operation_info, SERVER_INFO, "Get piscsi server information");
 	if (depth) {
-		AddOperationParameter(*operation, "folder_pattern", "Pattern for filtering image folder names").release();
+		AddOperationParameter(*operation, "folder_pattern", "Pattern for filtering image folder names");
 	}
-	AddOperationParameter(*operation, "file_pattern", "Pattern for filtering image file names").release();
-	operation.release();
+	AddOperationParameter(*operation, "file_pattern", "Pattern for filtering image file names");
 
-	CreateOperation(*operation_info, VERSION_INFO, "Get piscsi server version").release();
+	CreateOperation(operation_info, VERSION_INFO, "Get piscsi server version");
 
-	CreateOperation(*operation_info, DEVICES_INFO, "Get information on attached devices").release();
+	CreateOperation(operation_info, DEVICES_INFO, "Get information on attached devices");
 
-	CreateOperation(*operation_info, DEVICE_TYPES_INFO, "Get device properties by device type").release();
+	CreateOperation(operation_info, DEVICE_TYPES_INFO, "Get device properties by device type");
 
-	operation = CreateOperation(*operation_info, DEFAULT_IMAGE_FILES_INFO, "Get information on available image files");
+	operation = CreateOperation(operation_info, DEFAULT_IMAGE_FILES_INFO, "Get information on available image files");
 	if (depth) {
-		AddOperationParameter(*operation, "folder_pattern", "Pattern for filtering image folder names").release();
+		AddOperationParameter(*operation, "folder_pattern", "Pattern for filtering image folder names");
 	}
-	AddOperationParameter(*operation, "file_pattern", "Pattern for filtering image file names").release();
-	operation.release();
+	AddOperationParameter(*operation, "file_pattern", "Pattern for filtering image file names");
 
-	operation = CreateOperation(*operation_info, IMAGE_FILE_INFO, "Get information on image file");
-	AddOperationParameter(*operation, "file", "Image file name", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, IMAGE_FILE_INFO, "Get information on image file");
+	AddOperationParameter(*operation, "file", "Image file name", "", true);
 
-	CreateOperation(*operation_info, LOG_LEVEL_INFO, "Get log level information").release();
+	CreateOperation(operation_info, LOG_LEVEL_INFO, "Get log level information");
 
-	CreateOperation(*operation_info, NETWORK_INTERFACES_INFO, "Get the available network interfaces").release();
+	CreateOperation(operation_info, NETWORK_INTERFACES_INFO, "Get the available network interfaces");
 
-	CreateOperation(*operation_info, MAPPING_INFO, "Get mapping of extensions to device types").release();
+	CreateOperation(operation_info, MAPPING_INFO, "Get mapping of extensions to device types");
 
-	CreateOperation(*operation_info, RESERVED_IDS_INFO, "Get list of reserved device IDs").release();
+	CreateOperation(operation_info, RESERVED_IDS_INFO, "Get list of reserved device IDs");
 
-	operation = CreateOperation(*operation_info, DEFAULT_FOLDER, "Set default image file folder");
-	AddOperationParameter(*operation, "folder", "Default image file folder name", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, DEFAULT_FOLDER, "Set default image file folder");
+	AddOperationParameter(*operation, "folder", "Default image file folder name", "", true);
 
-	operation = CreateOperation(*operation_info, LOG_LEVEL, "Set log level");
-	AddOperationParameter(*operation, "level", "New log level", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, LOG_LEVEL, "Set log level");
+	AddOperationParameter(*operation, "level", "New log level", "", true);
 
-	operation = CreateOperation(*operation_info, RESERVE_IDS, "Reserve device IDs");
-	AddOperationParameter(*operation, "ids", "Comma-separated device ID list", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, RESERVE_IDS, "Reserve device IDs");
+	AddOperationParameter(*operation, "ids", "Comma-separated device ID list", "", true);
 
-	operation = CreateOperation(*operation_info, SHUT_DOWN, "Shut down or reboot");
-	auto parameter = AddOperationParameter(*operation, "mode", "Shutdown mode", "", true).release();
-	parameter->add_permitted_values("piscsi");
-	// System shutdown/reboot requires root permissions
-	if (!getuid()) {
-		parameter->add_permitted_values("system");
-		parameter->add_permitted_values("reboot");
+	operation = CreateOperation(operation_info, SHUT_DOWN, "Shut down or reboot");
+	if (getuid()) {
+		AddOperationParameter(*operation, "mode", "Shutdown mode", "", true, { "rascsi" } );
+	}
+	else {
+		// System shutdown/reboot requires root permissions
+		AddOperationParameter(*operation, "mode", "Shutdown mode", "", true, { "rascsi", "system", "reboot" } );
 	}
-	operation.release();
 
-	operation = CreateOperation(*operation_info, CREATE_IMAGE, "Create an image file");
-	AddOperationParameter(*operation, "file", "Image file name", "", true).release();
-	AddOperationParameter(*operation, "size", "Image file size in bytes", "", true).release();
-	parameter = AddOperationParameter(*operation, "read_only",  "Read-only flag", "false").release();
-	parameter->add_permitted_values("true");
-	parameter->add_permitted_values("false");
-	operation.release();
+	operation = CreateOperation(operation_info, CREATE_IMAGE, "Create an image file");
+	AddOperationParameter(*operation, "file", "Image file name", "", true);
+	AddOperationParameter(*operation, "size", "Image file size in bytes", "", true);
+	AddOperationParameter(*operation, "read_only",  "Read-only flag", "false", false, { "true", "false" } );
 
-	operation = CreateOperation(*operation_info, DELETE_IMAGE, "Delete image file");
-	AddOperationParameter(*operation, "file", "Image file name", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, DELETE_IMAGE, "Delete image file");
+	AddOperationParameter(*operation, "file", "Image file name", "", true);
 
-	operation = CreateOperation(*operation_info, RENAME_IMAGE, "Rename image file");
-	AddOperationParameter(*operation, "from", "Source image file name", "", true).release();
-	AddOperationParameter(*operation, "to", "Destination image file name", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, RENAME_IMAGE, "Rename image file");
+	AddOperationParameter(*operation, "from", "Source image file name", "", true);
+	AddOperationParameter(*operation, "to", "Destination image file name", "", true);
 
-	operation = CreateOperation(*operation_info, COPY_IMAGE, "Copy image file");
-	AddOperationParameter(*operation, "from", "Source image file name", "", true).release();
-	AddOperationParameter(*operation, "to", "Destination image file name", "", true).release();
-	parameter = AddOperationParameter(*operation, "read_only", "Read-only flag", "false").release();
-	parameter->add_permitted_values("true");
-	parameter->add_permitted_values("false");
-	operation.release();
+	operation = CreateOperation(operation_info, COPY_IMAGE, "Copy image file");
+	AddOperationParameter(*operation, "from", "Source image file name", "", true);
+	AddOperationParameter(*operation, "to", "Destination image file name", "", true);
+	AddOperationParameter(*operation, "read_only", "Read-only flag", "false", false, { "true", "false" } );
 
-	operation = CreateOperation(*operation_info, PROTECT_IMAGE, "Write-protect image file");
-	AddOperationParameter(*operation, "file", "Image file name", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, PROTECT_IMAGE, "Write-protect image file");
+	AddOperationParameter(*operation, "file", "Image file name", "", true);
 
-	operation = CreateOperation(*operation_info, UNPROTECT_IMAGE, "Make image file writable");
-	AddOperationParameter(*operation, "file", "Image file name", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, UNPROTECT_IMAGE, "Make image file writable");
+	AddOperationParameter(*operation, "file", "Image file name", "", true);
 
-	operation = CreateOperation(*operation_info, CHECK_AUTHENTICATION, "Check whether an authentication token is valid");
-	AddOperationParameter(*operation, "token", "Authentication token to be checked", "", true).release();
-	operation.release();
+	operation = CreateOperation(operation_info, CHECK_AUTHENTICATION, "Check whether an authentication token is valid");
+	AddOperationParameter(*operation, "token", "Authentication token to be checked", "", true);
 
-	CreateOperation(*operation_info, OPERATION_INFO, "Get operation meta data").release();
-
-	result.set_status(true);
-
-	return operation_info;
+	CreateOperation(operation_info, OPERATION_INFO, "Get operation meta data");
 }
 
-unique_ptr<PbOperationMetaData> PiscsiResponse::CreateOperation(PbOperationInfo& operation_info, const PbOperation& operation,
+// This method returns a raw pointer because protobuf does not have support for smart pointers
+PbOperationMetaData *PiscsiResponse::CreateOperation(PbOperationInfo& operation_info, const PbOperation& operation,
 		const string& description) const
 {
-	auto meta_data = make_unique<PbOperationMetaData>();
-	meta_data->set_server_side_name(PbOperation_Name(operation));
-	meta_data->set_description(description);
+	PbOperationMetaData meta_data;
+	meta_data.set_server_side_name(PbOperation_Name(operation));
+	meta_data.set_description(description);
 	int ordinal = PbOperation_descriptor()->FindValueByName(PbOperation_Name(operation))->index();
-	(*operation_info.mutable_operations())[ordinal] = *meta_data.release();
-	return unique_ptr<PbOperationMetaData>(&(*operation_info.mutable_operations())[ordinal]);
+	(*operation_info.mutable_operations())[ordinal] = meta_data;
+	return &(*operation_info.mutable_operations())[ordinal];
 }
 
-unique_ptr<PbOperationParameter> PiscsiResponse::AddOperationParameter(PbOperationMetaData& meta_data,
-		const string& name, const string& description, const string& default_value, bool is_mandatory) const
+void PiscsiResponse::AddOperationParameter(PbOperationMetaData& meta_data, const string& name,
+		const string& description, const string& default_value, bool is_mandatory,
+		const vector<string>& permitted_values) const
 {
-	auto parameter = unique_ptr<PbOperationParameter>(meta_data.add_parameters());
+	auto parameter = meta_data.add_parameters();
 	parameter->set_name(name);
 	parameter->set_description(description);
 	parameter->set_default_value(default_value);
 	parameter->set_is_mandatory(is_mandatory);
-
-	return parameter;
+	for (const auto& permitted_value : permitted_values) {
+		parameter->add_permitted_values(permitted_value);
+	}
 }
 
 set<id_set> PiscsiResponse::MatchDevices(const unordered_set<shared_ptr<PrimaryDevice>>& devices, PbResult& result,
@@ -503,7 +408,7 @@ set<id_set> PiscsiResponse::MatchDevices(const unordered_set<shared_ptr<PrimaryD
 		bool has_device = false;
 		for (const auto& d : devices) {
 			if (d->GetId() == device.id() && d->GetLun() == device.unit()) {
-				id_sets.insert(make_pair(device.id(), device.unit()));
+				id_sets.insert({ device.id(), device.unit() });
 				has_device = true;
 				break;
 			}
@@ -513,7 +418,7 @@ set<id_set> PiscsiResponse::MatchDevices(const unordered_set<shared_ptr<PrimaryD
 			id_sets.clear();
 
 			result.set_status(false);
-			result.set_msg("No device for ID " + to_string(device.id()) + ", unit " + to_string(device.unit()));
+			result.set_msg("No device for " + to_string(device.id()) + ":" + to_string(device.unit()));
 
 			break;
 		}
@@ -522,30 +427,45 @@ set<id_set> PiscsiResponse::MatchDevices(const unordered_set<shared_ptr<PrimaryD
 	return id_sets;
 }
 
-string PiscsiResponse::GetNextImageFile(const dirent *dir, const string& folder)
+bool PiscsiResponse::ValidateImageFile(const path& path)
 {
-	// Ignore unknown folder types and folder names starting with '.'
-	if ((dir->d_type != DT_REG && dir->d_type != DT_DIR && dir->d_type != DT_LNK && dir->d_type != DT_BLK)
-			|| dir->d_name[0] == '.') {
-		return "";
+	if (path.filename().string().starts_with(".")) {
+		return false;
 	}
 
-	const string filename = folder + "/" + dir->d_name;
+	filesystem::path p(path);
 
-	const bool file_exists = exists(path(filename));
-
-	// filesystem::file_size cannot be used here because gcc < 10.3.0 cannot handle files of more than 2 GiB
-	struct stat st;
-	stat(filename.c_str(), &st);
-	if (dir->d_type == DT_REG && file_exists && !st.st_size) {
-		LOGWARN("File '%s' in image folder '%s' is empty", dir->d_name, folder.c_str())
-		return "";
+	// Follow symlink
+	if (is_symlink(p)) {
+		p = read_symlink(p);
+		if (!exists(p)) {
+			spdlog::warn("Image file symlink '" + path.string() + "' is broken");
+			return false;
+		}
 	}
 
-	if (dir->d_type == DT_LNK && !file_exists) {
-		LOGWARN("Symlink '%s' in image folder '%s' is broken", dir->d_name, folder.c_str())
-		return "";
+	if (is_directory(p) || (is_other(p) && !is_block_file(p))) {
+		return false;
 	}
 
-	return filename;
+	if (!is_block_file(p) && file_size(p) < 256) {
+		spdlog::warn("Image file '" + p.string() + "' is invalid");
+		return false;
+	}
+
+	return true;
+}
+
+bool PiscsiResponse::FilterMatches(const string& input, string_view pattern_lower)
+{
+	if (!pattern_lower.empty()) {
+		string name_lower;
+		ranges::transform(input, back_inserter(name_lower), ::tolower);
+
+		if (name_lower.find(pattern_lower) == string::npos) {
+			return false;
+		}
+	}
+
+	return true;
 }

cpp/piscsi/piscsi_response.h

@@ -3,7 +3,7 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2021-2022 Uwe Seimet
+// Copyright (C) 2021-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -12,52 +12,51 @@
 #include "devices/device_factory.h"
 #include "devices/primary_device.h"
 #include "generated/piscsi_interface.pb.h"
-#include <dirent.h>
-#include <array>
 #include <string>
+#include <span>
 
 using namespace std;
+using namespace filesystem;
 using namespace piscsi_interface;
 
 class PiscsiResponse
 {
-	using id_set = pair<int, int>;
-
 public:
 
 	PiscsiResponse() = default;
 	~PiscsiResponse() = default;
 
 	bool GetImageFile(PbImageFile&, const string&, const string&) const;
-	unique_ptr<PbImageFilesInfo> GetAvailableImages(PbResult&, const string&, const string&, const string&, int) const;
-	unique_ptr<PbReservedIdsInfo> GetReservedIds(PbResult&, const unordered_set<int>&) const;
+	void GetImageFilesInfo(PbImageFilesInfo&, const string&, const string&, const string&, int) const;
+	void GetReservedIds(PbReservedIdsInfo&, const unordered_set<int>&) const;
 	void GetDevices(const unordered_set<shared_ptr<PrimaryDevice>>&, PbServerInfo&, const string&) const;
 	void GetDevicesInfo(const unordered_set<shared_ptr<PrimaryDevice>>&, PbResult&, const PbCommand&, const string&) const;
-	unique_ptr<PbDeviceTypesInfo> GetDeviceTypesInfo(PbResult&) const;
-	unique_ptr<PbVersionInfo> GetVersionInfo(PbResult&) const;
-	unique_ptr<PbServerInfo> GetServerInfo(const unordered_set<shared_ptr<PrimaryDevice>>&, PbResult&, const unordered_set<int>&,
-			const string&, const string&, const string&, const string&, int) const;
-	unique_ptr<PbNetworkInterfacesInfo> GetNetworkInterfacesInfo(PbResult&) const;
-	unique_ptr<PbMappingInfo> GetMappingInfo(PbResult&) const;
-	unique_ptr<PbLogLevelInfo> GetLogLevelInfo(PbResult&, const string&) const;
-	unique_ptr<PbOperationInfo> GetOperationInfo(PbResult&, int) const;
+	void GetDeviceTypesInfo(PbDeviceTypesInfo&) const;
+	void GetVersionInfo(PbVersionInfo&) const;
+	void GetServerInfo(PbServerInfo&, const unordered_set<shared_ptr<PrimaryDevice>>&, const unordered_set<int>&,
+			const string&, const string&, const string&, int) const;
+	void GetNetworkInterfacesInfo(PbNetworkInterfacesInfo&) const;
+	void GetMappingInfo(PbMappingInfo&) const;
+	void GetLogLevelInfo(PbLogLevelInfo&) const;
+	void GetOperationInfo(PbOperationInfo&, int) const;
 
 private:
 
-	DeviceFactory device_factory;
+	inline static const vector<string> EMPTY_VECTOR;
 
-	const inline static array<string, 6> log_levels = { "trace", "debug", "info", "warn", "err", "off" };
+	const DeviceFactory device_factory;
 
-	unique_ptr<PbDeviceProperties> GetDeviceProperties(const Device&) const;
+	void GetDeviceProperties(const Device&, PbDeviceProperties&) const;
 	void GetDevice(const Device&, PbDevice&, const string&) const;
-	void GetAllDeviceTypeProperties(PbDeviceTypesInfo&) const;
 	void GetDeviceTypeProperties(PbDeviceTypesInfo&, PbDeviceType) const;
-	void GetAvailableImages(PbImageFilesInfo&, const string&, const string&, const string&, const string&, int) const;
-	void GetAvailableImages(PbResult& result, PbServerInfo&, const string&, const string&, const string&, int) const;
-	unique_ptr<PbOperationMetaData> CreateOperation(PbOperationInfo&, const PbOperation&, const string&) const;
-	unique_ptr<PbOperationParameter> AddOperationParameter(PbOperationMetaData&, const string&, const string&,
-			const string& = "", bool = false) const;
+	void GetAvailableImages(PbImageFilesInfo&, const string&, const string&, const string&, int) const;
+	void GetAvailableImages(PbServerInfo&, const string&, const string&, const string&, int) const;
+	PbOperationMetaData *CreateOperation(PbOperationInfo&, const PbOperation&, const string&) const;
+	void AddOperationParameter(PbOperationMetaData&, const string&, const string&,
+			const string& = "", bool = false, const vector<string>& = EMPTY_VECTOR) const;
 	set<id_set> MatchDevices(const unordered_set<shared_ptr<PrimaryDevice>>&, PbResult&, const PbCommand&) const;
 
-	static string GetNextImageFile(const dirent *, const string&);
+	static bool ValidateImageFile(const path&);
+
+	static bool FilterMatches(const string&, string_view);
 };

cpp/piscsi/piscsi_service.cpp

@@ -3,140 +3,114 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2022 Uwe Seimet
+// Copyright (C) 2022-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
-#include "shared/log.h"
 #include "shared/piscsi_util.h"
-#include "shared/protobuf_serializer.h"
 #include "shared/piscsi_exceptions.h"
 #include "command_context.h"
-#include "localizer.h"
 #include "piscsi_service.h"
+#include <spdlog/spdlog.h>
+#include <sys/socket.h>
 #include <netinet/in.h>
+#include <arpa/inet.h>
 #include <csignal>
+#include <cassert>
 
-using namespace piscsi_interface;
 using namespace piscsi_util;
 
-void PiscsiService::Cleanup() const
+string PiscsiService::Init(const callback& cb, int port)
 {
-	running = false;
+	assert(service_socket == -1);
 
-	if (service_socket != -1) {
-		close(service_socket);
-	}
-}
-
-bool PiscsiService::Init(const callback& cb, int port)
-{
 	if (port <= 0 || port > 65535) {
-		return false;
+		return "Invalid port number " + to_string(port);
 	}
 
-	// Create socket for monitor
-	sockaddr_in server = {};
 	service_socket = socket(PF_INET, SOCK_STREAM, 0);
 	if (service_socket == -1) {
-		LOGERROR("Unable to create socket")
-		return false;
+		return "Unable to create service socket: " + string(strerror(errno));
 	}
 
+	if (const int yes = 1; setsockopt(service_socket, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes)) < 0) {
+		Stop();
+		return "Can't reuse address";
+	}
+
+	sockaddr_in server = {};
 	server.sin_family = PF_INET;
 	server.sin_port = htons((uint16_t)port);
-	server.sin_addr.s_addr = htonl(INADDR_ANY);
-
-	// Allow address reuse
-	if (int yes = 1; setsockopt(service_socket, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes)) < 0) {
-		return false;
+	server.sin_addr.s_addr = INADDR_ANY;
+	if (bind(service_socket, reinterpret_cast<const sockaddr *>(&server), sizeof(sockaddr_in)) < 0) { //NOSONAR bit_cast is not supported by the bullseye compiler
+		Stop();
+		return "Port " + to_string(port) + " is in use, is piscsi already running?";
 	}
 
-	signal(SIGPIPE, SIG_IGN);
-
-	if (bind(service_socket, (sockaddr *)&server, sizeof(sockaddr_in)) < 0) {
-		cerr << "Error: Port " << port << " is in use, is piscsi or rascsi already running?" << endl;
-		return false;
+	if (listen(service_socket, 2) == -1) {
+		Stop();
+		return "Can't listen to service socket: " + string(strerror(errno));
 	}
 
 	execute = cb;
 
-	monthread = thread(&PiscsiService::Execute, this);
-	monthread.detach();
+	return "";
+}
 
-	// Interrupt handler settings
-	return signal(SIGINT, KillHandler) != SIG_ERR && signal(SIGHUP, KillHandler) != SIG_ERR
-			&& signal(SIGTERM, KillHandler) != SIG_ERR;
+void PiscsiService::Start()
+{
+	assert(service_socket != -1);
+
+	service_thread = jthread([this] () { Execute(); } );
+}
+
+void PiscsiService::Stop()
+{
+	assert(service_socket != -1);
+
+	shutdown(service_socket, SHUT_RD);
+	close(service_socket);
+
+	service_socket = -1;
 }
 
 void PiscsiService::Execute() const
 {
 #ifdef __linux__
-    // Scheduler Settings
-	sched_param schedparam;
-	schedparam.sched_priority = 0;
+	// Run this thread with very low priority
+	sched_param schedparam = { .sched_priority = 0 };
 	sched_setscheduler(0, SCHED_IDLE, &schedparam);
 #endif
 
-	// Set the affinity to a specific processor core
-	FixCpu(2);
-
-	// Wait for the execution to start
-	const timespec ts = { .tv_sec = 0, .tv_nsec = 1000};
-	while (!running) {
-		nanosleep(&ts, nullptr);
-	}
-
-	// Set up the monitor socket to receive commands
-	listen(service_socket, 1);
-
-	while (true) {
-		CommandContext context;
-
-		try {
-			PbCommand command = ReadCommand(context);
-			if (context.IsValid()) {
-				execute(context, command);
-			}
+	// TODO Accept more than one command instead of closing the socket after a single command
+	while (service_socket != -1) {
+		const int fd = accept(service_socket, nullptr, nullptr);
+		if (fd != -1) {
+			ExecuteCommand(fd);
+			close(fd);
 		}
-		catch(const io_exception& e) {
-			LOGWARN("%s", e.what())
-
-            // Fall through
-		}
-
-		context.Cleanup();
 	}
 }
 
-PbCommand PiscsiService::ReadCommand(CommandContext& context) const
+void PiscsiService::ExecuteCommand(int fd) const
 {
-	// Wait for connection
-	sockaddr client = {};
-	socklen_t socklen = sizeof(client);
-	const int fd = accept(service_socket, &client, &socklen);
-	if (fd == -1) {
-		throw io_exception("accept() failed");
+	CommandContext context(fd);
+	try {
+		if (context.ReadCommand()) {
+			execute(context);
+		}
 	}
+	catch(const io_exception& e) {
+		spdlog::warn(e.what());
 
-	PbCommand command;
-
-	// Read magic string
-	vector<byte> magic(6);
-	const size_t bytes_read = context.GetSerializer().ReadBytes(fd, magic);
-	if (!bytes_read) {
-		return command;
+		// Try to return an error message (this may fail if the exception was caused when returning the actual result)
+		PbResult result;
+		result.set_msg(e.what());
+		try {
+			context.WriteResult(result);
+		}
+		catch(const io_exception&) { //NOSONAR Not handled on purpose
+			// Ignore
+		}
 	}
-
-	if (bytes_read != magic.size() || memcmp(magic.data(), "RASCSI", magic.size())) {
-		throw io_exception("Invalid magic");
-	}
-
-	// Fetch the command
-	context.GetSerializer().DeserializeMessage(fd, command);
-
-	context.SetFd(fd);
-
-	return command;
 }
-

cpp/piscsi/piscsi_service.h

@@ -3,49 +3,42 @@
 // SCSI Target Emulator PiSCSI
 // for Raspberry Pi
 //
-// Copyright (C) 2022 Uwe Seimet
+// Copyright (C) 2022-2023 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
 #pragma once
 
-#include "generated/piscsi_interface.pb.h"
 #include <functional>
 #include <thread>
+#include <string>
 
 class CommandContext;
 
 using namespace std;
-using namespace piscsi_interface;
 
 class PiscsiService
 {
-	using callback = function<bool(const CommandContext&, piscsi_interface::PbCommand&)>;
-
-	callback execute;
-
-	int service_socket = -1;
-
-	thread monthread;
-
-	static inline volatile bool running = false;
+	using callback = function<bool(CommandContext&)>;
 
 public:
 
 	PiscsiService() = default;
 	~PiscsiService() = default;
 
-	bool Init(const callback&, int);
-	void Cleanup() const;
-
-	bool IsRunning() const { return running; }
-	void SetRunning(bool b) const { running = b; }
+	string Init(const callback&, int);
+	void Start();
+	void Stop();
+	bool IsRunning() const { return service_socket != -1 && service_thread.joinable(); }
 
 private:
 
 	void Execute() const;
+	void ExecuteCommand(int) const;
 
-	PbCommand ReadCommand(CommandContext&) const;
+	callback execute;
 
-	static void KillHandler(int) { running = false; }
+	jthread service_thread;
+
+	int service_socket = -1;
 };