PR#16: appcreate: Ensure Btrfs volumes and subvolumes are torn down last - appliance-tools

appliance-tools

#16 appcreate: Ensure Btrfs volumes and subvolumes are torn down last

Merged 2 years ago by ngompa. Opened 2 years ago by ngompa.

fix-unmount-order-btrfs into master

appcreate: Ensure Btrfs volumes and subvolumes are torn down last

Neal Gompa • 2 years ago

94f331d

appcreate/partitionedfs.py

file modified

+30 -16

		`@@ -225,12 +225,14 @@`

		`def __calculate_mountorder(self):`
		`btrfs_mountOrder = []`
		`+ btrfs_unmountOrder = []`
		`for p in self.partitions:`
		`if p['fstype'] == 'btrfs':`
		`btrfs_mountOrder.append(p['mountpoint'])`
		`+ btrfs_unmountOrder.append(p['mountpoint'])`
		`else:`
		`self.mountOrder.append(p['mountpoint'])`
		`- self.unmountOrder.append(p['mountpoint'])`
		`+ self.unmountOrder.append(p['mountpoint'])`

		`self.mountOrder.sort()`
		`btrfs_mountOrder.sort()`
		`@@ -238,7 +240,12 @@`
		`self.mountOrder = btrfs_mountOrder + self.mountOrder`
		`self.unmountOrder.sort()`
		`self.unmountOrder.reverse()`
		`- logging.info(str(self.mountOrder))`
		`+ btrfs_unmountOrder.sort()`
		`+ btrfs_unmountOrder.reverse()`
		`+ # Btrfs mountpoints must be last in the list`
		`+ self.unmountOrder = self.unmountOrder + btrfs_unmountOrder`
		`+ logging.debug("Mount order: %s" % str(self.mountOrder))`
		`+ logging.debug("Unmount order: %s" % str(self.unmountOrder))`

		`def cleanup(self):`
		`Mount.cleanup(self)`
		`@@ -251,20 +258,6 @@`
		`pass`

		`def unmount(self):`
		`- if self.subvolumes:`
		`- ordered = []`
		`- others = []`
		`- for s in self.subvolumes:`
		`- if s['mountpoint'] == '/':`
		`- others.append(s)`
		`- else:`
		`- ordered.append(s)`
		`-`
		`- ordered += others`
		`-`
		`- for s in ordered:`
		`- subprocess.call(['umount', "%s%s" % (self.mountdir, s['mountpoint'])])`
		`-`
		`for mp in self.unmountOrder:`
		`if mp == 'swap':`
		`continue`
		`@@ -281,6 +274,27 @@`
		`pass`
		`p['mount'] = None`

		`+ if self.subvolumes:`
		`+ ordered = []`
		`+ others = []`
		`+ for s in self.subvolumes:`
		`+ if s['mountpoint'] == '/':`
		`+ others.append(s)`
		`+ else:`
		`+ ordered.append(s)`
		`+`
		`+ ordered = ordered + others`
		`+`
		`+ for s in ordered:`
		`+ logging.info("Unmounting directory %s%s" % (self.mountdir, s['mountpoint']))`
		`+ rc = subprocess.call(['umount', "%s%s" % (self.mountdir, s['mountpoint'])])`
		`+ if rc != 0:`
		`+ logging.warning("Unmounting directory %s%s failed, using lazy "`
		`+ "umount" % (self.mountdir, s['mountpoint']))`
		`+ print("Unmounting directory %s%s failed, using lazy umount" %`
		`+ (self.mountdir, s['mountpoint']), file=sys.stdout)`
		`+ subprocess.call(['umount', '-l', "%s%s" % (self.mountdir, s['mountpoint'])])`
		`+`
		`def setup_subvolumes(self):`
		`others = []`
		`ordered = []`

ngompa commented 2 years ago

This should ensure that the appliance image finalization does not
get stuck with out-of-order sequence of unmounting to clean up the
disk creation working area.

ngompa commented 2 years ago

@dcavalca Please test this ASAP to verify this works so I can land this in Fedora before the next compose.

ngompa commented 2 years ago

(cc: @pwhalen)

rebased onto a4d23e3

2 years ago

rebased onto 9e70c05

2 years ago

rebased onto 94f331d

2 years ago

dcavalca commented 2 years ago

Testing now with the ks from https://koji.fedoraproject.org/koji/taskinfo?taskID=50015418 converted for x86_64 (as https://pagure.io/fedora-btrfs/project/issue/11 is still an issue).

dcavalca commented 2 years ago

Running sudo /usr/bin/appliance-creator -c koji-image-f34-build-50015418-x86_64.ks -d -v --logfile appliance.log --cache koji-appliance -o app-output --format raw --name Fedora-Xfce-armhfp-Rawhide-20200823.n.2 --version Rawhide --release 20200823.n.2

dcavalca commented 2 years ago

I'm getting a weird package conflict on dnf (possibly because of the arm->x86_64 thing), but the umount logic looks correct:

umount:
/var/tmp/imgcreate-envc3fu7/install_root/var/cache/dnf:
target
is
busy.
Unable to unmount /var/tmp/imgcreate-envc3fu7/install_root/var/cache/dnf normally, using lazy unmount
lazy umount succeeded on /var/tmp/imgcreate-envc3fu7/install_root/var/cache/dnf
lazy umount succeeded on /var/tmp/imgcreate-envc3fu7/install_root/var/cache/dnf
Unmounting directory /var/tmp/imgcreate-envc3fu7/install_root/boot/efi
Unmounting directory /var/tmp/imgcreate-envc3fu7/install_root/boot
Unmounting directory /var/tmp/imgcreate-envc3fu7/install_rootbtrfs.007
Unmounting directory /var/tmp/imgcreate-envc3fu7/install_root/home
Unmounting directory /var/tmp/imgcreate-envc3fu7/install_root/
Removing compat symlinks
Unmapping /dev/loop0
Losetup remove /dev/loop0
[davide@localhost ~]$

Pull-Request has been merged by ngompa

2 years ago