Fix failure to unmount drives when one of their partitions is mounted (#2927).

3 files changed, 75 insertions, 17 deletions

diff --git a/src/lib/cross.h b/src/lib/cross.h
index 551895754..bb1cc91db 100644
--- a/src/lib/cross.h
+++ b/src/lib/cross.h
@@ -93,9 +93,19 @@ private:
 #endif
 };
 
+
 class Drive
 {
 public:
+#ifdef DCPOMATIC_OSX
+	Drive(std::string device, bool mounted, uint64_t size, boost::optional<std::string> vendor, boost::optional<std::string> model)
+		: _device(device)
+		, _mounted(mounted)
+		, _size(size)
+		, _vendor(vendor)
+		, _model(model)
+	{}
+#else
 	Drive (std::string device, std::vector<boost::filesystem::path> mount_points, uint64_t size, boost::optional<std::string> vendor, boost::optional<std::string> model)
 		: _device(device)
 		, _mount_points(mount_points)
@@ -103,6 +113,7 @@ public:
 		, _vendor(vendor)
 		, _model(model)
 	{}
+#endif
 
 	explicit Drive (std::string);
 
@@ -115,7 +126,11 @@ public:
 	}
 
 	bool mounted () const {
+#ifdef DCPOMATIC_OSX
+		return _mounted;
+#else
 		return !_mount_points.empty();
+#endif
 	}
 
 	std::string log_summary () const;
@@ -125,23 +140,28 @@ public:
 	 */
 	bool unmount ();
 
+#ifdef DCPOMATIC_OSX
+	void set_mounted() {
+		_mounted = true;
+	}
+#endif
+
 	static std::vector<Drive> get ();
 
 private:
 	std::string _device;
-	/** Descriptions of how this drive is mounted.  This is interpreted differently
-	 *  on different platforms.
-	 *
-	 *  On macOS it's a list of device nodes e.g. /dev/disk8, /dev/disk8s2, /dev/disk7s5 or
-	 *  filesystem mount points (the contents are not important, just if any exist).
-	 */
+#ifdef DCPOMATIC_OSX
+	bool _mounted;
+#else
 	std::vector<boost::filesystem::path> _mount_points;
+#endif
 	/** size in bytes */
 	uint64_t _size;
 	boost::optional<std::string> _vendor;
 	boost::optional<std::string> _model;
 };
 
+
 void disk_write_finished ();
 
 
diff --git a/src/lib/cross_common.cc b/src/lib/cross_common.cc
index 2e6f5c414..b6df88837 100644
--- a/src/lib/cross_common.cc
+++ b/src/lib/cross_common.cc
@@ -45,9 +45,13 @@ Drive::Drive (string xml)
 	cxml::Document doc;
 	doc.read_string (xml);
 	_device = doc.string_child("Device");
+#ifdef DCPOMATIC_OSX
+	_mounted = doc.bool_child("Mounted");
+#else
 	for (auto i: doc.node_children("MountPoint")) {
 		_mount_points.push_back (i->content());
 	}
+#endif
 	_size = doc.number_child<uint64_t>("Size");
 	_vendor = doc.optional_string_child("Vendor");
 	_model = doc.optional_string_child("Model");
@@ -60,9 +64,13 @@ Drive::as_xml () const
 	xmlpp::Document doc;
 	auto root = doc.create_root_node ("Drive");
 	cxml::add_text_child(root, "Device", _device);
+#ifdef DCPOMATIC_OSX
+	cxml::add_text_child(root, "Mounted", _mounted ? "1" : "0");
+#else
 	for (auto i: _mount_points) {
 		cxml::add_text_child(root, "MountPoint", i.string());
 	}
