From 3e58639a6887a899dc7752e36f1af87dcf2b252c Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 1 Nov 2021 16:31:07 +0100
Subject: [PATCH 01/16] [partition] Improve logging in ClearMountsJob

- mark internal bits as static
- explain what is being looked-for

SEE #1817
SEE #1564
---
 src/modules/partition/jobs/ClearMountsJob.cpp | 54 +++++++++++--------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index 825c82ec1..ea4ed0d00 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -29,28 +29,19 @@
 
 using CalamaresUtils::Partition::PartitionIterator;
 
-ClearMountsJob::ClearMountsJob( Device* device )
-    : Calamares::Job()
-    , m_device( device )
-{
-}
 
-
-QString
-ClearMountsJob::prettyName() const
-{
-    return tr( "Clear mounts for partitioning operations on %1" ).arg( m_device->deviceNode() );
-}
-
-
-QString
-ClearMountsJob::prettyStatusMessage() const
-{
-    return tr( "Clearing mounts for partitioning operations on %1." ).arg( m_device->deviceNode() );
-}
-
-
-QStringList
+/** @brief Returns list of partitions on a given @p deviceName
+ *
+ * The @p deviceName is a (whole-block) device, like "sda", and the partitions
+ * returned are then "sdaX". The whole-block device itself is ignored, if
+ * present.
+ *
+ * The format for /etc/partitions is, e.g.
+ *      major minor #blocks name
+ *      8     0     33554422 sda
+ *      8     1     33554400 sda1
+ */
+STATICTEST QStringList
 getPartitionsForDevice( const QString& deviceName )
 {
     QStringList partitions;
@@ -58,7 +49,7 @@ getPartitionsForDevice( const QString& deviceName )
     QFile dev_partitions( "/proc/partitions" );
     if ( dev_partitions.open( QFile::ReadOnly ) )
     {
-        cDebug() << "Reading from" << dev_partitions.fileName();
+        cDebug() << "Reading from" << dev_partitions.fileName() << "looking for" << deviceName;
         QTextStream in( &dev_partitions );
         (void)in.readLine();  // That's the header line, skip it
         while ( !in.atEnd() )
@@ -81,6 +72,25 @@ getPartitionsForDevice( const QString& deviceName )
     return partitions;
 }
 
+ClearMountsJob::ClearMountsJob( Device* device )
+    : Calamares::Job()
+    , m_device( device )
+{
+}
+
+QString
+ClearMountsJob::prettyName() const
+{
+    return tr( "Clear mounts for partitioning operations on %1" ).arg( m_device->deviceNode() );
+}
+
+
+QString
+ClearMountsJob::prettyStatusMessage() const
+{
+    return tr( "Clearing mounts for partitioning operations on %1." ).arg( m_device->deviceNode() );
+}
+
 Calamares::JobResult
 ClearMountsJob::exec()
 {

From ac34cfadea258224e4d6c881f312ec8ef4e16780 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 1 Nov 2021 20:23:49 +0100
Subject: [PATCH 02/16] [partition] Factor out finding the swap-partitions

---
 src/modules/partition/jobs/ClearMountsJob.cpp | 67 +++++++++++--------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index ea4ed0d00..817007e1a 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -72,42 +72,16 @@ getPartitionsForDevice( const QString& deviceName )
     return partitions;
 }
 
-ClearMountsJob::ClearMountsJob( Device* device )
-    : Calamares::Job()
-    , m_device( device )
+STATICTEST static QStringList
+getSwapsForDevice( const QString& deviceName )
 {
-}
-
-QString
-ClearMountsJob::prettyName() const
-{
-    return tr( "Clear mounts for partitioning operations on %1" ).arg( m_device->deviceNode() );
-}
-
-
-QString
-ClearMountsJob::prettyStatusMessage() const
-{
-    return tr( "Clearing mounts for partitioning operations on %1." ).arg( m_device->deviceNode() );
-}
-
-Calamares::JobResult
-ClearMountsJob::exec()
-{
-    CalamaresUtils::Partition::Syncer s;
-
-    QString deviceName = m_device->deviceNode().split( '/' ).last();
-
-    QStringList goodNews;
     QProcess process;
 
-    QStringList partitionsList = getPartitionsForDevice( deviceName );
-
     // Build a list of partitions of type 82 (Linux swap / Solaris).
     // We then need to clear them just in case they contain something resumable from a
     // previous suspend-to-disk.
     QStringList swapPartitions;
-    process.start( "sfdisk", { "-d", m_device->deviceNode() } );
+    process.start( "sfdisk", { "-d", deviceName } );
     process.waitForFinished();
     // Sample output:
     //    % sudo sfdisk -d /dev/sda
@@ -126,6 +100,41 @@ ClearMountsJob::exec()
         *it = ( *it ).simplified().split( ' ' ).first();
     }
 
+    return swapPartitions;
+}
+
+
+ClearMountsJob::ClearMountsJob( Device* device )
+    : Calamares::Job()
+    , m_device( device )
+{
+}
+
+QString
+ClearMountsJob::prettyName() const
+{
+    return tr( "Clear mounts for partitioning operations on %1" ).arg( m_device->deviceNode() );
+}
+
+QString
+ClearMountsJob::prettyStatusMessage() const
+{
+    return tr( "Clearing mounts for partitioning operations on %1." ).arg( m_device->deviceNode() );
+}
+
+Calamares::JobResult
+ClearMountsJob::exec()
+{
+    CalamaresUtils::Partition::Syncer s;
+
+    QString deviceName = m_device->deviceNode().split( '/' ).last();
+
+    QStringList goodNews;
+    QProcess process;
+
+    const QStringList partitionsList = getPartitionsForDevice( deviceName );
+    const QStringList swapPartitions = getSwapsForDevice( m_device->deviceNode() );
+
     const QStringList cryptoDevices = getCryptoDevices();
     for ( const QString& mapperPath : cryptoDevices )
     {

From 105517fed7b049558f41500796206a8cf3b1c218 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 1 Nov 2021 20:24:47 +0100
Subject: [PATCH 03/16] [partition] Coding style

---
 src/modules/partition/jobs/ClearMountsJob.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index 817007e1a..da60ebc3d 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -230,7 +230,7 @@ ClearMountsJob::exec()
         }
     }
 
-    foreach ( QString p, swapPartitions )
+    for ( const QString& p : swapPartitions )
     {
         QString news = tryClearSwap( p );
         if ( !news.isEmpty() )

From 0c84a87c67786eb43aab67f55dcb8b9389914a79 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 1 Nov 2021 20:28:47 +0100
Subject: [PATCH 04/16] [partition] Make internal methods static

---
 src/modules/partition/jobs/ClearMountsJob.cpp | 121 +++++++++---------
 src/modules/partition/jobs/ClearMountsJob.h   |   3 -
 2 files changed, 59 insertions(+), 65 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index da60ebc3d..f854daa72 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -72,7 +72,7 @@ getPartitionsForDevice( const QString& deviceName )
     return partitions;
 }
 
