Merge ~adobrawy/cloud-init:swap-linux into cloud-init:master

Proposed by Adam Dobrawy
Status: Rejected
Rejected by: Chad Smith
Proposed branch: ~adobrawy/cloud-init:swap-linux
Merge into: cloud-init:master
Diff against target: 27 lines (+10/-3)
1 file modified
cloudinit/config/cc_mounts.py (+10/-3)
Reviewer Review Type Date Requested Status
Chad Smith Needs Information
Review via email: mp+354680@code.launchpad.net

Commit message

Do not use fallocate in swap file creation on xfs.

When creating a swap file on an xfs filesystem, fallocate cannot be used.
Doing so results in failure of swapon and a message like:
 swapon: swapfile has holes

The solution here is to maintain a list (currently containing only XFS)
of filesystems where fallocate cannot be used. The, on those fileystems
use the slower but functional 'dd' method.

Description of the change

xfs, which is default on CentOS7 doesn't handle fallocate correctly when used with swapfiles.
Therefore we need to check if the FS is fallocate-safe. Not sure if other FS' are affected.
https://unix.stackexchange.com/questions/294600/i-cant-enable-swap-space-on-centos-7#294605

This is a friendly transfer of the pull-request originally reported on GitHub: https://github.com/cloud-init/cloud-init/pull/11
I work with Krzysztof Biernat and I have obtained permission from him for such an operation.

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

Hi,
Thank you for contributing to cloud-init.

To contribute, you must sign the Canonical Contributor License Agreement (CLA) [1].

If you have already signed it as an individual, your Launchpad user will be listed in the contributor-agreement-canonical launchpad group [2]. Unfortunately there is no easy way to check if an organization or company you are doing work for has signed. If you are unsure or have questions, email <email address hidden> or ping smoser in #cloud-init channel via freenode.

For information on how to sign, please see the HACKING document [3].

Thanks again, and please feel free to reach out with any questions.


[1] http://www.canonical.com/contributors
[2] https://launchpad.net/~contributor-agreement-canonical/+members
[3] http://cloudinit.readthedocs.io/en/latest/topics/hacking.html

Revision history for this message
Scott Moser (smoser) wrote :

Please add some sort of unit test to ensure we do not break this in the future.

Revision history for this message
Chad Smith (chad.smith) wrote :

Thanks again for the branch! So that it is temporarily off our radar until you would like us to re-review it, I'm going to update this branch as 'Work in progress'.

Please set this branch Status back to 'Needs review' at the top when the following is addressed:
  - unit test(s) in tests/unittests/test_handler/test_handler_mounts.py
  - a comment to the affect that you have been able to sign the CLA

Again thank you for this contribution. we look forward to pulling it into cloud-init.

review: Needs Information
Revision history for this message
Adam Dobrawy (adobrawy) wrote :

I'm afraid I can not add the required unit tests. However, we have signed the required agreement so that you can use this code further.

~adobrawy/cloud-init:swap-linux updated
435c907... by Adam Dobrawy <email address hidden>

Fix flake8 issue

Revision history for this message
Scott Moser (smoser) wrote :

http://paste.ubuntu.com/p/8pVhczVqG3/

I put that together... thats why I do the sort of "shell blobs" that I do.
Much longer with "proper python".

And no tests added there.

Also note that create_swapfile logic is also in curtin and the bug that I linked.

Revision history for this message
Chad Smith (chad.smith) wrote :
Revision history for this message
Chad Smith (chad.smith) wrote :

Thanks for the original submit Adam!

Unmerged commits

435c907... by Adam Dobrawy <email address hidden>

Fix flake8 issue

124b107... by Adam Dobrawy <email address hidden>

Check if filesystem is safe for fallocate

Co-Author: Krzysztof Biernat (<email address hidden>)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/config/cc_mounts.py b/cloudinit/config/cc_mounts.py
2index 339baba..c9c0cc1 100644
3--- a/cloudinit/config/cc_mounts.py
4+++ b/cloudinit/config/cc_mounts.py
5@@ -249,12 +249,19 @@ def setup_swapfile(fname, size=None, maxsize=None):
6 msg = "creating swap file '%s' of %sMB" % (fname, mbsize)
7 try:
8 util.ensure_dir(tdir)
9+ # Check if filesystem is safe for fallocate
10+ fname_fs_type = util.get_mount_info(fname)[1]
11+ if fname_fs_type in ['xfs']:
12+ create_swapfile_command = 'dd if=/dev/zero "of=$1" bs=1M "count=$2"'
13+ else:
14+ create_swapfile_command = 'fallocate -l "${2}M" "$1"'
15+
16 util.log_time(LOG.debug, msg, func=util.subp,
17 args=[['sh', '-c',
18 ('rm -f "$1" && umask 0066 && '
19- '{ fallocate -l "${2}M" "$1" || '
20- ' dd if=/dev/zero "of=$1" bs=1M "count=$2"; } && '
21- 'mkswap "$1" || { r=$?; rm -f "$1"; exit $r; }'),
22+ '%s && '
23+ 'mkswap "$1" || { r=$?; rm -f "$1"; exit $r; }' %
24+ create_swapfile_command),
25 'setup_swap', fname, mbsize]])
26
27 except Exception as e:

Subscribers

People subscribed via source and target branches