Add statistics and make scsictl accept generic key/value parameters (#…

…1237/#1238) (#1262)

* Add statistics and make scsictl accept generic key/value parameters

cpp/devices/disk.cpp

@@ -273,6 +273,9 @@ bool Disk::Eject(bool force)
 
 		// The image file for this drive is not in use anymore
 		UnreserveFile();
+
+		sector_read_count = 0;
+		sector_write_count = 0;
 	}
 
 	return status;
@@ -506,6 +509,8 @@ int Disk::Read(span<uint8_t> buf, uint64_t block)
 		throw scsi_exception(sense_key::medium_error, asc::read_fault);
 	}
 
+	++sector_read_count;
+
 	return GetSectorSizeInBytes();
 }
 
@@ -518,6 +523,8 @@ void Disk::Write(span<const uint8_t> buf, uint64_t block)
 	if (!cache->WriteSector(buf, static_cast<uint32_t>(block))) {
 		throw scsi_exception(sense_key::medium_error, asc::write_fault);
 	}
+
+	++sector_write_count;
 }
 
 void Disk::Seek()
@@ -711,3 +718,35 @@ bool Disk::SetConfiguredSectorSize(const DeviceFactory& device_factory, uint32_t
 
 	return true;
 }
+
+vector<PbStatistics> Disk::GetStatistics() const
+{
+	vector<PbStatistics> statistics = PrimaryDevice::GetStatistics();
+
+	// Enrich cache statistics with device information before adding them to device statistics
+	if (cache) {
+		for (auto& s : cache->GetStatistics(IsReadOnly())) {
+			s.set_id(GetId());
+			s.set_unit(GetLun());
+			statistics.push_back(s);
+		}
+	}
+
+	PbStatistics s;
+	s.set_id(GetId());
+	s.set_unit(GetLun());
+
+	s.set_category(PbStatisticsCategory::CATEGORY_INFO);
+
+	s.set_key(SECTOR_READ_COUNT);
+	s.set_value(sector_read_count);
+	statistics.push_back(s);
+
+	if (!IsReadOnly()) {
+		s.set_key(SECTOR_WRITE_COUNT);
+		s.set_value(sector_write_count);
+		statistics.push_back(s);
+	}
+
+	return statistics;
+}

cpp/devices/disk.h

@@ -8,14 +8,14 @@
 //	XM6i
 //	Copyright (C) 2010-2015 [email protected]
 //
-//  	Imported sava's Anex86/T98Next image and MO format support patch.
 //  	Comments translated to english by akuker.
 //
 //---------------------------------------------------------------------------
 
 #pragma once
 
 #include "shared/scsi.h"
+#include "shared/piscsi_util.h"
 #include "device_factory.h"
 #include "disk_track.h"
 #include "disk_cache.h"
@@ -42,6 +42,12 @@ class Disk : public StorageDevice, private ScsiBlockCommands
 	// Sector size shift count (9=512, 10=1024, 11=2048, 12=4096)
 	uint32_t size_shift_count = 0;
 
+	uint64_t sector_read_count = 0;
+	uint64_t sector_write_count = 0;
+
+	inline static const string SECTOR_READ_COUNT = "sector_read_count";
+	inline static const string SECTOR_WRITE_COUNT = "sector_write_count";
+
 public:
 
 	using StorageDevice::StorageDevice;
@@ -62,6 +68,8 @@ class Disk : public StorageDevice, private ScsiBlockCommands
 	bool SetConfiguredSectorSize(const DeviceFactory&, uint32_t);
 	void FlushCache() override;
 
+	vector<PbStatistics> GetStatistics() const override;
+
 private:
 
 	// Commands covered by the SCSI specifications (see https://www.t10.org/drafts.htm)

cpp/devices/disk_cache.cpp

@@ -27,11 +27,11 @@ DiskCache::DiskCache(const string& path, int size, uint32_t blocks, off_t imgoff
 	assert(imgoff >= 0);
 }
 
-bool DiskCache::Save() const
+bool DiskCache::Save()
 {
 	// Save valid tracks
 	return ranges::none_of(cache.begin(), cache.end(), [this](const cache_t& c)
-			{ return c.disktrk != nullptr && !c.disktrk->Save(sec_path); });
+			{ return c.disktrk != nullptr && !c.disktrk->Save(sec_path, cache_miss_write_count); });
 }
 
 shared_ptr<DiskTrack> DiskCache::GetTrack(uint32_t block)
@@ -120,7 +120,7 @@ shared_ptr<DiskTrack> DiskCache::Assign(int track)
 	}
 
 	// Save this track