-STATICTEST static QStringList
+STATICTEST QStringList
 getSwapsForDevice( const QString& deviceName )
 {
     QProcess process;
@@ -103,6 +103,64 @@ getSwapsForDevice( const QString& deviceName )
     return swapPartitions;
 }
 
+STATICTEST QString
+tryUmount( const QString& partPath )
+{
+    QProcess process;
+    process.start( "umount", { partPath } );
+    process.waitForFinished();
+    if ( process.exitCode() == 0 )
+    {
+        return QString( "Successfully unmounted %1." ).arg( partPath );
+    }
+
+    process.start( "swapoff", { partPath } );
+    process.waitForFinished();
+    if ( process.exitCode() == 0 )
+    {
+        return QString( "Successfully disabled swap %1." ).arg( partPath );
+    }
+
+    return QString();
+}
+
+
+STATICTEST QString
+tryClearSwap( const QString& partPath )
+{
+    QProcess process;
+    process.start( "blkid", { "-s", "UUID", "-o", "value", partPath } );
+    process.waitForFinished();
+    QString swapPartUuid = QString::fromLocal8Bit( process.readAllStandardOutput() ).simplified();
+    if ( process.exitCode() != 0 || swapPartUuid.isEmpty() )
+    {
+        return QString();
+    }
+
+    process.start( "mkswap", { "-U", swapPartUuid, partPath } );
+    process.waitForFinished();
+    if ( process.exitCode() != 0 )
+    {
+        return QString();
+    }
+
+    return QString( "Successfully cleared swap %1." ).arg( partPath );
+}
+
+
+STATICTEST QString
+tryCryptoClose( const QString& mapperPath )
+{
+    QProcess process;
+    process.start( "cryptsetup", { "close", mapperPath } );
+    process.waitForFinished();
+    if ( process.exitCode() == 0 )
+    {
+        return QString( "Successfully closed mapper device %1." ).arg( mapperPath );
+    }
+
+    return QString();
+}
 
 ClearMountsJob::ClearMountsJob( Device* device )
     : Calamares::Job()
@@ -248,67 +306,6 @@ ClearMountsJob::exec()
     return ok;
 }
 
-
-QString
-ClearMountsJob::tryUmount( const QString& partPath )
-{
-    QProcess process;
-    process.start( "umount", { partPath } );
-    process.waitForFinished();
-    if ( process.exitCode() == 0 )
-    {
-        return QString( "Successfully unmounted %1." ).arg( partPath );
-    }
-
-    process.start( "swapoff", { partPath } );
-    process.waitForFinished();
-    if ( process.exitCode() == 0 )
-    {
-        return QString( "Successfully disabled swap %1." ).arg( partPath );
-    }
-
-    return QString();
-}
-
-
-QString
-ClearMountsJob::tryClearSwap( const QString& partPath )
-{
-    QProcess process;
-    process.start( "blkid", { "-s", "UUID", "-o", "value", partPath } );
-    process.waitForFinished();
-    QString swapPartUuid = QString::fromLocal8Bit( process.readAllStandardOutput() ).simplified();
-    if ( process.exitCode() != 0 || swapPartUuid.isEmpty() )
-    {
-        return QString();
-    }
-
-    process.start( "mkswap", { "-U", swapPartUuid, partPath } );
-    process.waitForFinished();
-    if ( process.exitCode() != 0 )
-    {
-        return QString();
-    }
-
-    return QString( "Successfully cleared swap %1." ).arg( partPath );
-}
-
-
-QString
-ClearMountsJob::tryCryptoClose( const QString& mapperPath )
-{
-    QProcess process;
-    process.start( "cryptsetup", { "close", mapperPath } );
-    process.waitForFinished();
-    if ( process.exitCode() == 0 )
-    {
-        return QString( "Successfully closed mapper device %1." ).arg( mapperPath );
-    }
-
-    return QString();
-}
-
-
 QStringList
 ClearMountsJob::getCryptoDevices() const
 {
diff --git a/src/modules/partition/jobs/ClearMountsJob.h b/src/modules/partition/jobs/ClearMountsJob.h
index 99a7b4844..0f12ba8e2 100644
--- a/src/modules/partition/jobs/ClearMountsJob.h
+++ b/src/modules/partition/jobs/ClearMountsJob.h
@@ -28,9 +28,6 @@ public:
     Calamares::JobResult exec() override;
 
 private:
-    QString tryUmount( const QString& partPath );
-    QString tryClearSwap( const QString& partPath );
-    QString tryCryptoClose( const QString& mapperPath );
     QStringList getCryptoDevices() const;
     Device* m_device;
 };

From 6c2b2b0daaa65c9d9011a51e2edc3d6b1efe8fca Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 1 Nov 2021 20:44:55 +0100
Subject: [PATCH 05/16] [partition] Factor out loops, document return values

- the tryX() functions weirdly return a string that is used for
  debug-logging. Document that. The untranslated string is
  later used for user-facing messages. Mark that as FIXME.
- factor out the loop-over-names-and-append to news, because that
  makes the overall story of what is happening hard to read.
- all calls to tryCryptoClose() called tryUnmount() first, so
  put that call inside tryCryptoClose(), so the interface is simpler.
---
 src/modules/partition/jobs/ClearMountsJob.cpp | 78 +++++++++----------
 1 file changed, 35 insertions(+), 43 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index f854daa72..19666d301 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -103,6 +103,16 @@ getSwapsForDevice( const QString& deviceName )
     return swapPartitions;
 }
 
+
+/*
+ * The tryX() free functions, below, return an empty QString on
+ * failure, or a non-empty QString on success. The string is
+ * meant **only** for debugging and is not displayed to the user,
+ * which is why no translation is applied.
+ *
+ */
+
+///@brief Returns a debug-string if @p partPath could be unmounted
 STATICTEST QString
 tryUmount( const QString& partPath )
 {
@@ -124,7 +134,7 @@ tryUmount( const QString& partPath )
     return QString();
 }
 
-
+///@brief Returns a debug-string if @p partPath was swap and could be cleared
 STATICTEST QString
 tryClearSwap( const QString& partPath )
 {
@@ -147,10 +157,12 @@ tryClearSwap( const QString& partPath )
     return QString( "Successfully cleared swap %1." ).arg( partPath );
 }
 
