From f1c1d07dcaaa9d3e3037eaa5dec2d5f7f8fdb8b2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Nov 2018 11:57:49 -0400 Subject: [PATCH 1/5] [partition] Add convenience methods to FstabEntry - Add something like a constructor - Add validity checking --- src/modules/partition/core/OsproberEntry.h | 13 ++++++++++ src/modules/partition/core/PartUtils.cpp | 28 ++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/modules/partition/core/OsproberEntry.h b/src/modules/partition/core/OsproberEntry.h index 792f22b29..042f6ee9f 100644 --- a/src/modules/partition/core/OsproberEntry.h +++ b/src/modules/partition/core/OsproberEntry.h @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014-2016, Teo Mrnjavac + * Copyright 2018, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -29,6 +30,18 @@ struct FstabEntry QString options; int dump; int pass; + + /// Does this entry make sense and is it complete? + bool isValid() const; // implemented in Partutils.cpp + + /** @brief Create an entry from a live of /etc/fstab + * + * Splits the given string (which ought to follow the format + * of /etc/fstab) and returns a corresponding Fstab entry. + * If the string isn't valid (e.g. comment-line, or broken + * fstab entry) then the entry that is returned is invalid. + */ + FstabEntry fromEtcFstab( const QString& ); // implemented in Partutils.cpp }; typedef QList< FstabEntry > FstabEntryList; diff --git a/src/modules/partition/core/PartUtils.cpp b/src/modules/partition/core/PartUtils.cpp index 12aa489db..a6a607d94 100644 --- a/src/modules/partition/core/PartUtils.cpp +++ b/src/modules/partition/core/PartUtils.cpp @@ -372,3 +372,31 @@ isEfiBootable( const Partition* candidate ) } } // nmamespace PartUtils + +/* Implementation of methods for FstabEntry, from OsproberEntry.h */ + +bool +FstabEntry::isValid() const +{ + return !partitionNode.isEmpty() && !mountPoint.isEmpty() && !fsType.isEmpty(); +} + +FstabEntry +FstabEntry::fromEtcFstab( const QString& rawLine ) +{ + QString line = rawLine.simplified(); + if ( line.startsWith( '#' ) ) + return FstabEntry{ QString(), QString(), QString(), QString(), 0, 0 }; + + QStringList splitLine = line.split( ' ' ); + if ( splitLine.length() != 6 ) + return FstabEntry{ QString(), QString(), QString(), QString(), 0, 0 }; + + return FstabEntry{ splitLine.at( 0 ), // path, or UUID, or LABEL, etc. + splitLine.at( 1 ), // mount point + splitLine.at( 2 ), // fs type + splitLine.at( 3 ), // options + splitLine.at( 4 ).toInt(), //dump + splitLine.at( 5 ).toInt() //pass + }; + } From bd1b48224280676189d3593d6f866a0076ff1f64 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Nov 2018 11:58:49 -0400 Subject: [PATCH 2/5] [partition] Don't autoremove the tempdir - Dangerout since we're mounting things inside that tempdir, and then doing a "weak" unmount --- src/modules/partition/core/PartUtils.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/partition/core/PartUtils.cpp b/src/modules/partition/core/PartUtils.cpp index a6a607d94..0423e1dde 100644 --- a/src/modules/partition/core/PartUtils.cpp +++ b/src/modules/partition/core/PartUtils.cpp @@ -163,6 +163,7 @@ lookForFstabEntries( const QString& partitionPath ) { FstabEntryList fstabEntries; QTemporaryDir mountsDir; + mountsDir.setAutoRemove( false ); int exit = QProcess::execute( "mount", { partitionPath, mountsDir.path() } ); if ( !exit ) // if all is well From 516ae494bf07dc8ebcecaab008f1120813869273 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Nov 2018 12:04:09 -0400 Subject: [PATCH 3/5] [partition] Complain if unmount fails - If unmount fails, then warn and don't autoremove --- src/modules/partition/core/PartUtils.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/modules/partition/core/PartUtils.cpp b/src/modules/partition/core/PartUtils.cpp index 0423e1dde..e9d15ebb6 100644 --- a/src/modules/partition/core/PartUtils.cpp +++ b/src/modules/partition/core/PartUtils.cpp @@ -196,7 +196,12 @@ lookForFstabEntries( const QString& partitionPath ) fstabFile.close(); } - QProcess::execute( "umount", { "-R", mountsDir.path() } ); + if ( QProcess::execute( "umount", { "-R", mountsDir.path() } ) ) + { + cWarning() << "Could not unmount" << mountsDir.path(); + // There is stuff left in there, really don't remove + mountsDir.setAutoRemove( false ); + } } return fstabEntries; From ebbc1a1bcb78f1c3f99911f39633322eabfd5013 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Nov 2018 12:13:06 -0400 Subject: [PATCH 4/5] [partition] "constructor-like" needs to be static --- src/modules/partition/core/OsproberEntry.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/core/OsproberEntry.h b/src/modules/partition/core/OsproberEntry.h index 042f6ee9f..e8c7895f0 100644 --- a/src/modules/partition/core/OsproberEntry.h +++ b/src/modules/partition/core/OsproberEntry.h @@ -41,7 +41,7 @@ struct FstabEntry * If the string isn't valid (e.g. comment-line, or broken * fstab entry) then the entry that is returned is invalid. */ - FstabEntry fromEtcFstab( const QString& ); // implemented in Partutils.cpp + static FstabEntry fromEtcFstab( const QString& ); // implemented in Partutils.cpp }; typedef QList< FstabEntry > FstabEntryList; From 6de55e6951fd0956bcbd44b60714dad686132050 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Nov 2018 12:13:29 -0400 Subject: [PATCH 5/5] [partition] Construct, then winnow, the fstab entries --- src/modules/partition/core/PartUtils.cpp | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/src/modules/partition/core/PartUtils.cpp b/src/modules/partition/core/PartUtils.cpp index e9d15ebb6..e696a0569 100644 --- a/src/modules/partition/core/PartUtils.cpp +++ b/src/modules/partition/core/PartUtils.cpp @@ -175,25 +175,9 @@ lookForFstabEntries( const QString& partitionPath ) .split( '\n' ); for ( const QString& rawLine : fstabLines ) - { - QString line = rawLine.simplified(); - if ( line.startsWith( '#' ) ) - continue; - - QStringList splitLine = line.split( ' ' ); - if ( splitLine.length() != 6 ) - continue; - - fstabEntries.append( { splitLine.at( 0 ), // path, or UUID, or LABEL, etc. - splitLine.at( 1 ), // mount point - splitLine.at( 2 ), // fs type - splitLine.at( 3 ), // options - splitLine.at( 4 ).toInt(), //dump - splitLine.at( 5 ).toInt() //pass - } ); - } - + fstabEntries.append( FstabEntry::fromEtcFstab( rawLine ) ); fstabFile.close(); + std::remove_if( fstabEntries.begin(), fstabEntries.end(), [](const FstabEntry& x) { return !x.isValid(); } ); } if ( QProcess::execute( "umount", { "-R", mountsDir.path() } ) )