Re: [PATCH v3] Incremental: remove obsoleted calls to udisks

[Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>]

On Tue, 30 May 2023 00:07:54 +0800
Coly Li <colyli@suse.de> wrote:

> Utilility udisks is removed from udev upstream, calling this obsoleted
> command in run_udisks() doesn't make any sense now.
> 
> This patch removes the calls chain of udisks, which includes routines
> run_udisk(), force_remove(), and 2 locations where force_remove() are
> called. Considering force_remove() is removed with udisks util, it is
> fair to remove Manage_stop() inside force_remove() as well.
> 
> In the two modifications where calling force_remove() are removed,
> the failure from Manage_subdevs() can be safely ignored, because,
> 1) udisks doesn't exist, no need to check the return value to umount
>    the file system by udisks and remove the component disk again.
> 2) After the 'I' inremental remove, there is another 'r' hot remove
>    following up. The first incremental remove is a best-try effort.
Hi Coly,

I'm not sure what you meant here. I know that on "remove" event udev will call
mdadm -If <devname>. And that is all I'm familiar with. I don't see another
branch executed in code to handle "remove" event, no second attempt for clean
up is made. Could you clarify? How is it executed?
Perhaps, I understand it incorrectly as second action that is always executed
automatically. I know that there is an action "--remove" which can be manually
triggered. Is that what you meant?

> Therefore in this patch, where force_remove() is removed, the return
> value of calling Manage_subdevs() is not checked too.
> 
> Signed-off-by: Coly Li <colyli@suse.de>
> Cc: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
> Cc: Jes Sorensen <jes@trained-monkey.org>
> ---
> Changelog,
> v3: remove the almost-useless warning message, and make the change
>     more simplified.
> v2: improve based on code review comments from Mariusz.
> v1: initial version.

For the code:
Reviewed-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>

Mariusz