From 9d4655cf55ec0ab2d95b707e6b5ff009f0515bcd Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 20 Dec 2017 18:16:14 -0400 Subject: two diskimage edge case fixes * DiskImage: Fix rsync crash when a mount point does not exist in the chroot. * Fix bug in unmountBelow that caused unmounting of nested mounts to fail. This commit was sponsored by Jack Hill on Patreon. --- debian/changelog | 4 ++++ ...mment_2_f90109eb016065586225fc59674c431c._comment | 20 ++++++++++++++++++++ src/Propellor/Property/DiskImage.hs | 19 ++++++++++++------- src/Propellor/Property/Mount.hs | 4 +++- 4 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 doc/forum/imageBuiltFor_mount_points_not_automatically_created/comment_2_f90109eb016065586225fc59674c431c._comment diff --git a/debian/changelog b/debian/changelog index bfb884ce..3da0622f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -9,6 +9,10 @@ propellor (5.2.0) UNRELEASED; urgency=medium * DiskImage: Use safeAlignment. It didn't seem worth making the alignment configurable here. * Fixed rounding bug in Parted.calcPartTable. + * DiskImage: Fix rsync crash when a mount point does not exist in the + chroot. + * Fix bug in unmountBelow that caused unmounting of nested mounts to + fail. -- Joey Hess Wed, 29 Nov 2017 11:45:08 -0400 diff --git a/doc/forum/imageBuiltFor_mount_points_not_automatically_created/comment_2_f90109eb016065586225fc59674c431c._comment b/doc/forum/imageBuiltFor_mount_points_not_automatically_created/comment_2_f90109eb016065586225fc59674c431c._comment new file mode 100644 index 00000000..d8655a59 --- /dev/null +++ b/doc/forum/imageBuiltFor_mount_points_not_automatically_created/comment_2_f90109eb016065586225fc59674c431c._comment @@ -0,0 +1,20 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 2""" + date="2017-12-20T21:36:36Z" + content=""" +AFAICS, DiskImage.imageFinalized creates any mount points that were not +rsynced over from the chroot. But, I guess that partitionsPopulated is +expecting the mount point to exist in order to rsync its content over, +and if nothing created /boot/efi that would explain the rsync error. +I've added a check to prevent that problem. + +As for the unmount problem, it's a problem with the order +it traverses the mount points for unmounting. That is using +"unmountBelow" with the directory where the disk image partitions +are loop mounted. If that unmounts `boot` first, it will implicitly +unmount `boot/efi` (due to --lazy) and then will fail when it +tries to explicitly unmount it. Added sorting that should fix that. + +Let me know how that goes, I've not tested it with your config. +"""]] diff --git a/src/Propellor/Property/DiskImage.hs b/src/Propellor/Property/DiskImage.hs index 79865db4..c9998d3c 100644 --- a/src/Propellor/Property/DiskImage.hs +++ b/src/Propellor/Property/DiskImage.hs @@ -274,13 +274,18 @@ partitionsPopulated chrootdir mnts mntopts devs = property' desc $ \w -> desc = "partitions populated from " ++ chrootdir go _ Nothing _ _ = noChange - go w (Just mnt) mntopt loopdev = withTmpDir "mnt" $ \tmpdir -> bracket - (liftIO $ mount "auto" (partitionLoopDev loopdev) tmpdir mntopt) - (const $ liftIO $ umountLazy tmpdir) - $ \ismounted -> if ismounted - then ensureProperty w $ - syncDirFiltered (filtersfor mnt) (chrootdir ++ mnt) tmpdir - else return FailedChange + go w (Just mnt) mntopt loopdev = ifM (liftIO $ doesDirectoryExist srcdir) $ + ( withTmpDir "mnt" $ \tmpdir -> bracket + (liftIO $ mount "auto" (partitionLoopDev loopdev) tmpdir mntopt) + (const $ liftIO $ umountLazy tmpdir) + $ \ismounted -> if ismounted + then ensureProperty w $ + syncDirFiltered (filtersfor mnt) srcdir tmpdir + else return FailedChange + , return NoChange + ) + where + srcdir = chrootdir ++ mnt filtersfor mnt = let childmnts = map (drop (length (dropTrailingPathSeparator mnt))) $ diff --git a/src/Propellor/Property/Mount.hs b/src/Propellor/Property/Mount.hs index c047161d..e8f3f092 100644 --- a/src/Propellor/Property/Mount.hs +++ b/src/Propellor/Property/Mount.hs @@ -149,4 +149,6 @@ umountLazy mnt = unmountBelow :: FilePath -> IO () unmountBelow d = do submnts <- mountPointsBelow d - forM_ submnts umountLazy + -- sort so sub-mounts are unmounted before the mount point + -- containing them + forM_ (sort submnts) umountLazy -- cgit v1.2.3