+#endif
 	cxml::add_text_child(root, "Size", fmt::to_string(_size));
 	if (_vendor) {
 		cxml::add_text_child(root, "Vendor", *_vendor);
@@ -86,13 +94,13 @@ Drive::description () const
 		name += *_vendor;
 	}
 	if (_model) {
-		if (name.size() > 0) {
+		if (!name.empty()) {
 			name += " " + *_model;
 		} else {
 			name = *_model;
 		}
 	}
-	if (name.size() == 0) {
+	if (name.empty()) {
 		name = _("Unknown");
 	}
 
@@ -104,14 +112,23 @@ string
 Drive::log_summary () const
 {
 	string mp;
+#ifdef DCPOMATIC_OSX
+	if (_mounted) {
+		mp += "mounted ";
+	} else {
+		mp += "not mounted ";
+	}
+#else
+	mp = "mounted on ";
 	for (auto i: _mount_points) {
 		mp += i.string() + ",";
 	}
-	if (mp.empty()) {
+	if (_mount_points.empty()) {
 		mp = "[none]";
 	} else {
 		mp = mp.substr (0, mp.length() - 1);
 	}
+#endif
 
 	return String::compose(
 		"Device %1 mounted on %2 size %3 vendor %4 model %5",
diff --git a/src/lib/cross_osx.cc b/src/lib/cross_osx.cc
index a8ac44240..8d15e46b5 100644
--- a/src/lib/cross_osx.cc
+++ b/src/lib/cross_osx.cc
@@ -210,7 +210,7 @@ struct OSXDisk
 	std::string device;
 	boost::optional<std::string> vendor;
 	boost::optional<std::string> model;
-	std::vector<boost::filesystem::path> mount_points;
+	bool mounted;
 	unsigned long size;
 	bool system;
 	bool writeable;
@@ -306,10 +306,7 @@ disk_appeared (DADiskRef disk, void* context)
 	this_disk.model = get_model (description);
 	LOG_DISK("Vendor/model: %1 %2", this_disk.vendor.get_value_or("[none]"), this_disk.model.get_value_or("[none]"));
 
-	auto mp = mount_point (description);
-	if (mp) {
-		this_disk.mount_points.push_back (*mp);
-	}
+	this_disk.mounted = static_cast<bool>(mount_point(description));
 
 	auto media_size_cstr = CFDictionaryGetValue (description, kDADiskDescriptionMediaSizeKey);
 	if (!media_size_cstr) {
@@ -322,12 +319,12 @@ disk_appeared (DADiskRef disk, void* context)
 	this_disk.partition = string(this_disk.bsd_name).find("s", 5) != std::string::npos;
 
 	LOG_DISK(
-		"%1 %2 %3 %4 mounted at %5",
+		"%1 %2 %3 %4 %5",
 		this_disk.bsd_name,
 		this_disk.system ? "system" : "non-system",
 		this_disk.writeable ? "writeable" : "read-only",
 		this_disk.partition ? "partition" : "drive",
-		mp ? mp->string() : "[nowhere]"
+		this_disk.mounted ? "mounted" : "not mounted"
 		);
 
 	CFNumberGetValue ((CFNumberRef) media_size_cstr, kCFNumberLongType, &this_disk.size);
@@ -360,10 +357,34 @@ Drive::get ()
 	DAUnregisterCallback(session, (void *) disk_appeared, &disks);
 	CFRelease(session);
 
+	/* Find all the drives (not partitions) - these OSXDisks can be either */
 	vector<Drive> drives;
 	for (auto const& disk: disks) {
 		if (!disk.system && !disk.partition && disk.writeable) {
-			drives.push_back({disk.device, disk.mount_points, disk.size, disk.vendor, disk.model});
+			LOG_DISK("Have a non-system writeable drive: %1", disk.device);
+			drives.push_back({disk.device, disk.mounted, disk.size, disk.vendor, disk.model});
+		}
+	}
+
+	/* Find mounted partitions and mark their drives mounted */
+	for (auto const& disk: disks) {
+		if (!disk.system && disk.partition && disk.mounted) {
+			LOG_DISK("Have a mounted non-system partition: %1 (%2)", disk.device, disk.bsd_name);
+			if (boost::algorithm::starts_with(disk.bsd_name, "disk")) {
+				auto const second_s = disk.bsd_name.find('s', 4);
+				if (second_s != std::string::npos) {
+					/* We have a bsd_name of the form disk...s */
+					auto const drive_device = "/dev/" + disk.bsd_name.substr(0, second_s);
+					LOG_DISK("This belongs to the drive %1", drive_device);
+					auto iter = std::find_if(drives.begin(), drives.end(), [drive_device](Drive const& drive) {
+						return drive.device() == drive_device;
+					});
+					if (iter != drives.end()) {
+						LOG_DISK("Marking %1 as mounted", drive_device);
+						iter->set_mounted();
+					}
+				}
+			}
 		}
 	}