-	if (!cache[c].disktrk->Save(sec_path)) {
+	if (!cache[c].disktrk->Save(sec_path, cache_miss_write_count)) {
 		return nullptr;
 	}
 
@@ -156,17 +156,16 @@ bool DiskCache::Load(int index, int track, shared_ptr<DiskTrack> disktrk)
 		sectors = 0x100;
 	}
 
-	// Create a disk track
 	if (disktrk == nullptr) {
 		disktrk = make_shared<DiskTrack>();
 	}
 
-	// Initialize disk track
 	disktrk->Init(track, sec_size, sectors, cd_raw, imgoffset);
 
 	// Try loading
-	if (!disktrk->Load(sec_path)) {
-		// Failure
+	if (!disktrk->Load(sec_path, cache_miss_read_count)) {
+		++read_error_count;
+
 		return false;
 	}
 
@@ -190,3 +189,35 @@ void DiskCache::UpdateSerialNumber()
 	}
 }
 
+vector<PbStatistics> DiskCache::GetStatistics(bool is_read_only) const
+{
+	vector<PbStatistics> statistics;
+
+	PbStatistics s;
+
+	s.set_category(PbStatisticsCategory::CATEGORY_INFO);
+
+	s.set_key(CACHE_MISS_READ_COUNT);
+	s.set_value(cache_miss_read_count);
+	statistics.push_back(s);
+
+	if (!is_read_only) {
+		s.set_key(CACHE_MISS_WRITE_COUNT);
+		s.set_value(cache_miss_write_count);
+		statistics.push_back(s);
+	}
+
+	s.set_category(PbStatisticsCategory::CATEGORY_ERROR);
+
+	s.set_key(READ_ERROR_COUNT);
+	s.set_value(read_error_count);
+	statistics.push_back(s);
+
+	if (!is_read_only) {
+		s.set_key(WRITE_ERROR_COUNT);
+		s.set_value(write_error_count);
+		statistics.push_back(s);
+	}
+
+	return statistics;
+}

cpp/devices/disk_cache.h

@@ -15,18 +15,30 @@
 
 #pragma once
 
+#include "generated/piscsi_interface.pb.h"
 #include <span>
 #include <array>
 #include <memory>
 #include <string>
 
 using namespace std;
+using namespace piscsi_interface;
 
 class DiskCache
 {
 	// Number of tracks to cache
 	static const int CACHE_MAX = 16;
 
+	uint64_t read_error_count = 0;
+	uint64_t write_error_count = 0;
+	uint64_t cache_miss_read_count = 0;
+	uint64_t cache_miss_write_count = 0;
+
+	inline static const string READ_ERROR_COUNT = "read_error_count";
+	inline static const string WRITE_ERROR_COUNT = "write_error_count";
+	inline static const string CACHE_MISS_READ_COUNT = "cache_miss_read_count";
+	inline static const string CACHE_MISS_WRITE_COUNT = "cache_miss_write_count";
+
 public:
 
 	// Internal data definition
@@ -40,11 +52,12 @@ class DiskCache
 
 	void SetRawMode(bool b) { cd_raw = b; }		// CD-ROM raw mode setting
 
-	// Access
-	bool Save() const;							// Save and release all
+	bool Save();							// Save and release all
 	bool ReadSector(span<uint8_t>, uint32_t);			// Sector Read
 	bool WriteSector(span<const uint8_t>, uint32_t);	// Sector Write
 
+	vector<PbStatistics> GetStatistics(bool) const;
+
 private:
 
 	// Internal Management

cpp/devices/disk_track.cpp

@@ -48,14 +48,16 @@ void DiskTrack::Init(int track, int size, int sectors, bool raw, off_t imgoff)
 	dt.imgoffset = imgoff;
 }
 
