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

diff --git a/src/lib/cross.h b/src/lib/cross.h
index 5518957545b3450fe95d9bc555ff08608aa3b25d..bb1cc91db43d7eecbfdcf74ec7ecc2643a55b517 100644 (file)
--- 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 2e6f5c4141c1fafcd6f196760ea4554e9dcd74f2..b6df88837058c445a048150e81769ea073d7bd67 100644 (file)
--- 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 a8ac44240ef77bc7e92ce6c5f987a02ac19836be..8d15e46b58cdc5d730da3dbce0b61ce5fb2370ea 100644 (file)
--- 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();
+                                       }
+                               }
+                       }
                }
        }