On Thu, 9 Jun 2022 15:11:22 -0600
Logan Gunthorpe <logang@xxxxxxxxxxxx> wrote:
> The commit noted below globally adds a parameter to the -r option but missed
> the fact that -r is used for another purpose: --remove.
>
> After that commit, a command such as:
>
> mdadm /dev/md0 -r /dev/loop0
>
> will do nothing seeing the device parameter will be consumed as a
> argument to the -r option; thus, there will only be one device
> seen one the command line, devs_found will only be 1 and nothing will
> happen.
>
> This caused the 01r5integ and 01raid6integ tests to hang indefinitely
> as mdadm did not remove the failed device. With the device not removed,
> it would not be readded. Then the loop waiting for the array status to
> change would loop forever.
>
> To fix this, revert the changes in the noted patch and create a new subopt
> type for the monitor mode with parameters required for -r.
>
> Fixes: 546047688e1c ("mdadm: fix coredump of mdadm --monitor -r")
> Cc: Wu Guanghao <wuguanghao3@xxxxxxxxxx>
> Cc: Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx>
> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> ---
> ReadMe.c | 7 ++++---
> mdadm.c | 1 +
> mdadm.h | 1 +
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/ReadMe.c b/ReadMe.c
> index 8f873c4895da..556104f75d72 100644
> --- a/ReadMe.c
> +++ b/ReadMe.c
> @@ -81,11 +81,12 @@ char Version[] = "mdadm - v" VERSION " - " VERS_DATE
> EXTRAVERSION "\n";
> * found, it is started.
> */
>
> -char
> short_options[]="-ABCDEFGIQhVXYWZ:vqbc:i:l:p:m:r:n:x:u:c:d:z:U:N:safRSow1tye:k";
> +char
> short_options[]="-ABCDEFGIQhVXYWZ:vqbc:i:l:p:m:n:x:u:c:d:z:U:N:sarfRSow1tye:k";
> +char
> short_monitor_options[]="-ABCDEFGIQhVXYWZ:vqbc:i:l:p:m:r:n:x:u:c:d:z:U:N:safRSow1tye:k";
> char short_bitmap_options[]=
> -
> "-ABCDEFGIQhVXYWZ:vqb:c:i:l:p:m:r:n:x:u:c:d:z:U:N:sarfRSow1tye:k:";
> +
> "-ABCDEFGIQhVXYWZ:vqb:c:i:l:p:m:n:x:u:c:d:z:U:N:sarfRSow1tye:k:"; char
> short_bitmap_auto_options[]=
> -
> "-ABCDEFGIQhVXYWZ:vqb:c:i:l:p:m:r:n:x:u:c:d:z:U:N:sa:rfRSow1tye:k:";
> +
> "-ABCDEFGIQhVXYWZ:vqb:c:i:l:p:m:n:x:u:c:d:z:U:N:sa:rfRSow1tye:k:";
> struct option long_options[] = {
> {"manage", 0, 0, ManageOpt},
> diff --git a/mdadm.c b/mdadm.c
> index be40686cf91b..d0c5e6def901 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -227,6 +227,7 @@ int main(int argc, char *argv[])
> shortopt = short_bitmap_auto_options;
> break;
> case 'F': newmode = MONITOR;
> + shortopt = short_monitor_options;
> break;
> case 'G': newmode = GROW;
> shortopt = short_bitmap_options;
> diff --git a/mdadm.h b/mdadm.h
> index 09915a0009d9..559da3f6f440 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -419,6 +419,7 @@ enum mode {
> };
>
> extern char short_options[];
> +extern char short_monitor_options[];
> extern char short_bitmap_options[];
> extern char short_bitmap_auto_options[];
> extern struct option long_options[];
Jes applied Nigel's revert but I cannot see it on repository yet.
I consider adding short monitor options as valuable.
Could you apply revert manually and then adopt your patch?
https://lore.kernel.org/linux-raid/5f9a4417-d044-a87e-3945-2c6b29278d8c@xxxxxxxxxxxxxxxxxx/#t
Thanks,
Mariusz
