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

Add swap support. #109

Merged
merged 1 commit into from
Jun 22, 2015
Merged

Add swap support. #109

merged 1 commit into from
Jun 22, 2015

Conversation

ckoenig
Copy link
Contributor

@ckoenig ckoenig commented May 19, 2015

This pull request is based on #57 / #89, which wasn't merged.

It would be great, if you could have another look at this pull request.

@underscorgan
Copy link
Contributor

@ckoenig thanks for the contribution! Could you please squash this down to a single commit?

@ckoenig
Copy link
Contributor Author

ckoenig commented Jun 18, 2015

No problem, now as a single commit.

exec { "swapon for '${mount_title}'":
path => [ '/bin', '/usr/bin', '/sbin' ],
command => "swapon ${lvm_device_path}",
unless => "grep `readlink -f ${lvm_device_path}` /proc/swaps",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use swapon -s | grep ${mount_title} for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, sorry, but that doesn't really work in my case. It looks like this:

  • ${lvm_device_path} = ${mount_title} = /dev/vg0/swap, which is a link to /dev/dm-2
  • The output of swapon -s lists /dev/mapper/vg0-swap, which is also a link to /dev/dm-2
  • The output of cat /proc/swaps lists /dev/dm-2

So in order to use the output of swapon -s we either have to extract the device and follow its redirect with readlink -f and do the same for ${lvm_device_path} to make the comparison work, or we adjust the way the device path is built. At the moment ${lvm_device_path} reflects the logic, which was used up until now and sets the path to /dev/${volume_group}/${name}. Instead of that we could probably use /dev/mapper/${volume_group}-${name}. That would make the comparison of the output of swapon -s easier.

Is there any specific problem with my approach, which doesn't occur with your proposed alternative?

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense 👍

underscorgan pushed a commit that referenced this pull request Jun 22, 2015
@underscorgan underscorgan merged commit 5b6b21d into puppetlabs:master Jun 22, 2015
@underscorgan
Copy link
Contributor

👍 thanks!

cegeka-jenkins pushed a commit to cegeka/puppet-lvm that referenced this pull request Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants