From c3524c07adf9574677fdffc062dd08425dcac819 Mon Sep 17 00:00:00 2001 From: dalto Date: Sat, 13 Nov 2021 13:43:26 -0600 Subject: [PATCH] [zfs] Ensure overlapping datasets don't get created and code cleanup --- src/modules/zfs/ZfsJob.cpp | 90 ++++++++++++++++++++++++++++---------- src/modules/zfs/ZfsJob.h | 35 +++++++++++++-- 2 files changed, 99 insertions(+), 26 deletions(-) diff --git a/src/modules/zfs/ZfsJob.cpp b/src/modules/zfs/ZfsJob.cpp index 26d96183c..d0367f5f6 100644 --- a/src/modules/zfs/ZfsJob.cpp +++ b/src/modules/zfs/ZfsJob.cpp @@ -32,6 +32,43 @@ ZfsJob::prettyName() const return tr( "Create ZFS pools and datasets" ); } +QString +ZfsJob::AlphaNumeric( QString input ) const +{ + return input.remove( QRegExp( "[^a-zA-Z\\d\\s]" ) ); +} + +void +ZfsJob::CollectMountpoints( const QVariantList& partitions ) +{ + m_mountpoints.empty(); + for ( const QVariant& partition : partitions ) + { + if ( partition.canConvert( QVariant::Map ) ) + { + QString mountpoint = partition.toMap().value( "mountPoint" ).toString(); + if ( !mountpoint.isEmpty() ) + { + m_mountpoints.append( mountpoint ); + } + } + } +} + +bool +ZfsJob::IsMountpointOverlapping( const QString& targetMountpoint ) const +{ + for ( const QString& mountpoint : m_mountpoints ) + { + if ( mountpoint != '/' && targetMountpoint.startsWith( mountpoint ) ) + { + return true; + } + } + return false; +} + + ZfsResult ZfsJob::CreateZpool( QString deviceName, QString poolName, QString poolOptions, bool encrypt, QString passphrase ) const { @@ -85,7 +122,7 @@ ZfsJob::CreateZpool( QString deviceName, QString poolName, QString poolOptions, Calamares::JobResult ZfsJob::exec() { - QList< QVariant > partitions; + QVariantList partitions; Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); if ( gs && gs->contains( "partitions" ) && gs->value( "partitions" ).canConvert( QVariant::List ) ) { @@ -132,27 +169,28 @@ ZfsJob::exec() continue; } + // If the partition doesn't have a mountpoint, skip it QString mountpoint = pMap[ "mountPoint" ].toString(); if ( mountpoint.isEmpty() ) { continue; } - // Build a poolname off the mountpoint, this is not ideal but should work until there is UI built for zfs + // Build a poolname off config pool name and the mountpoint, this is not ideal but should work until there is UI built for zfs QString poolName = m_poolName; if ( mountpoint != '/' ) { - QString suffix = mountpoint; - poolName += suffix.remove( QRegExp( "[^a-zA-Z\\d\\s]" ) ); + poolName += AlphaNumeric( mountpoint ); } + // Check to ensure the list of zfs info from the partition module is available and convert it to a list if ( !gs->contains( "zfsInfo" ) && gs->value( "zfsInfo" ).canConvert( QVariant::List ) ) { return Calamares::JobResult::error( tr( "Internal data missing" ), tr( "Failed to create zpool" ) ); } - QVariantList zfsInfoList = gs->value( "zfsInfo" ).toList(); + // Look in the zfs info list to see if this partition should be encrypted bool encrypt = false; QString passphrase; for ( const QVariant& zfsInfo : qAsConst( zfsInfoList ) ) @@ -165,11 +203,11 @@ ZfsJob::exec() } } + // Create the zpool ZfsResult zfsResult; if ( encrypt ) { - zfsResult - = CreateZpool( deviceName, poolName, m_poolOptions, true, passphrase ); + zfsResult = CreateZpool( deviceName, poolName, m_poolOptions, true, passphrase ); } else { @@ -181,16 +219,17 @@ ZfsJob::exec() return Calamares::JobResult::error( tr( "Failed to create zpool" ), zfsResult.failureMessage ); } - // Add the poolname, dataset name and mountpoint to the list + // Save the poolname, dataset name and mountpoint. It will later be added to a list and placed in global storage. + // This will be used by later modules including mount and umount QVariantMap poolNameEntry; - poolNameEntry["poolName"] = poolName; - poolNameEntry["mountpoint"] = mountpoint; - poolNameEntry["dsName"] = "none"; - + poolNameEntry[ "poolName" ] = poolName; + poolNameEntry[ "mountpoint" ] = mountpoint; + poolNameEntry[ "dsName" ] = "none"; + // If the mountpoint is /, create datasets per the config file. If not, create a single dataset mounted at the partitions mountpoint if ( mountpoint == '/' ) { - // Create the datasets + CollectMountpoints( partitions ); QVariantList datasetList; for ( const auto& dataset : qAsConst( m_datasets ) ) { @@ -204,6 +243,12 @@ ZfsJob::exec() continue; } + // We should skip this dataset if it conflicts with a permanent mountpoint + if ( IsMountpointOverlapping( datasetMap[ "mountpoint" ].toString() ) ) + { + continue; + } + // Create the dataset. We set canmount=no regardless of the setting for now. // It is modified to the correct value in the mount module to ensure mount order is maintained auto r = system->runCommand( { "sh", @@ -218,7 +263,8 @@ ZfsJob::exec() continue; } - // Add the dataset to the list for global storage + // Add the dataset to the list for global storage this information is used later to properly set + // the mount options on each dataset datasetMap[ "zpool" ] = m_poolName; datasetList.append( datasetMap ); } @@ -231,24 +277,23 @@ ZfsJob::exec() } else { - // Create the dataset. We set canmount=no regardless of the setting for now. + // This is a zpool with a single dataset We again set canmount=no regardless of the desired setting. // It is modified to the correct value in the mount module to ensure mount order is maintained QString dsName = mountpoint; - dsName.remove( QRegExp( "[^a-zA-Z\\d\\s]" ) ); + dsName = AlphaNumeric( mountpoint ); auto r = system->runCommand( { "sh", "-c", "zfs create " + m_datasetOptions + " -o canmount=off -o mountpoint=" - + mountpoint + " " + poolName + "/" - + dsName }, + + mountpoint + " " + poolName + "/" + dsName }, std::chrono::seconds( 10 ) ); if ( r.getExitCode() != 0 ) { cWarning() << "Failed to create dataset" << dsName; } - poolNameEntry["dsName"] = dsName; + poolNameEntry[ "dsName" ] = dsName; } - poolNames.append(poolNameEntry); + poolNames.append( poolNameEntry ); // Export the zpool so it can be reimported at the correct location later auto r = system->runCommand( { "zpool", "export", poolName }, std::chrono::seconds( 10 ) ); @@ -258,9 +303,10 @@ ZfsJob::exec() } } - if (!poolNames.isEmpty()) + // Put the list of zpools into global storage + if ( !poolNames.isEmpty() ) { - gs->insert("zfsPoolInfo", poolNames); + gs->insert( "zfsPoolInfo", poolNames ); } return Calamares::JobResult::ok(); diff --git a/src/modules/zfs/ZfsJob.h b/src/modules/zfs/ZfsJob.h index b2feb9e87..4744954c2 100644 --- a/src/modules/zfs/ZfsJob.h +++ b/src/modules/zfs/ZfsJob.h @@ -20,7 +20,8 @@ #include "DllMacro.h" -struct ZfsResult { +struct ZfsResult +{ bool success; QString failureMessage; }; @@ -46,12 +47,12 @@ private: QString m_poolName; QString m_poolOptions; QString m_datasetOptions; + QStringList m_mountpoints; - QList m_datasets; + QList< QVariant > m_datasets; /** @brief Creates a zpool based on the provided arguments * - * Creates a zpool * @p deviceName is a full path to the device the zpool should be created on * @p poolName is a string containing the name of the pool to create * @p poolOptions are the options to pass to zpool create @@ -59,9 +60,35 @@ private: * @p passphrase is a string continaing the passphrase * */ - ZfsResult CreateZpool(QString deviceName, QString poolName, QString poolOptions, bool encrypt, QString passphrase = QString() ) const; + ZfsResult CreateZpool( QString deviceName, + QString poolName, + QString poolOptions, + bool encrypt, + QString passphrase = QString() ) const; + /** @brief Returns the alphanumeric portion of a string + * + * @p input is the input string + * + */ + QString AlphaNumeric( QString input ) const; + /** @brief Collects all the mountpoints from the partitions + * + * Iterates over @p partitions to gather each mountpoint present + * in the list of maps and populates m_mountpoints + * + */ + void CollectMountpoints( const QVariantList& partitions ); + + /** @brief Check to see if a given mountpoint overlaps with one of the defined moutnpoints + * + * Iterates over m_partitions and checks if @p targetMountpoint overlaps with them by comparing + * the beginning of targetMountpoint with all the values in m_mountpoints. Of course, / is excluded + * since all the mountpoints would begin with / + * + */ + bool IsMountpointOverlapping( const QString& targetMountpoint ) const; }; CALAMARES_PLUGIN_FACTORY_DECLARATION( ZfsJobFactory )