From 14e2a8a21200099c6c3c915e1204a58ca8d4a785 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 6 Feb 2019 04:06:08 -0500 Subject: [PATCH 01/12] Changes: fix typo's and phrasing --- CHANGES | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 205727ff8..b165c98b7 100644 --- a/CHANGES +++ b/CHANGES @@ -47,7 +47,7 @@ This release contains contributions from (alphabetically by first name): * The currently-selected disk device is remembered between manual partitioning and the partitioning-overview pages. (Thanks to Arnaud) * *partition* There is new support for partitioning layout presets. - See `partitionc.conf` for documentation and details. + See `partition.conf` for documentation and details. * The *keyboard* module now handles the (bogus) Austrian keymap for the system console properly. (Thanks to Kevin) * The *preservefiles* module now has a mechanism for setting the permissions @@ -56,7 +56,8 @@ This release contains contributions from (alphabetically by first name): for use in OEM installs where an image of fixed size is created, and then sized to the actual SD card the user has used. * The *mount* module now handles missing *extraMounts* and *extraMountsEfi* - keys gracefully (this is probably a misconfiguration issue). + keys gracefully (this is probably a misconfiguration, though, and gives a + warning). * The *packages* module now supports pre- and post-script options for all operations, not just during install (keep in mind that these run as three separate shells, though). From 2c6ff26aaa52c1d0e303511188f642cfa9bfc9b2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 6 Feb 2019 04:08:21 -0500 Subject: [PATCH 02/12] [partition] Reduce warnings --- src/modules/partition/gui/ChoicePage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 1ef1a00e5..88659f3ef 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -107,9 +107,9 @@ ChoicePage::ChoicePage( const SwapChoiceSet& swapChoices, QWidget* parent ) , m_bootloaderComboBox( nullptr ) , m_lastSelectedDeviceIndex( -1 ) , m_enableEncryptionWidget( true ) - , m_allowManualPartitioning( true ) , m_availableSwapChoices( swapChoices ) , m_eraseSwapChoice( pickOne( swapChoices ) ) + , m_allowManualPartitioning( true ) { setupUi( this ); From 273461a497d0b8e137ff6af6614efe0a969f5779 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 05:32:42 -0500 Subject: [PATCH 03/12] [partition] Be verbose about handling osprober results --- src/modules/partition/gui/ChoicePage.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 88659f3ef..10ffbcee3 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -1246,6 +1246,7 @@ ChoicePage::setupActions() if ( osproberEntriesForCurrentDevice.count() == 0 ) { CALAMARES_RETRANSLATE( + cDebug() << "Setting texts for 0 osprober entries"; m_messageLabel->setText( tr( "This storage device does not seem to have an operating system on it. " "What would you like to do?
" "You will be able to review and confirm your choices " @@ -1278,6 +1279,7 @@ ChoicePage::setupActions() if ( !osName.isEmpty() ) { CALAMARES_RETRANSLATE( + cDebug() << "Setting texts for 1 non-empty osprober entry"; m_messageLabel->setText( tr( "This storage device has %1 on it. " "What would you like to do?
" "You will be able to review and confirm your choices " @@ -1301,6 +1303,7 @@ ChoicePage::setupActions() else { CALAMARES_RETRANSLATE( + cDebug() << "Setting texts for 1 empty osprober entry"; m_messageLabel->setText( tr( "This storage device already has an operating system on it. " "What would you like to do?
" "You will be able to review and confirm your choices " @@ -1325,6 +1328,8 @@ ChoicePage::setupActions() // osproberEntriesForCurrentDevice has at least 2 items. CALAMARES_RETRANSLATE( + cDebug() << "Setting texts for >= 2 osprober entries"; + m_messageLabel->setText( tr( "This storage device has multiple operating systems on it. " "What would you like to do?
" "You will be able to review and confirm your choices " From 2fda5957f1139772961e28f191ea52e4641ce998 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 09:06:27 -0500 Subject: [PATCH 04/12] [partition] Complain about unsupported swap choices --- src/modules/partition/gui/PartitionViewStep.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index 503924860..02af39bfa 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -614,6 +614,17 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) } m_swapChoices = choices; + // Not all are supported right now // FIXME + static const char unsupportedSetting[] = "Partition-module does not support *userSwapChoices* setting"; + +#define COMPLAIN_UNSUPPORTED(x) \ + if ( choices.contains( x ) ) \ + { cWarning() << unsupportedSetting << PartitionActions::Choices::choiceToName( x ); } + + COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::SwapFile ) + COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::ReuseSwap ) +#undef COMPLAIN_UNSUPPORTED + // These gs settings seem to be unused (in upstream Calamares) outside of // the partition module itself. gs->insert( "ensureSuspendToDisk", ensureSuspendToDisk ); From 0fdc73796897dae348cda5096a23795c5478435b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 09:18:21 -0500 Subject: [PATCH 05/12] [partition] Fix logging output - Using the assignment-operator just generates blank lines. - Using QLog with a log-level avoids the cDebug()-style special handling of warnings and errors (useless here, but may as well fix code style). --- src/modules/partition/gui/PartitionViewStep.cpp | 2 +- src/modules/partition/jobs/FillGlobalStorageJob.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index 02af39bfa..ac0599de7 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -538,7 +538,7 @@ findFS( QString defaultFS ) // This bit is for distro's debugging their settings, and shows // all the strings that KPMCore is matching against for FS type. { - Logger::CLog d( Logger::LOGDEBUG ); + Logger::CDebug d; using TR = Logger::DebugRow< int, QString >; const auto fstypes = FileSystem::types(); d << "Available types (" << fstypes.count() << ')'; diff --git a/src/modules/partition/jobs/FillGlobalStorageJob.cpp b/src/modules/partition/jobs/FillGlobalStorageJob.cpp index 597d62a82..1f4026dec 100644 --- a/src/modules/partition/jobs/FillGlobalStorageJob.cpp +++ b/src/modules/partition/jobs/FillGlobalStorageJob.cpp @@ -96,7 +96,7 @@ mapForPartition( Partition* partition, const QString& uuid ) // Debugging for inside the loop in createPartitionList(), // so indent a bit - Logger::CLog deb = cDebug(); + Logger::CDebug deb; using TR = Logger::DebugRow; deb << " .. mapping for" << partition->partitionPath() << partition->deviceNode() << TR( "mtpoint:", PartitionInfo::mountPoint( partition ) ) From 04b4e37bd0a1bdac0a04d4050c25f28227f1fd36 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 09:23:50 -0500 Subject: [PATCH 06/12] [partition] Don't display unsupported swap styles - Suppress unsupported options while reading the config file. --- src/modules/partition/gui/PartitionViewStep.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index ac0599de7..b7433450d 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -612,19 +612,20 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) else choices.insert( PartitionActions::Choices::SwapChoice::SmallSwap ); } - m_swapChoices = choices; // Not all are supported right now // FIXME static const char unsupportedSetting[] = "Partition-module does not support *userSwapChoices* setting"; #define COMPLAIN_UNSUPPORTED(x) \ if ( choices.contains( x ) ) \ - { cWarning() << unsupportedSetting << PartitionActions::Choices::choiceToName( x ); } + { cWarning() << unsupportedSetting << PartitionActions::Choices::choiceToName( x ); choices.remove( x ); } COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::SwapFile ) COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::ReuseSwap ) #undef COMPLAIN_UNSUPPORTED + m_swapChoices = choices; + // These gs settings seem to be unused (in upstream Calamares) outside of // the partition module itself. gs->insert( "ensureSuspendToDisk", ensureSuspendToDisk ); From abf1f1460453051301c94683ef82d9b34f88c0b0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 17:14:39 -0500 Subject: [PATCH 07/12] [partition] Initialize members of PartitionEntry --- src/modules/partition/core/PartitionLayout.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/modules/partition/core/PartitionLayout.h b/src/modules/partition/core/PartitionLayout.h index 5ec65cf22..8c17c3a9f 100644 --- a/src/modules/partition/core/PartitionLayout.h +++ b/src/modules/partition/core/PartitionLayout.h @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2018, Collabora Ltd + * Copyright 2019, 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 @@ -47,11 +48,11 @@ public: { QString partLabel; QString partMountPoint; - int partFileSystem; - double partSize; - SizeUnit partSizeUnit; - double partMinSize; - SizeUnit partMinSizeUnit; + int partFileSystem = 0; + double partSize = 0.0L; + SizeUnit partSizeUnit = Percent; + double partMinSize = 0.0L; + SizeUnit partMinSizeUnit = Percent; }; PartitionLayout(); From 5863300f67e1dc043a19201fafdf9e30dbcdcd97 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 17:27:45 -0500 Subject: [PATCH 08/12] [partition] Use const QString& - minor code-layout and idiomatic-C++ --- src/modules/partition/core/PartitionLayout.cpp | 6 +++--- src/modules/partition/core/PartitionLayout.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp index f8ff79d2c..4f3225e94 100644 --- a/src/modules/partition/core/PartitionLayout.cpp +++ b/src/modules/partition/core/PartitionLayout.cpp @@ -53,7 +53,7 @@ PartitionLayout::addEntry( PartitionLayout::PartitionEntry entry ) } static double -parseSizeString( QString sizeString, PartitionLayout::SizeUnit *unit ) +parseSizeString( const QString& sizeString, PartitionLayout::SizeUnit* unit ) { double value; bool ok; @@ -103,7 +103,7 @@ parseSizeString( QString sizeString, PartitionLayout::SizeUnit *unit ) } void -PartitionLayout::addEntry( QString mountPoint, QString size, QString min ) +PartitionLayout::addEntry( const QString& mountPoint, const QString& size, const QString& min ) { PartitionLayout::PartitionEntry entry; @@ -119,7 +119,7 @@ PartitionLayout::addEntry( QString mountPoint, QString size, QString min ) } void -PartitionLayout::addEntry( QString label, QString mountPoint, QString fs, QString size, QString min ) +PartitionLayout::addEntry( const QString& label, const QString& mountPoint, const QString& fs, const QString& size, const QString& min ) { PartitionLayout::PartitionEntry entry; diff --git a/src/modules/partition/core/PartitionLayout.h b/src/modules/partition/core/PartitionLayout.h index 8c17c3a9f..65ed1d850 100644 --- a/src/modules/partition/core/PartitionLayout.h +++ b/src/modules/partition/core/PartitionLayout.h @@ -61,8 +61,8 @@ public: ~PartitionLayout(); void addEntry( PartitionEntry entry ); - void addEntry( QString mountPoint, QString size, QString min = "" ); - void addEntry( QString label, QString mountPoint, QString fs, QString size, QString min = "" ); + void addEntry( const QString& mountPoint, const QString& size, const QString& min = QString() ); + void addEntry( const QString& label, const QString& mountPoint, const QString& fs, const QString& size, const QString& min = QString() ); /** * @brief Apply the current partition layout to the selected drive space. From a6edb3ed342ed44adb82eb28f537ed95fadb34ed Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 17:37:14 -0500 Subject: [PATCH 09/12] [partition] Refactor PartitionEntry - add a constructor that parses size and min - minor reduction in code duplication --- .../partition/core/PartitionLayout.cpp | 21 ++++++++----------- src/modules/partition/core/PartitionLayout.h | 5 +++++ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp index 4f3225e94..c2489620f 100644 --- a/src/modules/partition/core/PartitionLayout.cpp +++ b/src/modules/partition/core/PartitionLayout.cpp @@ -102,18 +102,20 @@ parseSizeString( const QString& sizeString, PartitionLayout::SizeUnit* unit ) return value; } +PartitionLayout::PartitionEntry::PartitionEntry(const QString& size, const QString& min) +{ + partSize = parseSizeString( size , &partSizeUnit ); + if ( !min.isEmpty() ) + partMinSize = parseSizeString( min , &partMinSizeUnit ); +} + void PartitionLayout::addEntry( const QString& mountPoint, const QString& size, const QString& min ) { - PartitionLayout::PartitionEntry entry; + PartitionLayout::PartitionEntry entry( size, min ); entry.partMountPoint = mountPoint; entry.partFileSystem = FileSystem::Ext4; - entry.partSize = parseSizeString( size , &entry.partSizeUnit ); - if (min.isEmpty()) - entry.partMinSize = 0; - else - entry.partMinSize = parseSizeString( min , &entry.partMinSizeUnit ); partLayout.append( entry ); } @@ -121,16 +123,11 @@ PartitionLayout::addEntry( const QString& mountPoint, const QString& size, const void PartitionLayout::addEntry( const QString& label, const QString& mountPoint, const QString& fs, const QString& size, const QString& min ) { - PartitionLayout::PartitionEntry entry; + PartitionLayout::PartitionEntry entry( size, min ); entry.partLabel = label; entry.partMountPoint = mountPoint; entry.partFileSystem = FileSystem::typeForName( fs ); - entry.partSize = parseSizeString( size , &entry.partSizeUnit ); - if (min.isEmpty()) - entry.partMinSize = 0; - else - entry.partMinSize = parseSizeString( min , &entry.partMinSizeUnit ); partLayout.append( entry ); } diff --git a/src/modules/partition/core/PartitionLayout.h b/src/modules/partition/core/PartitionLayout.h index 65ed1d850..5e216122c 100644 --- a/src/modules/partition/core/PartitionLayout.h +++ b/src/modules/partition/core/PartitionLayout.h @@ -53,6 +53,11 @@ public: SizeUnit partSizeUnit = Percent; double partMinSize = 0.0L; SizeUnit partMinSizeUnit = Percent; + + /// @brief All-zeroes PartitionEntry + PartitionEntry() {}; + /// @brief Parse @p size and @p min to their respective member variables + PartitionEntry( const QString& size, const QString& min ); }; PartitionLayout(); From 6316173f1ba87aede1a1c872b9997a6919374c30 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 17:45:23 -0500 Subject: [PATCH 10/12] [partition] More conservative handling of device pointers - thanks @abucodonosor --- src/modules/partition/gui/PartitionPage.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index 86de992e6..2743ec53a 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -165,9 +165,13 @@ PartitionPage::updateButtons() if ( m_ui->deviceComboBox->currentIndex() >= 0 ) { + Device* device = nullptr; QModelIndex deviceIndex = m_core->deviceModel()->index( m_ui->deviceComboBox->currentIndex(), 0 ); - auto device = m_core->deviceModel()->deviceForIndex( deviceIndex ); - if ( device->type() != Device::Type::LVM_Device ) + if ( deviceIndex.isValid() ) + device = m_core->deviceModel()->deviceForIndex( deviceIndex ); + if ( !device ) + cWarning() << "Device for updateButtons is nullptr"; + else if ( device->type() != Device::Type::LVM_Device ) { createTable = true; From b9fa0398c0d286d98869b0f07a6078ed8148c2cd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 17:58:43 -0500 Subject: [PATCH 11/12] [partition] Disable one call to updateButtons() - suggested by @abucodonosor, removing this one call seems to solve the data race for the device model. --- src/modules/partition/gui/PartitionPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index 2743ec53a..a2f2eab60 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -581,7 +581,7 @@ void PartitionPage::onPartitionModelReset() { m_ui->partitionTreeView->expandAll(); - updateButtons(); + // updateButtons(); updateBootLoaderIndex(); } From dff5afe2275bd7c2bc1eca3ce216ff4b4866fb0c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 Feb 2019 19:03:09 -0500 Subject: [PATCH 12/12] [partition] Reduce refreshes when reverting --- src/modules/partition/core/PartitionCoreModule.cpp | 9 +++++---- src/modules/partition/core/PartitionCoreModule.h | 7 ++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index 16e7c7f34..07cb0fcfd 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -880,7 +880,7 @@ PartitionCoreModule::revertAllDevices() } } - revertDevice( ( *it )->device.data() ); + revertDevice( ( *it )->device.data(), false ); ++it; } @@ -889,7 +889,7 @@ PartitionCoreModule::revertAllDevices() void -PartitionCoreModule::revertDevice( Device* dev ) +PartitionCoreModule::revertDevice( Device* dev, bool individualRevert ) { QMutexLocker locker( &m_revertMutex ); DeviceInfo* devInfo = infoForDevice( dev ); @@ -915,7 +915,8 @@ PartitionCoreModule::revertDevice( Device* dev ) m_bootLoaderModel->init( devices ); - refreshAfterModelChange(); + if ( individualRevert ) + refreshAfterModelChange(); emit deviceReverted( newDev ); } @@ -931,7 +932,7 @@ PartitionCoreModule::asyncRevertDevice( Device* dev, std::function< void() > cal watcher->deleteLater(); } ); - QFuture< void > future = QtConcurrent::run( this, &PartitionCoreModule::revertDevice, dev ); + QFuture< void > future = QtConcurrent::run( this, &PartitionCoreModule::revertDevice, dev, true ); watcher->setFuture( future ); } diff --git a/src/modules/partition/core/PartitionCoreModule.h b/src/modules/partition/core/PartitionCoreModule.h index 52cb47a59..c48c1562c 100644 --- a/src/modules/partition/core/PartitionCoreModule.h +++ b/src/modules/partition/core/PartitionCoreModule.h @@ -191,7 +191,12 @@ public: void revert(); // full revert, thread safe, calls doInit void revertAllDevices(); // convenience function, calls revertDevice - void revertDevice( Device* dev ); // rescans a single Device and updates DeviceInfo + /** @brief rescans a single Device and updates DeviceInfo + * + * When @p individualRevert is true, calls refreshAfterModelChange(), + * used to reduce number of refreshes when calling revertAllDevices(). + */ + void revertDevice( Device* dev, bool individualRevert=true ); void asyncRevertDevice( Device* dev, std::function< void() > callback ); //like revertDevice, but asynchronous void clearJobs(); // only clear jobs, the Device* states are preserved