-
+///@brief Returns a debug-string if @p mapperPath could be closed
 STATICTEST QString
 tryCryptoClose( const QString& mapperPath )
 {
+    /* ignored */ tryUmount( mapperPath );
+
     QProcess process;
     process.start( "cryptsetup", { "close", mapperPath } );
     process.waitForFinished();
@@ -162,6 +174,21 @@ tryCryptoClose( const QString& mapperPath )
     return QString();
 }
 
+///@brief Apply @p f to all the @p paths, appending successes to @p news
+template < typename F >
+void
+apply( const QStringList& paths, F f, QStringList& news )
+{
+    for ( const QString& p : qAsConst( paths ) )
+    {
+        QString n = f( p );
+        if ( !n.isEmpty() )
+        {
+            news.append( n );
+        }
+    }
+}
+
 ClearMountsJob::ClearMountsJob( Device* device )
     : Calamares::Job()
     , m_device( device )
@@ -193,16 +220,7 @@ ClearMountsJob::exec()
     const QStringList partitionsList = getPartitionsForDevice( deviceName );
     const QStringList swapPartitions = getSwapsForDevice( m_device->deviceNode() );
 
-    const QStringList cryptoDevices = getCryptoDevices();
-    for ( const QString& mapperPath : cryptoDevices )
-    {
-        tryUmount( mapperPath );
-        QString news = tryCryptoClose( mapperPath );
-        if ( !news.isEmpty() )
-        {
-            goodNews.append( news );
-        }
-    }
+    apply( getCryptoDevices(), tryCryptoClose, goodNews );
 
     // First we umount all LVM logical volumes we can find
     process.start( "lvscan", { "-a" } );
@@ -266,40 +284,14 @@ ClearMountsJob::exec()
         cWarning() << "this system does not seem to have LVM2 tools.";
     }
 
-    const QStringList cryptoDevices2 = getCryptoDevices();
-    for ( const QString& mapperPath : cryptoDevices2 )
-    {
-        tryUmount( mapperPath );
-        QString news = tryCryptoClose( mapperPath );
-        if ( !news.isEmpty() )
-        {
-            goodNews.append( news );
-        }
-    }
-
-    for ( const QString& p : partitionsList )
-    {
-        QString partPath = QString( "/dev/%1" ).arg( p );
-
-        QString news = tryUmount( partPath );
-        if ( !news.isEmpty() )
-        {
-            goodNews.append( news );
-        }
-    }
-
-    for ( const QString& p : swapPartitions )
-    {
-        QString news = tryClearSwap( p );
-        if ( !news.isEmpty() )
-        {
-            goodNews.append( news );
-        }
-    }
+    apply( getCryptoDevices(), tryCryptoClose, goodNews );
+    apply(
+        partitionsList, []( const QString& p ) { return tryUmount( QString( "/dev/%1" ).arg( p ) ); }, goodNews );
+    apply( swapPartitions, tryClearSwap, goodNews );
 
     Calamares::JobResult ok = Calamares::JobResult::ok();
     ok.setMessage( tr( "Cleared all mounts for %1" ).arg( m_device->deviceNode() ) );
-    ok.setDetails( goodNews.join( "\n" ) );
+    ok.setDetails( goodNews.join( "\n" ) ); // FIXME: this exposes untranslated strings
 
     cDebug() << "ClearMountsJob finished. Here's what was done:\n" << goodNews.join( "\n" );
 

From e56158f5b4507e3e3bcaef5c93ead1af8c64666d Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 1 Nov 2021 20:52:24 +0100
Subject: [PATCH 06/16] [partition] Generate partition paths with /dev/

Returning partition full-paths instead of only the block-device-name
simplifies later code -- which would prepend /dev/ to the block-
device-name and umount that.
---
 src/modules/partition/jobs/ClearMountsJob.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index 19666d301..1ebb00310 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -34,7 +34,7 @@ using CalamaresUtils::Partition::PartitionIterator;
  *
  * The @p deviceName is a (whole-block) device, like "sda", and the partitions
  * returned are then "sdaX". The whole-block device itself is ignored, if
- * present.
+ * present. Partitions are returned with their full /dev/ path (e.g. /dev/sda1).
  *
  * The format for /etc/partitions is, e.g.
  *      major minor #blocks name
@@ -60,7 +60,7 @@ getPartitionsForDevice( const QString& deviceName )
             if ( ( columns.count() >= 4 ) && ( columns[ 3 ].startsWith( deviceName ) )
                  && ( columns[ 3 ] != deviceName ) )
             {
-                partitions.append( columns[ 3 ] );
+                partitions.append( QStringLiteral( "/dev/" ) + columns[ 3 ] );
             }
         }
     }
@@ -285,8 +285,7 @@ ClearMountsJob::exec()
     }
 
     apply( getCryptoDevices(), tryCryptoClose, goodNews );
-    apply(
-        partitionsList, []( const QString& p ) { return tryUmount( QString( "/dev/%1" ).arg( p ) ); }, goodNews );
+    apply( partitionsList, tryUmount, goodNews );
     apply( swapPartitions, tryClearSwap, goodNews );
 
     Calamares::JobResult ok = Calamares::JobResult::ok();

From c322eaa43023615320d7a1095aaf531df52281e7 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 1 Nov 2021 23:48:18 +0100
Subject: [PATCH 07/16] [partition] Fix translation issues

- Strings were being used as logical values, and then logged
  (which should be in English) and also used in the UI (which
  should be localized). Replace with a MessageAndPath class,
  used only locally, that defers the translation until called-
  upon explicitly.
- Replace some VG stuff with similar calls to apply().
---
 src/modules/partition/jobs/ClearMountsJob.cpp | 121 ++++++++++++------
 1 file changed, 84 insertions(+), 37 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index 1ebb00310..e5c56126a 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -23,6 +23,7 @@
 #include <kpmcore/core/partition.h>
 #include <kpmcore/util/report.h>
 
+#include <QCoreApplication>
 #include <QDir>
 #include <QProcess>
 #include <QStringList>
@@ -112,8 +113,42 @@ getSwapsForDevice( const QString& deviceName )
  *
  */
 
+class MessageAndPath
+{
+public:
+    ///@brief An unsuccessful attempt at something
+    MessageAndPath() {}
+    ///@brief A success at doing @p thing to @p path
+    MessageAndPath( const char* thing, const QString& path )
+        : m_message( thing )
+        , m_path( path )
+    {
+    }
+
+    bool isEmpty() const { return m_message; }
+
+    explicit operator QString() const
+    {
+        return isEmpty() ? QString() : QCoreApplication::translate( "ClearMountsJob", m_message );
+    }
+
+    const char* const m_message = nullptr;
+    QString const m_path;
+};
+
+STATICTEST inline QDebug&
+operator<<( QDebug& s, const MessageAndPath& m )
+{
+    if ( m.isEmpty() )
+    {
+        return s;
+    }
+    return s << QString( m.m_message ).arg( m.m_path );
+}
+
+
 ///@brief Returns a debug-string if @p partPath could be unmounted
