Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

During installation, if a swap partition exists, enable swap. #2097

Closed
wants to merge 0 commits into from

Conversation

phoepsilonix
Copy link
Contributor

If there is no swap at all when the partition of the installation destination device is mounted, if a Swap partition exists, enable swap.

@dalto8
Copy link
Contributor

dalto8 commented Feb 13, 2023

I will put comments on this in a few minutes. However, please stop closing the PRs and creating new ones. You can continue to push changes to your PR. You don't need to create new ones each time you make changes.

@kkofler
Copy link
Contributor

kkofler commented Feb 13, 2023

In particular, you can amend your commit and force-push to your branch, and the pull request will automatically pick up the amended commit.

@phoepsilonix
Copy link
Contributor Author

I understand. Could you please teach me how to make changes, as I don't know how to do it?

Copy link
Contributor

@dalto8 dalto8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting closer now.

src/modules/mount/main.py Outdated Show resolved Hide resolved
src/modules/mount/main.py Outdated Show resolved Hide resolved
@@ -321,6 +327,12 @@ def run():
return (_("Configuration Error"),
_("No partitions are defined for <pre>{!s}</pre> to use.").format("mount"))

# swap
swap_partitions = [p['device'] for p in partitions if p['fs'] == 'linuxswap']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to ensure the swap partition is "claimed". We shouldn't enable an unclaimed partition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you ask?
It seems that in some cases the swap partition may not be recognized even though I manually configure the partition and expect it to be recognized as claimed false.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "claimed" flag should be set if we are using it with Calamares.

We can't use other swap partitions because we don't know what is in them. For example, it might hold resume data for another distro.

src/modules/mount/main.py Outdated Show resolved Hide resolved
src/modules/mount/main.py Outdated Show resolved Hide resolved
@@ -307,6 +307,12 @@ def mount_partition(root_mount_point, partition, partitions, mount_options, moun
mount_option) != 0:
libcalamares.utils.warning("Cannot mount {}".format(device))

def enable_swap_partition(swap_device):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably need to handle the condition where the swap partition is encrypted.

@dalto8
Copy link
Contributor

dalto8 commented Feb 13, 2023

I understand. Could you please teach me how to make changes, as I don't know how to do it?

Whenever you update your branch, the PR is automatically updated.

src/modules/mount/main.py Outdated Show resolved Hide resolved
src/modules/mount/main.py Outdated Show resolved Hide resolved
@@ -321,6 +327,11 @@ def run():
return (_("Configuration Error"),
_("No partitions are defined for <pre>{!s}</pre> to use.").format("mount"))

# swap
swap_partitions = [p['device'] for p in partitions if p['fs'] == 'linuxswap' and p['claimed'] == False ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't claimed be True here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"As far as I can see from checking the log output, I believe False is the correct answer."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that in case of manual configuration it is False, in case of automatic configuration it will beTrue as well.

@phoepsilonix phoepsilonix force-pushed the calamares branch 3 times, most recently from 3d4c1ab to 6a01e2f Compare February 13, 2023 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants