[systemd-devel] [PATCH] core: don't change removed devices to state "tentative" [was: Re: [PATCH] unit: When stopping due to BindsTo=, log which unit caused it]

[Lennart Poettering]

On Fri, 13.03.15 08:30, Martin Pitt (martin.pitt at ubuntu.com) wrote:

> From 05ffa415fa4f75f2e71830d47179b6f4a67c7215 Mon Sep 17 00:00:00 2001
> From: Martin Pitt <martin.pitt at ubuntu.com>
> Date: Fri, 13 Mar 2015 08:23:02 +0100
> Subject: [PATCH] core: don't change removed devices to state "tentative"
> 
> Commit 628c89c introduced the "tentative" device state, which caused devices to
> go from "plugged" to "tentative" on a remove uevent. This breaks the cleanup
> of stale mounts (see commit 3b48ce4), as that only applies to "dead" devices.
> 
> The "tentative" state only really makes sense on adding a device when we don't
> know where it was coming from (i. e. not from udev). But when we get a device
> removal from udev we definitively know that it's gond, so change the device
> state back to "dead" as before 628c89c.

Hmm, so this patch doesn't look right. The "add" boolean you are
checking does not encode what the previous state of the device was,
but simply whether to add or remove a flag bit from the found
variable.

The right way to handle this is to explicitly check if the device was
seen by udev before. I have now fixed that in git.

Also, we need to save/restore the state across daemon reloads, so that
after coming back we can also know whether the device has been
previously seen by udev. I have also added a fix for this now.

I only gave this light testing, I'd really appreciate if you could
test this, if this still does the right thing!

Thanks!

> ---
>  src/core/device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/core/device.c b/src/core/device.c
> index 6b489a4..098a000 100644
> --- a/src/core/device.c
> +++ b/src/core/device.c
> @@ -419,7 +419,7 @@ static void device_update_found_one(Device *d, bool add, DeviceFound found, bool
>          if (now) {
>                  if (d->found & DEVICE_FOUND_UDEV)
>                          device_set_state(d, DEVICE_PLUGGED);
> -                else if (d->found != DEVICE_NOT_FOUND)
> +                else if (add && d->found != DEVICE_NOT_FOUND)
>                          device_set_state(d, DEVICE_TENTATIVE);
>                  else
>                          device_set_state(d, DEVICE_DEAD);
> -- 
> 2.1.4
> 





Lennart

-- 
Lennart Poettering, Red Hat