-STATICTEST QString
+STATICTEST MessageAndPath
 tryUmount( const QString& partPath )
 {
     QProcess process;
@@ -121,21 +156,21 @@ tryUmount( const QString& partPath )
     process.waitForFinished();
     if ( process.exitCode() == 0 )
     {
-        return QString( "Successfully unmounted %1." ).arg( partPath );
+        return { QT_TRANSLATE_NOOP( "ClearMountsJob", "Successfully unmounted %1." ), partPath };
     }
 
     process.start( "swapoff", { partPath } );
     process.waitForFinished();
     if ( process.exitCode() == 0 )
     {
-        return QString( "Successfully disabled swap %1." ).arg( partPath );
+        return { QT_TRANSLATE_NOOP( "ClearMountsJob", "Successfully disabled swap %1." ), partPath };
     }
 
-    return QString();
+    return {};
 }
 
 ///@brief Returns a debug-string if @p partPath was swap and could be cleared
-STATICTEST QString
+STATICTEST MessageAndPath
 tryClearSwap( const QString& partPath )
 {
     QProcess process;
@@ -144,21 +179,21 @@ tryClearSwap( const QString& partPath )
     QString swapPartUuid = QString::fromLocal8Bit( process.readAllStandardOutput() ).simplified();
     if ( process.exitCode() != 0 || swapPartUuid.isEmpty() )
     {
-        return QString();
+        return {};
     }
 
     process.start( "mkswap", { "-U", swapPartUuid, partPath } );
     process.waitForFinished();
     if ( process.exitCode() != 0 )
     {
-        return QString();
+        return {};
     }
 
-    return QString( "Successfully cleared swap %1." ).arg( partPath );
+    return { QT_TRANSLATE_NOOP( "ClearMountsJob", "Successfully cleared swap %1." ), partPath };
 }
 
 ///@brief Returns a debug-string if @p mapperPath could be closed
-STATICTEST QString
+STATICTEST MessageAndPath
 tryCryptoClose( const QString& mapperPath )
 {
     /* ignored */ tryUmount( mapperPath );
@@ -168,20 +203,20 @@ tryCryptoClose( const QString& mapperPath )
     process.waitForFinished();
     if ( process.exitCode() == 0 )
     {
-        return QString( "Successfully closed mapper device %1." ).arg( mapperPath );
+        return { QT_TRANSLATE_NOOP( "ClearMountsJob", "Successfully closed mapper device %1." ), mapperPath };
     }
 
-    return QString();
+    return {};
 }
 
 ///@brief Apply @p f to all the @p paths, appending successes to @p news
-template < typename F >
+template < typename T, typename F >
 void
-apply( const QStringList& paths, F f, QStringList& news )
+apply( const T& paths, F f, QList< MessageAndPath >& news )
 {
     for ( const QString& p : qAsConst( paths ) )
     {
-        QString n = f( p );
+        auto n = f( p );
         if ( !n.isEmpty() )
         {
             news.append( n );
@@ -189,6 +224,17 @@ apply( const QStringList& paths, F f, QStringList& news )
     }
 }
 
+STATICTEST QStringList
+stringify( const QList< MessageAndPath >& news )
+{
+    QStringList l;
+    for ( const auto& m : qAsConst( news ) )
+    {
+        l << QString( m );
+    }
+    return l;
+}
+
 ClearMountsJob::ClearMountsJob( Device* device )
     : Calamares::Job()
     , m_device( device )
@@ -214,7 +260,7 @@ ClearMountsJob::exec()
 
     QString deviceName = m_device->deviceNode().split( '/' ).last();
 
-    QStringList goodNews;
+    QList< MessageAndPath > goodNews;
     QProcess process;
 
     const QStringList partitionsList = getPartitionsForDevice( deviceName );
@@ -228,17 +274,15 @@ ClearMountsJob::exec()
     if ( process.exitCode() == 0 )  //means LVM2 tools are installed
     {
         const QStringList lvscanLines = QString::fromLocal8Bit( process.readAllStandardOutput() ).split( '\n' );
-        for ( const QString& lvscanLine : lvscanLines )
-        {
-            QString lvPath = lvscanLine.simplified().split( ' ' ).value( 1 );  //second column
-            lvPath = lvPath.replace( '\'', "" );
+        apply(
+            lvscanLines,
+            []( const QString& lvscanLine ) {
+                QString lvPath = lvscanLine.simplified().split( ' ' ).value( 1 );  //second column
+                lvPath = lvPath.replace( '\'', "" );
 
-            QString news = tryUmount( lvPath );
-            if ( !news.isEmpty() )
-            {
-                goodNews.append( news );
-            }
-        }
+                return tryUmount( lvPath );
+            },
+            goodNews );
     }
     else
     {
@@ -268,15 +312,19 @@ ClearMountsJob::exec()
                 vgSet.insert( vgName );
             }
 
-            foreach ( const QString& vgName, vgSet )
-            {
-                process.start( "vgchange", { "-an", vgName } );
-                process.waitForFinished();
-                if ( process.exitCode() == 0 )
-                {
-                    goodNews.append( QString( "Successfully disabled volume group %1." ).arg( vgName ) );
-                }
-            }
+            apply(
+                vgSet,
+                []( const QString& vgName ) {
+                    QProcess vgProcess;
+                    vgProcess.start( "vgchange", { "-an", vgName } );
+                    vgProcess.waitForFinished();
+                    return ( vgProcess.exitCode() == 0 )
+                        ? MessageAndPath { QT_TRANSLATE_NOOP( "ClearMountsJob",
+                                                              "Successfully disabled volume group %1." ),
+                                           vgName }
+                        : MessageAndPath {};
+                },
+                goodNews );
         }
     }
     else
@@ -290,9 +338,8 @@ ClearMountsJob::exec()
 
     Calamares::JobResult ok = Calamares::JobResult::ok();
     ok.setMessage( tr( "Cleared all mounts for %1" ).arg( m_device->deviceNode() ) );
-    ok.setDetails( goodNews.join( "\n" ) ); // FIXME: this exposes untranslated strings
-
-    cDebug() << "ClearMountsJob finished. Here's what was done:\n" << goodNews.join( "\n" );
+    ok.setDetails( stringify( goodNews ).join( "\n" ) );
+    cDebug() << "ClearMountsJob finished. Here's what was done:" << Logger::DebugListT< MessageAndPath >( goodNews );
 
     return ok;
 }

From f49389a408fe32b1f56c47fbb62d5288795235d1 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 00:09:52 +0100
Subject: [PATCH 08/16] [partition] Fix logic errors in stringification of
 MessageAndPath

