[systemd-devel] [PATCH v3] device: Fix overzealous unmounting of tentative device mounts

[Lennart Poettering]

On Tue, 19.05.15 10:23, Martin Pitt (martin.pitt at ubuntu.com) wrote:

> Hello all,
> 
> I have a better fix now.
> 
> > So the problem is that this tentative → dead transition only works if
> > a device is referenced once, but causes overzealous unmounts if there
> > are more references.

Ah, indeed, that makes sense!

> > We need a reference count, or check if the device is bound by other
> > device still. Come to think of it now, we actually already have that:
> > 
> >   Id=dev-foo.device
> >   BoundBy=tmp-boot.mount tmp-etc.mount
> > 
> > But my patch doesn't take that into account yet.
> 
> This one does now. I left a fairly comprehensive commit log as this
> isn't that easy to explain/understand. If it's too verbose for you I
> can also trim it back a bit.

I have now committed a different fix now, that keeps counting of the
mount points in mount.c, instead of "reaching over" from device.c.

I only gave this light testing, would be cool if you could check if
this fixes things for you.

http://cgit.freedesktop.org/systemd/systemd/commit/?id=fcd8b266edf0df2b85079fcf7b099cd4028740e6

This commit will now collect two sets of devices while going through
/proc/self/mountinfo: the devices of lines that are no longer there,
and the devices of lines that are there. Only for devices in the
former set that are not in the latter we will now propagate an event
to device.c.

Does this make sense?

> +        /* If we found a tentative device via mountinfo which is still
> +         * referenced by other mounts than the one that just got unmounted, we
> +         * must leave it FOUND_MOUNT/tentative; otherwise we'd trigger
> +         * unmounting of all other bound mounts. */
> +        if (d->found == DEVICE_FOUND_MOUNT && n == 0) {
> +                Iterator i;
> +                Unit *bound;
> +
> +                SET_FOREACH(bound, UNIT(d)->dependencies[UNIT_BOUND_BY], i) {
> +                        if (bound->type == UNIT_MOUNT &&
> +                            MOUNT(bound)->state != MOUNT_DEAD && MOUNT(bound)->state != MOUNT_FAILED) {
> +                                log_unit_debug(UNIT(d), "Still bound by %s (%s), keeping state.",
> +                                               bound->id, mount_state_to_string(MOUNT(bound)->state));
> +                                return;
> +                        }
> +                }
> +                log_unit_debug(UNIT(d), "Not referenced by any mounts any more, changing to dead.");
> +        }
> +
>          previous = d->found;
>          d->found = n;

Lennart

-- 
Lennart Poettering, Red Hat