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

Do not use fallocate in swap file creation on xfs. #70

Merged
merged 7 commits into from Jan 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 50 additions & 17 deletions cloudinit/config/cc_mounts.py
Expand Up @@ -223,42 +223,75 @@ def suggested_swapsize(memsize=None, maxsize=None, fsys=None):
return size


def create_swapfile(fname, size):
"""Size is in MiB."""

errmsg = "Failed to create swapfile '%s' of size %dMB via %s: %s"

Choose a reason for hiding this comment

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

This too should likely be a '%s' format modifier instead of '%d' since size is a string.


def create_swap(fname, size, method):
otubo marked this conversation as resolved.
Show resolved Hide resolved
LOG.debug("Creating swapfile in '%s' on fstype '%s' using '%s'",
fname, fstype, method)

if method == "fallocate":
cmd = ['fallocate', '-l', '%dM' % size, fname]

Choose a reason for hiding this comment

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

Size is a string, not a number, so '%s' format specifier should be used.

elif method == "dd":
cmd = ['dd', 'if=/dev/zero', 'of=%s' % fname, 'bs=1M',
'count=%d' % size]

Choose a reason for hiding this comment

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

While I don't use xfs, I suspect this too should be using a '%s' format specifier since size is a string.


try:
util.subp(cmd, capture=True)
except util.ProcessExecutionError as e:
LOG.warning(errmsg, fname, size, method, e)
util.del_file(fname)

swap_dir = os.path.dirname(fname)
util.ensure_dir(swap_dir)

fstype = util.get_mount_info(swap_dir)[1]

if fstype in ("xfs", "btrfs"):
create_swap(fname, size, "dd")
else:
otubo marked this conversation as resolved.
Show resolved Hide resolved
try:
create_swap(fname, size, "fallocate")
except util.ProcessExecutionError as e:
LOG.warning(errmsg, fname, size, "dd", e)
LOG.warning("Will attempt with dd.")
create_swap(fname, size, "dd")

util.chmod(fname, 0o600)
try:
util.subp(['mkswap', fname])
except util.ProcessExecutionError:
util.del_file(fname)
raise


def setup_swapfile(fname, size=None, maxsize=None):
"""
fname: full path string of filename to setup
size: the size to create. set to "auto" for recommended
maxsize: the maximum size
"""
tdir = os.path.dirname(fname)
swap_dir = os.path.dirname(fname)
mibsize = str(int(size / (2 ** 20)))
if str(size).lower() == "auto":
try:
memsize = util.read_meminfo()['total']
except IOError:
LOG.debug("Not creating swap: failed to read meminfo")
return

util.ensure_dir(tdir)
size = suggested_swapsize(fsys=tdir, maxsize=maxsize,
util.ensure_dir(swap_dir)
size = suggested_swapsize(fsys=swap_dir, maxsize=maxsize,
memsize=memsize)

if not size:
LOG.debug("Not creating swap: suggested size was 0")
return

mbsize = str(int(size / (2 ** 20)))
msg = "creating swap file '%s' of %sMB" % (fname, mbsize)
try:
util.ensure_dir(tdir)
util.log_time(LOG.debug, msg, func=util.subp,
args=[['sh', '-c',
('rm -f "$1" && umask 0066 && '
otubo marked this conversation as resolved.
Show resolved Hide resolved
'{ fallocate -l "${2}M" "$1" || '
'dd if=/dev/zero "of=$1" bs=1M "count=$2"; } && '
'mkswap "$1" || { r=$?; rm -f "$1"; exit $r; }'),
'setup_swap', fname, mbsize]])

except Exception as e:
raise IOError("Failed %s: %s" % (msg, e))
util.log_time(LOG.debug, msg="Setting up swap file", func=create_swapfile,
args=[fname, mibsize])

return fname

Expand Down
12 changes: 12 additions & 0 deletions tests/unittests/test_handler/test_handler_mounts.py
Expand Up @@ -181,6 +181,18 @@ def device_name_to_device(self, path):

return dev

def test_swap_integrity(self):
'''Ensure that the swap file is correctly created and can
swapon successfully. Fixing the corner case of:
kernel: swapon: swapfile has holes'''

fstab = '/swap.img swap swap defaults 0 0\n'

with open(cc_mounts.FSTAB_PATH, 'w') as fd:
fd.write(fstab)
cc = {'swap': ['filename: /swap.img', 'size: 512', 'maxsize: 512']}
cc_mounts.handle(None, cc, self.mock_cloud, self.mock_log, [])

def test_fstab_no_swap_device(self):
'''Ensure that cloud-init adds a discovered swap partition
to /etc/fstab.'''
Expand Down