---
 src/modules/partition/jobs/ClearMountsJob.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index e5c56126a..23a2c287f 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -125,13 +125,14 @@ public:
     {
     }
 
-    bool isEmpty() const { return m_message; }
+    bool isEmpty() const { return !m_message; }
 
     explicit operator QString() const
     {
-        return isEmpty() ? QString() : QCoreApplication::translate( "ClearMountsJob", m_message );
+        return isEmpty() ? QString() : QCoreApplication::translate( "ClearMountsJob", m_message ).arg( m_path );
     }
 
+private:
     const char* const m_message = nullptr;
     QString const m_path;
 };
@@ -143,7 +144,7 @@ operator<<( QDebug& s, const MessageAndPath& m )
     {
         return s;
     }
-    return s << QString( m.m_message ).arg( m.m_path );
+    return s << QString( m );
 }
 
 

From 04b119b0519bcb024411f550c4f81243c7c85fd0 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 00:25:37 +0100
Subject: [PATCH 09/16] [partition] Crypto device-list needn't be a member,
 either

---
 src/modules/partition/jobs/ClearMountsJob.cpp | 61 ++++++++++++-------
 src/modules/partition/jobs/ClearMountsJob.h   |  1 -
 2 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index 23a2c287f..4bab3b0ae 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -104,6 +104,45 @@ getSwapsForDevice( const QString& deviceName )
     return swapPartitions;
 }
 
+static inline bool
+isControl( const QString& baseName )
+{
+    return baseName == "control";
+}
+
+static inline bool
+isFedoraSpecial( const QString& baseName )
+{
+    // Fedora live images use /dev/mapper/live-* internally. We must not
+    // unmount those devices, because they are used by the live image and
+    // because we need /dev/mapper/live-base in the unpackfs module.
+    return baseName.startsWith( "live-" );
+}
+
+/** @brief Returns a list of unneeded crypto devices
+ *
+ * These are the crypto devices to unmount and close; some are "needed"
+ * for system operation: on Fedora, the live- mappers are special.
+ * Some other devices are special, too, so those do not end up in
+ * the list.
+ */
+STATICTEST QStringList
+getCryptoDevices()
+{
+    QDir mapperDir( "/dev/mapper" );
+    const QFileInfoList fiList = mapperDir.entryInfoList( QDir::Files );
+    QStringList list;
+    for ( const QFileInfo& fi : fiList )
+    {
+        QString baseName = fi.baseName();
+        if ( isControl( baseName ) || isFedoraSpecial( baseName ) )
+        {
+            continue;
+        }
+        list.append( fi.absoluteFilePath() );
+    }
+    return list;
+}
 
 /*
  * The tryX() free functions, below, return an empty QString on
@@ -344,25 +383,3 @@ ClearMountsJob::exec()
 
     return ok;
 }
-
-QStringList
-ClearMountsJob::getCryptoDevices() const
-{
-    QDir mapperDir( "/dev/mapper" );
-    const QFileInfoList fiList = mapperDir.entryInfoList( QDir::Files );
-    QStringList list;
-    QProcess process;
-    for ( const QFileInfo& fi : fiList )
-    {
-        QString baseName = fi.baseName();
-        // Fedora live images use /dev/mapper/live-* internally. We must not
-        // unmount those devices, because they are used by the live image and
-        // because we need /dev/mapper/live-base in the unpackfs module.
-        if ( baseName == "control" || baseName.startsWith( "live-" ) )
-        {
-            continue;
-        }
-        list.append( fi.absoluteFilePath() );
-    }
-    return list;
-}
diff --git a/src/modules/partition/jobs/ClearMountsJob.h b/src/modules/partition/jobs/ClearMountsJob.h
index 0f12ba8e2..070e06c30 100644
--- a/src/modules/partition/jobs/ClearMountsJob.h
+++ b/src/modules/partition/jobs/ClearMountsJob.h
@@ -28,7 +28,6 @@ public:
     Calamares::JobResult exec() override;
 
 private:
-    QStringList getCryptoDevices() const;
     Device* m_device;
 };
 

From 2a1ec84c87bc0e9b89f47b89c8d17cf8bc653ed1 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 11:03:10 +0100
Subject: [PATCH 10/16] [partition] Don't hang on to pointer longer than needed

---
 src/modules/partition/jobs/ClearMountsJob.cpp | 12 ++++++------
 src/modules/partition/jobs/ClearMountsJob.h   | 10 +++++++++-
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index 4bab3b0ae..dfca20718 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -277,20 +277,20 @@ stringify( const QList< MessageAndPath >& news )
 
 ClearMountsJob::ClearMountsJob( Device* device )
     : Calamares::Job()
-    , m_device( device )
+    , m_deviceNode( device->deviceNode() )
 {
 }
 
 QString
 ClearMountsJob::prettyName() const
 {
-    return tr( "Clear mounts for partitioning operations on %1" ).arg( m_device->deviceNode() );
+    return tr( "Clear mounts for partitioning operations on %1" ).arg( m_deviceNode );
 }
 
 QString
 ClearMountsJob::prettyStatusMessage() const
 {
-    return tr( "Clearing mounts for partitioning operations on %1." ).arg( m_device->deviceNode() );
+    return tr( "Clearing mounts for partitioning operations on %1." ).arg( m_deviceNode );
 }
 
 Calamares::JobResult
@@ -298,13 +298,13 @@ ClearMountsJob::exec()
 {
     CalamaresUtils::Partition::Syncer s;
 
-    QString deviceName = m_device->deviceNode().split( '/' ).last();
+    const QString deviceName = m_deviceNode.split( '/' ).last();
 
     QList< MessageAndPath > goodNews;
     QProcess process;
 
     const QStringList partitionsList = getPartitionsForDevice( deviceName );
-    const QStringList swapPartitions = getSwapsForDevice( m_device->deviceNode() );
+    const QStringList swapPartitions = getSwapsForDevice( m_deviceNode );
 
     apply( getCryptoDevices(), tryCryptoClose, goodNews );
 
@@ -377,7 +377,7 @@ ClearMountsJob::exec()
     apply( swapPartitions, tryClearSwap, goodNews );
 
     Calamares::JobResult ok = Calamares::JobResult::ok();
-    ok.setMessage( tr( "Cleared all mounts for %1" ).arg( m_device->deviceNode() ) );
+    ok.setMessage( tr( "Cleared all mounts for %1" ).arg( m_deviceNode ) );
     ok.setDetails( stringify( goodNews ).join( "\n" ) );
     cDebug() << "ClearMountsJob finished. Here's what was done:" << Logger::DebugListT< MessageAndPath >( goodNews );
 
diff --git a/src/modules/partition/jobs/ClearMountsJob.h b/src/modules/partition/jobs/ClearMountsJob.h
index 070e06c30..f0ef3e104 100644
--- a/src/modules/partition/jobs/ClearMountsJob.h
+++ b/src/modules/partition/jobs/ClearMountsJob.h
@@ -22,13 +22,21 @@ class ClearMountsJob : public Calamares::Job
 {
     Q_OBJECT
 public:
+    /** @brief Creates a job freeing mounts on @p device
+     *
+     * All /dev/mapper entries are closed, regardless of device.
+     *
+     * No ownership is transferred; the @p device is used only to access
+     * the device node (name).
+     */
     explicit ClearMountsJob( Device* device );
