Re: [PATCH] 473260 - unmount swap files on upgrades

[David Cantrell <dcantrell redhat com>]

Chris Lumens wrote:
> The above referenced bug seems to be a really big deal on F10 upgrades.
> The problem is that swap files never get unmounted, so the filesystems
> they reside on never get unmounted, so migrateFilesystems fails.   The
> attached patch corrects this by adding support to just unmount swap
> files.  If we come to some sort of agreement on this patch, I'm in favor
> of including it in an updates.img.
> 
> - Chris
> 

Comments below.

> 
> diff --git a/fsset.py b/fsset.py
> index a4b3cd4..d425a48 100644
> --- a/fsset.py
> +++ b/fsset.py
> @@ -839,8 +839,19 @@ class swapFileSystem(FileSystemType):
>          isys.swapon (device)
>  
>      def umount(self, device, path):
> -        # unfortunately, turning off swap is bad.
> -        raise RuntimeError, "unable to turn off swap"
> +        # Turning off swap devices is bad, but we can get away with turning
> +        # off swap files.
> +        if device.device.startswith("/dev/"):
> +            raise RuntimeError, "unable to turn off swap"
> +
> +        # path is something stupid like instPath+"swap", which is totally
> +        # useless.  But we don't have instPath anywhere else so trim off
> +        # the end of path.
> +        swapFile = path[:path.rfind("/")] + device.device

I'm not quite following what this line does.  Is it doing a dirname on
path and then adding device.device to it?  I guess I don't understand
why this is necessary, or why we can't use realpath() to normalize it
before sending it to swapoff.

> +
> +        iutil.execWithRedirect("swapoff", [swapFile],
> +                               stdout="/dev/tty5", stderr="/dev/tty5",
> +                               searchPath=1)
>  

Do we care about swapoff failing and notifying the caller?

>      def formatDevice(self, entry, progress, chroot='/'):
>          file = entry.device.setupDevice(chroot)
> @@ -2103,7 +2114,10 @@ MAILADDR root
>          reverse.reverse()
>  
>          for entry in reverse:
> -            if entry.mountpoint == "swap" and not swapoff:
> +            # Always unmount swap files, or we won't be able to unmount the
> +            # device they reside on.
> +            if entry.mountpoint == "swap" and not swapoff and \
> +               entry.device.device.startswith("/dev/"):
>                  continue
>              entry.umount(instPath)
>              entry.device.cleanupDevice(instPath)
> 

-- 
David Cantrell <dcantrell redhat com>
Red Hat / Honolulu, HI