Fix lvm volumes appearing as removable devices

- remove obsolete/erroneous code meddling with
  GENHD_FL_MEDIA_CHANGE_NOTIFY
- use /sys/block/<dev>/removable rather than
  /sys/block/<dev>/capabilities
- deprecate TDEDiskDeviceStatus::Hotpluggable and use
  TDEDiskDeviceStatus::Removable for all removable devices
- put some local utility function into anonymous namespace

See https://mirror.git.trinitydesktop.org/gitea/TDE/tde/issues/148

Signed-off-by: Alexander Golubev <fatzer2@gmail.com>

3 files changed, 26 insertions, 56 deletions

diff --git a/tdecore/tdehw/tdehardwaredevices.cpp b/tdecore/tdehw/tdehardwaredevices.cpp
index 70a7b7c45..39f57451b 100644
--- a/tdecore/tdehw/tdehardwaredevices.cpp
+++ b/tdecore/tdehw/tdehardwaredevices.cpp
@@ -89,22 +89,13 @@ timespec diff(timespec start, timespec end)
 	return temp;
 }
 
-// BEGIN BLOCK
-// Copied from include/linux/genhd.h
-#define GENHD_FL_REMOVABLE                      1
-#define GENHD_FL_MEDIA_CHANGE_NOTIFY            4
-#define GENHD_FL_CD                             8
-#define GENHD_FL_UP                             16
-#define GENHD_FL_SUPPRESS_PARTITION_INFO        32
-#define GENHD_FL_EXT_DEVT                       64
-#define GENHD_FL_NATIVE_CAPACITY                128
-#define GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE     256
-// END BLOCK
-
 // NOTE TO DEVELOPERS
 // This command will greatly help when attempting to find properties to distinguish one device from another
 // udevadm info --query=all --path=/sys/....
 
+// Some local utility functions and constants
+namespace {
+
 // This routine is courtsey of an answer on "Stack Overflow"
 // It takes an LSB-first int and makes it an MSB-first int (or vice versa)
 unsigned int reverse_bits(unsigned int x)
@@ -116,6 +107,19 @@ unsigned int reverse_bits(unsigned int x)
 	return((x >> 16) | (x << 16));
 }
 
+// Read the content of a file that supposed to contain a single line
+TQString readLineFile(TQString fname) {
+	TQFile file( fname );
+	if ( file.open( IO_ReadOnly ) ) {
+		TQTextStream stream( &file );
+		return stream.readLine();
+	} else {
+		return TQString::null;
+	}
+}
+
+} // namespace
+
 // Helper function implemented in tdestoragedevice.cpp
 TQString decodeHexEncoding(TQString str);
 
@@ -2511,36 +2515,6 @@ void TDEHardwareDevices::updateExistingDeviceInformation(TDEGenericDevice *devic
 			sdevice->internalSetFileSystemName("pictbridge");
 		}
 		else {
-			bool removable = false;
-			bool hotpluggable = false;
-
-			// We can get the removable flag, but we have no idea if the device has the ability to notify on media insertion/removal
-			// If there is no such notification possible, then we should not set the removable flag
-			// udev can be such an amazing pain at times
-			// It exports a /capabilities node with no info on what the bits actually mean
-			// This information is very poorly documented as a set of #defines in include/linux/genhd.h
-			// We are specifically interested in GENHD_FL_REMOVABLE and GENHD_FL_MEDIA_CHANGE_NOTIFY
-			// The "removable" flag should also really be renamed to "hotpluggable", as that is far more precise...
-			TQString capabilitynodename = systempath;
-			capabilitynodename.append("/capability");
-			TQFile capabilityfile( capabilitynodename );
-			unsigned int capabilities = 0;
-			if ( capabilityfile.open( IO_ReadOnly ) ) {
-				TQTextStream stream( &capabilityfile );
-				TQString capabilitystring;
-				capabilitystring = stream.readLine();
-				capabilities = capabilitystring.toUInt();
-				capabilityfile.close();
-			}
-			if (capabilities & GENHD_FL_REMOVABLE) {
-				// FIXME
-				// For added fun this is not always true; i.e. GENHD_FL_REMOVABLE can be set when the device cannot be hotplugged (floppy drives).
-				hotpluggable = true;
-			}
-			if (capabilities & GENHD_FL_MEDIA_CHANGE_NOTIFY) {
-				removable = true;
-			}
-
 			// See if any other devices are exclusively using this device, such as the Device Mapper
 			TQStringList holdingDeviceNodes;
 			TQString holdersnodename = udev_device_get_syspath(dev);
@@ -2733,21 +2707,16 @@ void TDEHardwareDevices::updateExistingDeviceInformation(TDEGenericDevice *devic
 				}
 			}
 