+
     QString prettyName() const override;
     QString prettyStatusMessage() const override;
     Calamares::JobResult exec() override;
 
 private:
-    Device* m_device;
+    const QString m_deviceNode;
 };
 
 #endif  // CLEARMOUNTSJOB_H

From 7fa02fd41c332bf3c87d59c9c1dd6509680f604e Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 11:10:09 +0100
Subject: [PATCH 11/16] [partition] Extract the get-LVM-volumes code to its own
 function

---
 src/modules/partition/jobs/ClearMountsJob.cpp | 47 ++++++++++---------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index dfca20718..0b48612e4 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -144,6 +144,31 @@ getCryptoDevices()
     return list;
 }
 
+STATICTEST QStringList
+getLVMVolumes()
+{
+    QProcess process;
+
+    // First we umount all LVM logical volumes we can find
+    process.start( "lvscan", { "-a" } );
+    process.waitForFinished();
+    if ( process.exitCode() == 0 )  //means LVM2 tools are installed
+    {
+        QStringList lvscanLines = QString::fromLocal8Bit( process.readAllStandardOutput() ).split( '\n' );
+        // Get the second column (`value(1)`) sinec that is the device name,
+        // remove quoting.
+        std::transform( lvscanLines.begin(), lvscanLines.end(), lvscanLines.begin(), []( const QString& lvscanLine ) {
+            return lvscanLine.simplified().split( ' ' ).value( 1 ).replace( '\'', "" );
+        } );
+        return lvscanLines;
+    }
+    else
+    {
+        cWarning() << "this system does not seem to have LVM2 tools.";
+        return QStringList();
+    }
+}
+
 /*
  * The tryX() free functions, below, return an empty QString on
  * failure, or a non-empty QString on success. The string is
@@ -307,27 +332,7 @@ ClearMountsJob::exec()
     const QStringList swapPartitions = getSwapsForDevice( m_deviceNode );
 
     apply( getCryptoDevices(), tryCryptoClose, goodNews );
-
-    // First we umount all LVM logical volumes we can find
-    process.start( "lvscan", { "-a" } );
-    process.waitForFinished();
-    if ( process.exitCode() == 0 )  //means LVM2 tools are installed
-    {
-        const QStringList lvscanLines = QString::fromLocal8Bit( process.readAllStandardOutput() ).split( '\n' );
-        apply(
-            lvscanLines,
-            []( const QString& lvscanLine ) {
-                QString lvPath = lvscanLine.simplified().split( ' ' ).value( 1 );  //second column
-                lvPath = lvPath.replace( '\'', "" );
-
-                return tryUmount( lvPath );
-            },
-            goodNews );
-    }
-    else
-    {
-        cWarning() << "this system does not seem to have LVM2 tools.";
-    }
+    apply( getLVMVolumes(), tryUmount, goodNews );
 
     // Then we go looking for volume groups that use this device for physical volumes
     process.start( "pvdisplay", { "-C", "--noheadings" } );

From ca4a187d1a0a267cf09e11a92fed5f78066f718d Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 11:18:14 +0100
Subject: [PATCH 12/16] [partition] Extract the get-PV-groups code to its own
 function

- get the list (once)
- move the lambda to a named function for readability
---
 src/modules/partition/jobs/ClearMountsJob.cpp | 96 ++++++++++---------
 1 file changed, 50 insertions(+), 46 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index 0b48612e4..fd0375ca4 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -165,8 +165,43 @@ getLVMVolumes()
     else
     {
         cWarning() << "this system does not seem to have LVM2 tools.";
-        return QStringList();
     }
+    return QStringList();
+}
+STATICTEST QStringList
+getPVGroups( const QString& deviceName )
+{
+    QProcess process;
+    // Then we go looking for volume groups that use this device for physical volumes
+    process.start( "pvdisplay", { "-C", "--noheadings" } );
+    process.waitForFinished();
+    if ( process.exitCode() == 0 )  //means LVM2 tools are installed
+    {
+        QString pvdisplayOutput = process.readAllStandardOutput();
+        if ( !pvdisplayOutput.simplified().isEmpty() )  //means there is at least one LVM PV
+        {
+            QSet< QString > vgSet;
+
+            const QStringList pvdisplayLines = pvdisplayOutput.split( '\n' );
+            for ( const QString& pvdisplayLine : pvdisplayLines )
+            {
+                QString pvPath = pvdisplayLine.simplified().split( ' ' ).value( 0 );
+                QString vgName = pvdisplayLine.simplified().split( ' ' ).value( 1 );
+                if ( !pvPath.contains( deviceName ) )
+                {
+                    continue;
+                }
+
+                vgSet.insert( vgName );
+            }
+            return QStringList { vgSet.cbegin(), vgSet.cend() };
+        }
+    }
+    else
+    {
+        cWarning() << "this system does not seem to have LVM2 tools.";
+    }
+    return QStringList();
 }
 
 /*
@@ -274,10 +309,21 @@ tryCryptoClose( const QString& mapperPath )
     return {};
 }
 
+STATICTEST MessageAndPath
+tryVGDisable( const QString& vgName )
+{
+    QProcess vgProcess;
+    vgProcess.start( "vgchange", { "-an", vgName } );
+    vgProcess.waitForFinished();
+    return ( vgProcess.exitCode() == 0 )
+        ? MessageAndPath { QT_TRANSLATE_NOOP( "ClearMountsJob", "Successfully disabled volume group %1." ), vgName }
+        : MessageAndPath {};
+}
+
 ///@brief Apply @p f to all the @p paths, appending successes to @p news
-template < typename T, typename F >
+template < typename F >
 void
-apply( const T& paths, F f, QList< MessageAndPath >& news )
+apply( const QStringList& paths, F f, QList< MessageAndPath >& news )
 {
     for ( const QString& p : qAsConst( paths ) )
     {
@@ -333,49 +379,7 @@ ClearMountsJob::exec()
 
     apply( getCryptoDevices(), tryCryptoClose, goodNews );
     apply( getLVMVolumes(), tryUmount, goodNews );
-
-    // Then we go looking for volume groups that use this device for physical volumes
-    process.start( "pvdisplay", { "-C", "--noheadings" } );
-    process.waitForFinished();
-    if ( process.exitCode() == 0 )  //means LVM2 tools are installed
-    {
-        QString pvdisplayOutput = process.readAllStandardOutput();
-        if ( !pvdisplayOutput.simplified().isEmpty() )  //means there is at least one LVM PV
-        {
-            QSet< QString > vgSet;
-
-            const QStringList pvdisplayLines = pvdisplayOutput.split( '\n' );
-            for ( const QString& pvdisplayLine : pvdisplayLines )
-            {
-                QString pvPath = pvdisplayLine.simplified().split( ' ' ).value( 0 );
-                QString vgName = pvdisplayLine.simplified().split( ' ' ).value( 1 );
-                if ( !pvPath.contains( deviceName ) )
-                {
-                    continue;
-                }
-
-                vgSet.insert( vgName );
-            }
-
-            apply(
-                vgSet,
-                []( const QString& vgName ) {
-                    QProcess vgProcess;
-                    vgProcess.start( "vgchange", { "-an", vgName } );
-                    vgProcess.waitForFinished();
-                    return ( vgProcess.exitCode() == 0 )
-                        ? MessageAndPath { QT_TRANSLATE_NOOP( "ClearMountsJob",
-                                                              "Successfully disabled volume group %1." ),
-                                           vgName }
-                        : MessageAndPath {};
-                },
-                goodNews );
-        }
-    }
-    else
-    {
-        cWarning() << "this system does not seem to have LVM2 tools.";
-    }
+    apply( getPVGroups( deviceName ), tryVGDisable, goodNews );
 
     apply( getCryptoDevices(), tryCryptoClose, goodNews );
     apply( partitionsList, tryUmount, goodNews );

From 0253977778a975eccee8cea5810e22cf7b7873b7 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 11:50:51 +0100
Subject: [PATCH 13/16] [partition] Coding style

---
 src/modules/partition/core/PartitionCoreModule.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp
index e2c91fbee..0f53ba413 100644
--- a/src/modules/partition/core/PartitionCoreModule.cpp
+++ b/src/modules/partition/core/PartitionCoreModule.cpp
@@ -604,7 +604,7 @@ PartitionCoreModule::jobs( const Config* config ) const
     lst << automountControl;
     lst << Calamares::job_ptr( new ClearTempMountsJob() );
 
-    for ( auto info : m_deviceInfos )
+    for ( const auto* info : m_deviceInfos )
     {
         if ( info->isDirty() )
         {
@@ -612,7 +612,7 @@ PartitionCoreModule::jobs( const Config* config ) const
         }
     }
 
-    for ( auto info : m_deviceInfos )
+    for ( const auto* info : m_deviceInfos )
     {
         lst << info->jobs();
         devices << info->device.data();

From 1410157356aa6ec8bb17cd339dcc251bff8abbd8 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 11:55:13 +0100
Subject: [PATCH 14/16] [partition] Simplify and document ClearMounts

- note that the job indiscriminately closes all LUKS and LV
- don't hang on to lists we don't need
---
 src/modules/partition/jobs/ClearMountsJob.cpp | 12 +++---------
 src/modules/partition/jobs/ClearMountsJob.h   | 11 +++++++++--
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index fd0375ca4..cde4519a4 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -367,23 +367,17 @@ ClearMountsJob::prettyStatusMessage() const
 Calamares::JobResult
 ClearMountsJob::exec()
 {
-    CalamaresUtils::Partition::Syncer s;
-
     const QString deviceName = m_deviceNode.split( '/' ).last();
-
+    CalamaresUtils::Partition::Syncer s;
     QList< MessageAndPath > goodNews;
-    QProcess process;
-
-    const QStringList partitionsList = getPartitionsForDevice( deviceName );
-    const QStringList swapPartitions = getSwapsForDevice( m_deviceNode );
 
     apply( getCryptoDevices(), tryCryptoClose, goodNews );
     apply( getLVMVolumes(), tryUmount, goodNews );
     apply( getPVGroups( deviceName ), tryVGDisable, goodNews );
 
     apply( getCryptoDevices(), tryCryptoClose, goodNews );
-    apply( partitionsList, tryUmount, goodNews );
-    apply( swapPartitions, tryClearSwap, goodNews );
+    apply( getPartitionsForDevice( deviceName ), tryUmount, goodNews );
+    apply( getSwapsForDevice( m_deviceNode ), tryClearSwap, goodNews );
 
     Calamares::JobResult ok = Calamares::JobResult::ok();
     ok.setMessage( tr( "Cleared all mounts for %1" ).arg( m_deviceNode ) );
diff --git a/src/modules/partition/jobs/ClearMountsJob.h b/src/modules/partition/jobs/ClearMountsJob.h
index f0ef3e104..a49c07d3f 100644
--- a/src/modules/partition/jobs/ClearMountsJob.h
+++ b/src/modules/partition/jobs/ClearMountsJob.h
@@ -17,14 +17,21 @@ class Device;
 /**
  * This job tries to free all mounts for the given device, so partitioning
  * operations can proceed.
+ *
+ * - partitions on the device are unmounted
+ * - swap on the device is disabled and cleared
+ * - physical volumes for LVM on the device are disabled
+ *
+ * In addition, regardless of device:
+ * - all /dev/mapper entries (crypto / LUKS) are closed
+ * - all logical volumes for LVM are unmounted
+ *
  */
 class ClearMountsJob : public Calamares::Job
 {
     Q_OBJECT
 public:
     /** @brief Creates a job freeing mounts on @p device
-     *
-     * All /dev/mapper entries are closed, regardless of device.
      *
      * No ownership is transferred; the @p device is used only to access
      * the device node (name).

From 7b45793b60cf0bce74230a7f16194bbc4ad35d85 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 15:26:25 +0100
Subject: [PATCH 15/16] [partition] Allow exceptions when closing /dev/mapper

- some names should not be closed, like "control"
- allow a list of names to be added which should not be closed
---
 src/modules/partition/jobs/ClearMountsJob.cpp |  8 ++++----
 src/modules/partition/jobs/ClearMountsJob.h   | 12 +++++++++++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/modules/partition/jobs/ClearMountsJob.cpp b/src/modules/partition/jobs/ClearMountsJob.cpp
index cde4519a4..74a783d03 100644
--- a/src/modules/partition/jobs/ClearMountsJob.cpp
+++ b/src/modules/partition/jobs/ClearMountsJob.cpp
@@ -127,7 +127,7 @@ isFedoraSpecial( const QString& baseName )
  * the list.
  */
 STATICTEST QStringList
-getCryptoDevices()
+getCryptoDevices( const QStringList& mapperExceptions )
 {
     QDir mapperDir( "/dev/mapper" );
     const QFileInfoList fiList = mapperDir.entryInfoList( QDir::Files );
@@ -135,7 +135,7 @@ getCryptoDevices()
     for ( const QFileInfo& fi : fiList )
     {
         QString baseName = fi.baseName();
-        if ( isControl( baseName ) || isFedoraSpecial( baseName ) )
+        if ( isControl( baseName ) || isFedoraSpecial( baseName ) || mapperExceptions.contains( baseName ) )
         {
             continue;
         }
@@ -371,11 +371,11 @@ ClearMountsJob::exec()
     CalamaresUtils::Partition::Syncer s;
     QList< MessageAndPath > goodNews;
 
-    apply( getCryptoDevices(), tryCryptoClose, goodNews );
+    apply( getCryptoDevices( m_mapperExceptions ), tryCryptoClose, goodNews );
     apply( getLVMVolumes(), tryUmount, goodNews );
     apply( getPVGroups( deviceName ), tryVGDisable, goodNews );
 
-    apply( getCryptoDevices(), tryCryptoClose, goodNews );
+    apply( getCryptoDevices( m_mapperExceptions ), tryCryptoClose, goodNews );
     apply( getPartitionsForDevice( deviceName ), tryUmount, goodNews );
     apply( getSwapsForDevice( m_deviceNode ), tryClearSwap, goodNews );
 
diff --git a/src/modules/partition/jobs/ClearMountsJob.h b/src/modules/partition/jobs/ClearMountsJob.h
index a49c07d3f..fb3aca1e4 100644
--- a/src/modules/partition/jobs/ClearMountsJob.h
+++ b/src/modules/partition/jobs/ClearMountsJob.h
@@ -23,8 +23,14 @@ class Device;
  * - physical volumes for LVM on the device are disabled
  *
  * In addition, regardless of device:
- * - all /dev/mapper entries (crypto / LUKS) are closed
+ * - almost all(*) /dev/mapper entries (crypto / LUKS, also LVM) are closed
  * - all logical volumes for LVM are unmounted
+ * Exceptions to "all /dev/mapper" may be configured through
+ * the setMapperExceptions() method. Pass in names of mapper
+ * files that should not be closed (e.g. "myvg-mylv").
+ *
+ * (*) Some exceptions always exist: /dev/mapper/control is never
+ *     closed. /dev/mapper/live-* is never closed.
  *
  */
 class ClearMountsJob : public Calamares::Job
@@ -42,8 +48,12 @@ public:
     QString prettyStatusMessage() const override;
     Calamares::JobResult exec() override;
 
+    ///@brief Sets the list of exceptions (names) when closing /dev/mapper
+    void setMapperExceptions( const QStringList& names ) { m_mapperExceptions = names; }
+
 private:
     const QString m_deviceNode;
+    QStringList m_mapperExceptions;
 };
 
 #endif  // CLEARMOUNTSJOB_H

From b5dba9108c626ecc46b444f0b70e0d62a3e76bf0 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Nov 2021 15:33:34 +0100
Subject: [PATCH 16/16] [partition] Check for LVs that will be formatted, don't
 close them

- when (manually) using an existing LV, it shouldn't be closed
  prior to formatting, since that kills the volume and then the
  path (/dev/myvg/mylv) no longer exists. Then creating the
  filesysytem on that device path fails.
---
 .../partition/core/PartitionCoreModule.cpp    | 42 ++++++++++++++++++-
 .../partition/core/PartitionCoreModule.h      |  3 +-
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp
index 0f53ba413..f9a4706c4 100644
--- a/src/modules/partition/core/PartitionCoreModule.cpp
+++ b/src/modules/partition/core/PartitionCoreModule.cpp
@@ -580,6 +580,42 @@ PartitionCoreModule::setPartitionFlags( Device* device, Partition* partition, Pa
     PartitionInfo::setFlags( partition, flags );
 }
 
+STATICTEST QStringList
+findEssentialLVs( const QList< PartitionCoreModule::DeviceInfo* >& infos )
+{
+    QStringList doNotClose;
+    cDebug() << "Checking LVM use on" << infos.count() << "devices";
+    for ( const auto* info : infos )
+    {
+        if ( info->device->type() != Device::Type::LVM_Device )
+        {
+            continue;
+        }
+
+        for ( const auto& j : qAsConst( info->jobs() ) )
+        {
+            FormatPartitionJob* format = dynamic_cast< FormatPartitionJob* >( j.data() );
+            if ( format )
+            {
+                // device->deviceNode() is /dev/<vg name>
+                // partition()->partitionPath() is /dev/<vg name>/<lv>
+                const auto* partition = format->partition();
+                const QString partPath = partition->partitionPath();
+                const QString devicePath = info->device->deviceNode() + '/';
+                const bool isLvm = partition->roles().has( PartitionRole::Lvm_Lv );
+                if ( isLvm && partPath.startsWith( devicePath ) )
+                {
+                    cDebug() << Logger::SubEntry << partPath
+                             << "is an essential LV filesystem=" << partition->fileSystem().type();
+                    QString lvName = partPath.right( partPath.length() - devicePath.length() );
+                    doNotClose.append( info->device->name() + '-' + lvName );
+                }
+            }
+        }
+    }
+    return doNotClose;
+}
+
 Calamares::JobList
 PartitionCoreModule::jobs( const Config* config ) const
 {
@@ -604,11 +640,15 @@ PartitionCoreModule::jobs( const Config* config ) const
     lst << automountControl;
     lst << Calamares::job_ptr( new ClearTempMountsJob() );
 
+    const QStringList doNotClose = findEssentialLVs( m_deviceInfos );
+
     for ( const auto* info : m_deviceInfos )
     {
         if ( info->isDirty() )
         {
-            lst << Calamares::job_ptr( new ClearMountsJob( info->device.data() ) );
+            auto* job = new ClearMountsJob( info->device.data() );
+            job->setMapperExceptions( doNotClose );
+            lst << Calamares::job_ptr( job );
         }
     }
 
diff --git a/src/modules/partition/core/PartitionCoreModule.h b/src/modules/partition/core/PartitionCoreModule.h
index 693569310..eae16f0be 100644
--- a/src/modules/partition/core/PartitionCoreModule.h
+++ b/src/modules/partition/core/PartitionCoreModule.h
@@ -84,6 +84,8 @@ public:
         PartitionModel* partitionModelAfter;
     };
 
+    struct DeviceInfo;
+
     PartitionCoreModule( QObject* parent = nullptr );
     ~PartitionCoreModule() override;
 
@@ -239,7 +241,6 @@ Q_SIGNALS:
     void deviceReverted( Device* device );
 
 private:
-    struct DeviceInfo;
     void refreshAfterModelChange();
 
     void doInit();