-bool DiskTrack::Load(const string& path)
+bool DiskTrack::Load(const string& path, uint64_t& cache_miss_read_count)
 {
 	// Not needed if already loaded
 	if (dt.init) {
 		assert(dt.buffer);
 		return true;
 	}
 
+	++cache_miss_read_count;
+
 	// Calculate offset (previous tracks are considered to hold 256 sectors)
 	off_t offset = ((off_t)dt.track << 8);
 	if (dt.raw) {
@@ -138,7 +140,7 @@ bool DiskTrack::Load(const string& path)
 	return true;
 }
 
-bool DiskTrack::Save(const string& path)
+bool DiskTrack::Save(const string& path, uint64_t& cache_miss_write_count)
 {
 	// Not needed if not initialized
 	if (!dt.init) {
@@ -150,6 +152,8 @@ bool DiskTrack::Save(const string& path)
 		return true;
 	}
 
+	++cache_miss_write_count;
+
 	// Need to write
 	assert(dt.buffer);
 	assert((dt.sectors > 0) && (dt.sectors <= 0x100));

cpp/devices/disk_track.h

@@ -50,10 +50,9 @@ class DiskTrack
 	friend class DiskCache;
 
 	void Init(int track, int size, int sectors, bool raw = false, off_t imgoff = 0);
-	bool Load(const string& path);
-	bool Save(const string& path);
+	bool Load(const string& path, uint64_t&);
+	bool Save(const string& path, uint64_t&);
 
-	// Read / Write
 	bool ReadSector(span<uint8_t>, int) const;				// Sector Read
 	bool WriteSector(span<const uint8_t> buf, int);			// Sector Write
 

cpp/devices/primary_device.h

@@ -54,7 +54,13 @@ class PrimaryDevice: private ScsiPrimaryCommands, public Device
 	void Reset() override;
 
 	virtual void FlushCache() {
-		// Devices with a cache have to implement this method
+		// Devices with a cache have to override this method
+	}
+
+	virtual vector<PbStatistics> GetStatistics() const {
+		// Devices which provide statistics have to override this method
+
+		return vector<PbStatistics>();
 	}
 
 protected:

cpp/devices/scsi_daynaport.cpp

@@ -119,7 +119,7 @@ vector<uint8_t> SCSIDaynaPort::InquiryInternal() const
 //    - The SCSI/Link apparently has about 6KB buffer space for packets.
 //
 //---------------------------------------------------------------------------
-int SCSIDaynaPort::Read(cdb_t cdb, vector<uint8_t>& buf, uint64_t) const
+int SCSIDaynaPort::Read(cdb_t cdb, vector<uint8_t>& buf, uint64_t)
 {
 	int rx_packet_size = 0;
 	const auto response = (scsi_resp_read_t*)buf.data();
@@ -155,6 +155,8 @@ int SCSIDaynaPort::Read(cdb_t cdb, vector<uint8_t>& buf, uint64_t) const
 			return DAYNAPORT_READ_HEADER_SZ;
 		}
 
+		byte_read_count += rx_packet_size * read_count;
+
 		LogTrace("Packet Size " + to_string(rx_packet_size) + ", read count: " + to_string(read_count));
 
 		// This is a very basic filter to prevent unnecessary packets from
@@ -252,17 +254,19 @@ int SCSIDaynaPort::Read(cdb_t cdb, vector<uint8_t>& buf, uint64_t) const
 //               XX XX ... is the actual packet
 //
 //---------------------------------------------------------------------------
-bool SCSIDaynaPort::Write(cdb_t cdb, span<const uint8_t> buf) const
+bool SCSIDaynaPort::Write(cdb_t cdb, span<const uint8_t> buf)
 {
 	if (const int data_format = cdb[5]; data_format == 0x00) {
 		const int data_length = GetInt16(cdb, 3);
 		tap.Send(buf.data(), data_length);
+		byte_write_count += data_length;
 		LogTrace("Transmitted " + to_string(data_length) + " byte(s) (00 format)");
 	}
 	else if (data_format == 0x80) {
 		// The data length is specified in the first 2 bytes of the payload
 		const int data_length = buf[1] + ((static_cast<int>(buf[0]) & 0xff) << 8);
 		tap.Send(&(buf.data()[4]), data_length);
+		byte_write_count += data_length;
 		LogTrace("Transmitted " + to_string(data_length) + "byte(s) (80 format)");
 	}
 	else {
@@ -305,7 +309,7 @@ void SCSIDaynaPort::TestUnitReady()
 	EnterStatusPhase();
 }
 
-void SCSIDaynaPort::Read6() const
+void SCSIDaynaPort::Read6()
 {
 	// Get record number and block number
     const uint32_t record = GetInt24(GetController()->GetCmd(), 1) & 0x1fffff;
@@ -478,3 +482,23 @@ void SCSIDaynaPort::EnableInterface() const
 	EnterStatusPhase();
 }
 
+vector<PbStatistics> SCSIDaynaPort::GetStatistics() const
+{
+	vector<PbStatistics> statistics = PrimaryDevice::GetStatistics();
+
+	PbStatistics s;
+	s.set_id(GetId());
+	s.set_unit(GetLun());
+
+	s.set_category(PbStatisticsCategory::CATEGORY_INFO);
+
+	s.set_key(BYTE_READ_COUNT);
+	s.set_value(byte_read_count);
+	statistics.push_back(s);
+
+	s.set_key(BYTE_WRITE_COUNT);
+	s.set_value(byte_write_count);
+	statistics.push_back(s);
+
+	return statistics;
+}