Re: [PATCH] FIX: Cannot remove failed disk from container

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2165 bytes --]

On Thu, 29 Dec 2011 14:27:38 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When disk is failed by mdadm e.g.:
>     mdadm -f /dev/md/raid_array /dev/sdX
> and then it is tried to be removed from container e.g.:
>     mdadm --remove /dev/md/container /dev/sdX
> 
> mdadm refuses it with information:
>     mdadm: /dev/sdX is still in use, cannot remove.
> 
> Problem was introduced in commit:
>     monitor: don't unblock a device that isn't blocked.
>     /2011-12-06/
> Disk without unblocking it cannot be really removed from array
> and reference to if is still reported under 'holders' sysfs entry.
> 
> As this commit is necessary for managing degraded array during
> reshape and rebuild code for unconditional unblocking disk on removal
> is added.
> Guard for setting DS_UNBLOCK during reshape/rebuild avoids process
> performance degradation.

You seem to be addressing the symptom rather than understanding the real
problem.

If a device isn't marked as 'blocked' then it simply doesn't make any sense
to "unblock" it - that cannot do anything useful.

If the commit you identify broke things for you, then we need to understand
exactly why.  What exactly is the problem, how was the code previously
allowing things to work?  What is the minimal thing we need to do to allow
things to work again?

Just setting DS_UNBLOCK because it seems to work but without a clear
justification isn't acceptable.

NeilBrown


> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  monitor.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 29bde18..ce66625 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -346,6 +346,12 @@ static int read_and_act(struct active_array *a)
>  				a->next_state = active;
>  			}
>  			if (a->curr_state > readonly)
> +				/* For disk removal when no md process
> +				 * /e.g. reshape/ is in progress, trigger
> +				 * unconditional device unblock first
> +				 */
> +				if (a->curr_action == idle)
> +					mdi->next_state |= DS_UNBLOCK;
>  				mdi->next_state |= DS_REMOVE;
>  		}
>  	}


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]