-			if (removable) {
+			if (readLineFile( systempath + "/removable" ).toUInt()) {
 				diskstatus = diskstatus | TDEDiskDeviceStatus::Removable;
 			}
-			if (hotpluggable) {
-				diskstatus = diskstatus | TDEDiskDeviceStatus::Hotpluggable;
-			}
 			// Force removable flag for flash disks
 			// udev reports disks as non-removable for card readers on PCI controllers
-			if (((disktype & TDEDiskDeviceType::CompactFlash)
+			else if ((disktype & TDEDiskDeviceType::CompactFlash)
 			     || (disktype & TDEDiskDeviceType::MemoryStick)
 			     || (disktype & TDEDiskDeviceType::SmartMedia)
-			     || (disktype & TDEDiskDeviceType::SDMMC))
-			    && !(diskstatus & TDEDiskDeviceStatus::Removable)
-			    && !(diskstatus & TDEDiskDeviceStatus::Hotpluggable)) {
-				diskstatus = diskstatus | TDEDiskDeviceStatus::Hotpluggable;
+			     || (disktype & TDEDiskDeviceType::SDMMC)) {
+				diskstatus = diskstatus | TDEDiskDeviceStatus::Removable;
 			}
 
 			if ((!filesystemtype.isEmpty()) && (filesystemtype.upper() != "CRYPTO_LUKS") &&
@@ -2765,7 +2734,7 @@ void TDEHardwareDevices::updateExistingDeviceInformation(TDEGenericDevice *devic
 				// type string too. For example for LUKS disk, ID_TYPE is null and DEVTYPE is "disk"
 				diskstatus = diskstatus & ~TDEDiskDeviceStatus::Mountable;
 			}
-			if (removable) {
+			if ( diskstatus & TDEDiskDeviceStatus::Removable ) {
 				if (sdevice->mediaInserted()) {
 					diskstatus = diskstatus | TDEDiskDeviceStatus::Inserted;
 				}
diff --git a/tdecore/tdehw/tdestoragedevice.cpp b/tdecore/tdehw/tdestoragedevice.cpp
index 08fb246e1..df5f90ab7 100644
--- a/tdecore/tdehw/tdestoragedevice.cpp
+++ b/tdecore/tdehw/tdestoragedevice.cpp
@@ -504,7 +504,7 @@ TQString TDEStorageDevice::friendlyName() {
 	TQString label = diskLabel();
 	if (label.isNull()) {
 		if (deviceSize() > 0) {
-			if (checkDiskStatus(TDEDiskDeviceStatus::Hotpluggable)) {
+			if (checkDiskStatus(TDEDiskDeviceStatus::Removable)) {
 				label = i18n("%1 Removable Device").arg(deviceFriendlySize());
 			}
 			else {
@@ -561,7 +561,7 @@ TQString TDEStorageDevice::friendlyDeviceType() {
 
 	if (isDiskOfType(TDEDiskDeviceType::HDD)) {
 		ret = i18n("Hard Disk Drive");
-		if (checkDiskStatus(TDEDiskDeviceStatus::Hotpluggable)) {
+		if (checkDiskStatus(TDEDiskDeviceStatus::Removable)) {
 			ret = i18n("Removable Storage");
 		}
 		if (isDiskOfType(TDEDiskDeviceType::CompactFlash)) {
@@ -632,7 +632,7 @@ TQPixmap TDEStorageDevice::icon(TDEIcon::StdSizes size) {
 
 	if (isDiskOfType(TDEDiskDeviceType::HDD)) {
 		ret = DesktopIcon("drive-harddisk" + mountString, size);
-		if (checkDiskStatus(TDEDiskDeviceStatus::Hotpluggable)) {
+		if (checkDiskStatus(TDEDiskDeviceStatus::Removable)) {
 			ret = DesktopIcon("media-flash-usb" + mountString, size);
 		}
 		if (isDiskOfType(TDEDiskDeviceType::CompactFlash)) {
diff --git a/tdecore/tdehw/tdestoragedevice.h b/tdecore/tdehw/tdestoragedevice.h
index c279586f8..a16bf978b 100644
--- a/tdecore/tdehw/tdestoragedevice.h
+++ b/tdecore/tdehw/tdestoragedevice.h
@@ -22,6 +22,7 @@
 
 #include "tdegenericdevice.h"
 #include "tqvariant.h"
+#include "kdemacros.h"
 
 struct crypt_device;
 
@@ -107,7 +108,7 @@ enum TDEDiskDeviceStatus {
 	UsedByDevice       = 0x00000010,
 	UsesDevice         = 0x00000020,
 	ContainsFilesystem = 0x00000040,
-	Hotpluggable       = 0x00000080,
+	Hotpluggable KDE_DEPRECATED = 0x00000080 , //< @deprecated; use Removable instead
 	Hidden             = 0x00000100,
 	Other              = 0x80000000
 };