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

WIP: cc_resizefs: Fix accepted opts for FreeBSD newfs #636

Closed
wants to merge 1 commit into from

Conversation

igalic
Copy link
Collaborator

@igalic igalic commented Oct 29, 2020

Proposed Commit Message

cc_resizefs: Fix accepted opts for FreeBSD newfs

On FreeBSD , if a UFS has trim: (-t) or MAC multilabel: (-l) flag, resize FS fail.

This pull-request adds these options to the dumpfs "parser", as well as
other missing options, and fully documents what's where these options
are coming from, and which ones we've left out, and why.

Additional Context

FreeBSD Bugzilla: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250496
LP# 1901958

Test Steps

  • Create a partition with newfs -t -l (among other useful options, such as -Uj)
  • instruct cloud-init to resize the filesystem
  • it fails

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly (there are none)
  • I have updated or added any documentation accordingly (i've added a long comment explaining what's going on)

@igalic
Copy link
Collaborator Author

igalic commented Oct 29, 2020

@TheRealFalcon thanks for the approval, but i'm still looking into if or how to test this 😅

@f-andrey
Copy link

make ufs partition and set flags tunefs?

@mitechie
Copy link
Contributor

mitechie commented Nov 2, 2020

Thank you @igalic, if you don't mind maybe make this a WIP PR until the testing situation is set up?

@igalic igalic changed the title cc_resizefs: Fix accepted opts for FreeBSD newfs WIP: cc_resizefs: Fix accepted opts for FreeBSD newfs Nov 2, 2020
@igalic
Copy link
Collaborator Author

igalic commented Nov 3, 2020

@mitechie please note that i don't have the permissions to set labels; so I changed the title.

@TheRealFalcon TheRealFalcon self-assigned this Nov 3, 2020
On FreeBSD , if a UFS has trim: (-t) or MAC multilabel: (-l) flag,
resize FS fail.

This pull-request adds these options to the dumpfs "parser", as well as
other missing options, and fully documents what's where these options
are coming from, and which ones we've left out, and why.

FreeBSD Bugzilla: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250496
LP# 1901958
@igalic
Copy link
Collaborator Author

igalic commented Nov 3, 2020

i have a new, better patch…

diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py
index 2febecf5..4e688800 100644
--- a/cloudinit/config/cc_resizefs.py
+++ b/cloudinit/config/cc_resizefs.py
@@ -9,10 +9,7 @@
 """Resizefs: cloud-config module which resizes the filesystem"""
 
 import errno
-import getopt
 import os
-import re
-import shlex
 import stat
 from textwrap import dedent
 
@@ -88,71 +85,13 @@ def _resize_zfs(mount_point, devpth):
     return ('zpool', 'online', '-e', mount_point, devpth)
 
 
-def _get_dumpfs_output(mount_point):
-    return subp.subp(['dumpfs', '-m', mount_point])[0]
-
-
-def _get_gpart_output(part):
-    return subp.subp(['gpart', 'show', part])[0]
-
-
-def _can_skip_resize_ufs(mount_point, devpth):
-    # extract the current fs sector size
-    """
-    # dumpfs -m /
-    # newfs command for / (/dev/label/rootfs)
-      newfs -L rootf -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384
-            -h 64 -i 8192 -j -k 6408 -m 8 -o time -s 58719232 /dev/label/rootf
-    """
-    cur_fs_sz = None
-    frag_sz = None
-    dumpfs_res = _get_dumpfs_output(mount_point)
-    for line in dumpfs_res.splitlines():
-        if not line.startswith('#'):
-            newfs_cmd = shlex.split(line)
-            opt_value = 'JUjltL:O:a:b:c:d:e:f:g:h:i:k:m:p:r:s:'
-            optlist, _args = getopt.getopt(newfs_cmd[1:], opt_value)
-            for o, a in optlist:
-                if o == "-s":
-                    cur_fs_sz = int(a)
-                if o == "-f":
-                    frag_sz = int(a)
-    # check the current partition size
-    # Example output from `gpart show /dev/da0`:
-    # =>      40  62914480  da0  GPT  (30G)
-    #         40      1024    1  freebsd-boot  (512K)
-    #       1064  58719232    2  freebsd-ufs  (28G)
-    #   58720296   3145728    3  freebsd-swap  (1.5G)
-    #   61866024   1048496       - free -  (512M)
-    expect_sz = None
-    m = re.search('^(/dev/.+)p([0-9])$', devpth)
-    gpart_res = _get_gpart_output(m.group(1))
-    for line in gpart_res.splitlines():
-        if re.search(r"freebsd-ufs", line):
-            fields = line.split()
-            expect_sz = int(fields[1])
-    # Normalize the gpart sector size,
-    # because the size is not exactly the same as fs size.
-    normal_expect_sz = (expect_sz - expect_sz % (frag_sz / 512))
-    if normal_expect_sz == cur_fs_sz:
+def _can_skip_resize_ufs(_mount_point, devpth):
+    (_out, err) = subp.subp(['growfs', '-N', devpth], rcs=[0, 1])
+    # possible errors:
+    # growfs: requested size 2.0GB is not larger than the current
+    #   filesystem size 2.0GB
+    # growfs: superblock not recognized
+    if err is not None:
         return True
     else:
         return False 

@igalic
Copy link
Collaborator Author

igalic commented Nov 4, 2020

closing this, and will re-open with the better version.

@igalic igalic closed this Nov 4, 2020
@igalic igalic deleted the fix/freebsd-fix-ufs-resize branch November 4, 2020 19:31
OddBloke pushed a commit that referenced this pull request Nov 19, 2020
On FreeBSD, if a UFS has trim: (-t) or MAC multilabel: (-l) flag, resize
FS fail, because the _can_skip_ufs_resize check gets tripped up by the
missing options.

This was reported at FreeBSD Bugzilla: 
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250496 and as
LP: #1901958

Rather than fixing the parser as in the patches proposed there (and
attempted in #636) this pull-request rips out all of it, and simplifies
the code.  We now use `growfs -N` and check if that returns an error. If
it returns the correct kind of error, we can skip the resize, because we
either are at the correct size, or the filesystem in question is broken
or not UFS. If it returns the wrong kind of error, we just re-raise it.

LP: #1901958
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.

4 participants