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

Don't create ambiguous filesystems, and warn when reusing one #991

Merged
merged 7 commits into from
Jun 3, 2020
Merged

Don't create ambiguous filesystems, and warn when reusing one #991

merged 7 commits into from
Jun 3, 2020

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Jun 3, 2020

ZFS has an unusual property: it has multiple superblocks distributed throughout the disk, and none of mkfs.ext4, mkfs.xfs, or mkfs.vfat clobber all of them. If blkid or lsblk discover superblocks of both ZFS and one of those other filesystems, they won't report a filesystem type at all (blkid will ignore the entire partition), and mount will refuse to mount the filesystem without an explicit -t argument.

When creating a new filesystem, run wipefs on the partition to ensure we don't create an ambiguous (or, as libblkid calls it, "ambivalent") filesystem. When reusing an existing filesystem, log a warning if it is ambivalent, but otherwise proceed as before.

Pursuant to coreos/fedora-coreos-tracker#500.

Copy link
Contributor

@arithx arithx left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

LGTM

@bgilbert
Copy link
Contributor Author

bgilbert commented Jun 3, 2020

kola tests are failing because we're not pulling wipefs into the initramfs.

@jlebon
Copy link
Member

jlebon commented Jun 3, 2020

kola tests are failing because we're not pulling wipefs into the initramfs.

This will be better once we do coreos/fedora-coreos-tracker#511. But until then instead of waiting for coreos/ignition-dracut#190 to get into an RPM, we can pretty easily just "build" ignition-dracut from scratch here. Something like (untested):

diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile
index 8b09b66..de332f6 100644
--- a/.cci.jenkinsfile
+++ b/.cci.jenkinsfile
@@ -15,7 +15,12 @@ cosaPod(buildroot: true) {
             mkdir -p overrides/rpm && cd overrides/rpm
             curl -L --remote-name-all https://kojipkgs.fedoraproject.org//packages/kernel/5.6.7/200.fc31/x86_64/kernel{,-core,-modules}-5.6.7-200.fc31.x86_64.rpm
         """)
-        fcosBuild(skipInit: true, make: true, skipKola: true)
+        shwrap("""
+            // and we want latest ignition-dracut until it's merged
+            // https://github.com/coreos/fedora-coreos-tracker/issues/511
+            git clone https://github.com/coreos/ignition-dracut
+        """)
+        fcosBuild(skipInit: true, make: true, makeDirs: ["ignition-dracut"], skipKola: true)
     }

     // we run the blackbox tests separately instead of as part of the main kola

@bgilbert
Copy link
Contributor Author

bgilbert commented Jun 3, 2020

#994

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM overall, just one bikeshed comment.

bgilbert added 6 commits June 3, 2020 13:02
We need to test overwriting a ZFS filesystem with another filesystem,
but we don't have access to a mkfs.zfs.  Embed a small ZFS image and copy
it to a partition when needed.
Needed when mounting ambivalent filesystems.
We still do three separate blkid probes, but returning a single struct
presents a better high-level interface.  The disks stage was already
aggregating the three calls at a higher level.
When checking whether an existing filesystem matches the config, detect
multiple filesystem signatures on a target partition.  If found, log a
warning.  Don't do more than that for now, for backward compatibility
and because it's not clear that we should reject an otherwise-matching
filesystem just because it contains some leftover bits.

Disallow multiple signatures during test output validation, unless the
test requests otherwise.
@bgilbert
Copy link
Contributor Author

bgilbert commented Jun 3, 2020

Updated!

Creating a filesystem doesn't necessarily remove all remnants of previous
superblocks, notably if the old filesystem is ZFS.  Run wipefs first, to
prevent blkid from refusing to identify the resulting filesystem.
@bgilbert bgilbert merged commit b0a8d3c into coreos:master Jun 3, 2020
@bgilbert bgilbert deleted the wipefs branch June 3, 2020 18:55
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