[systemd-devel] [PATCH] Fix starting swap unit on symlink made it unstoppable

[Zbigniew Jędrzejewski-Szmek]

On Tue, Oct 23, 2012 at 10:15:00PM +0200, jjacky wrote:
> 
> On 10/23/12 21:05, Lennart Poettering wrote:
> > On Sat, 13.10.12 14:24, Olivier Brunel (i.am.jack.mail at gmail.com) wrote:
> > 
> >> Starting a swap unit pointing to (What) a symlink (e.g. /dev/mapper/swap
> >> or /dev/disk/by-uuid/...) would have said unit be marked active, follow
> >> the one using the "actual" device (/dev/{dm-1,sda3}), but that new unit
> >> would be seen as inactive.
> >> Since all requests to stop swap units would follow/redirect to it,
> >> and it is seen inactive, nothing would be done (swapoff never called).
> >>
> >> This is because this unit would be treated twice in
> >> swap_process_new_swap, the second call to swap_add_one causing it to
> >> eventually be marked inactive.
> >>
> >> Signed-off-by: Olivier Brunel <i.am.jack.mail at gmail.com>
> >> ---
> >> The patch removes the call to udev_device_get_devnode, assuming that
> >> device nodes (and not symlinks) are used under /proc/swaps, which
> >> seems to be the case.
> > 
> > This is not the case on Fedora at least.
> 
> Oh? I assumed it would be more of a kernel thing, so likely to be the
> same everywhere. Also, a quick test in a VM running FC17, when I enable
> a swap on /dev/mapper/foo it gets listed as /dev/dm-2 under /proc/swaps,
> aka where the symlink points to.
> Anyhow...
> 
> > 
> > swap.c really needs to handle "following" devices properly (see other
> > mail). We cut some corners there, and we shouldn't.
> 
> Does that mean keeping the call to udev_device_get_devnode and simply
> using strcmp to make sure the same unit isn't processed twice (which
> causes the bug) isn't enough?
> 
> At least AFAICS it does fix the bug, and the "following" seems to be
> working fine.
Hi,
I now pushed an ammended version of your patch, which,
like you suggested, does a string compare on the name to avoid
calling swap_add_one twice with the same name. This definitely
helps in some cases.

Nevertheless, it is still possible to confuse the swap logic without too
much trouble. E.g. doing

  systemctl start dev-sda3.swap 'dev-disk-by\x2dlabel-SWAP.swap'

when they both refer to the same thing, causes one of the jobs to
fail. Start jobs are normally idempotent, so this case shouldn't fail
either.

Zbyszek