All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r"
@ 2022-04-18 17:44 Nigel Croxon
  2022-05-23 15:43 ` Coly Li
  2022-06-14 14:11 ` Nigel Croxon
  0 siblings, 2 replies; 8+ messages in thread
From: Nigel Croxon @ 2022-04-18 17:44 UTC (permalink / raw)
  To: jes, linux-raid, mariusz.tkaczyk, wuguanghao3, colyli

This reverts commit 546047688e1c64638f462147c755b58119cabdc8.

The change from commit mdadm: fix coredump of mdadm
--monitor -r broke the printing of the return message when
passing -r to mdadm --manage, the removal of a device from
an array.

If the current code reverts this commit, both issues are
still fixed.

The original problem reported that the fix tried to address
was:  The --monitor -r option requires a parameter,
otherwise a null pointer will be manipulated when
converting to integer data, and a core dump will appear.

The original problem was really fixed with:
60815698c0a Refactor parse_num and use it to parse optarg.
Which added a check for NULL in 'optarg' before moving it
to the 'increments' variable.

New issue: When trying to remove a device using the short
argument -r, instead of the long argument --remove, the
output is empty. The problem started when commit
546047688e1c was added.

Steps to Reproduce:
1. create/assemble /dev/md0 device
2. mdadm --manage /dev/md0 -r /dev/vdxx

Actual results:
Nothing, empty output, nothing happens, the device is still
connected to the array.

The output should have stated "mdadm: hot remove failed
for /dev/vdxx: Device or resource busy", if the device was
still active. Or it should remove the device and print
a message:

mdadm: set /dev/vdd faulty in /dev/md0
mdadm: hot removed /dev/vdd from /dev/md0

The following commit should be reverted as it breaks
mdadm --manage -r.

commit 546047688e1c64638f462147c755b58119cabdc8
Author: Wu Guanghao <wuguanghao3@huawei.com>
Date:   Mon Aug 16 15:24:51 2021 +0800
mdadm: fix coredump of mdadm --monitor -r

-Nigel

Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
---
 ReadMe.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ReadMe.c b/ReadMe.c
index 8f873c48..bec1be9a 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -81,11 +81,11 @@ 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_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},
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r"
  2022-04-18 17:44 [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r" Nigel Croxon
@ 2022-05-23 15:43 ` Coly Li
  2022-06-14 14:11 ` Nigel Croxon
  1 sibling, 0 replies; 8+ messages in thread
From: Coly Li @ 2022-05-23 15:43 UTC (permalink / raw)
  To: Nigel Croxon; +Cc: jes, linux-raid, wuguanghao3, mariusz.tkaczyk

On 4/19/22 1:44 AM, Nigel Croxon wrote:
> This reverts commit 546047688e1c64638f462147c755b58119cabdc8.
>
> The change from commit mdadm: fix coredump of mdadm
> --monitor -r broke the printing of the return message when
> passing -r to mdadm --manage, the removal of a device from
> an array.
>
> If the current code reverts this commit, both issues are
> still fixed.
>
> The original problem reported that the fix tried to address
> was:  The --monitor -r option requires a parameter,
> otherwise a null pointer will be manipulated when
> converting to integer data, and a core dump will appear.
>
> The original problem was really fixed with:
> 60815698c0a Refactor parse_num and use it to parse optarg.
> Which added a check for NULL in 'optarg' before moving it
> to the 'increments' variable.
>
> New issue: When trying to remove a device using the short
> argument -r, instead of the long argument --remove, the
> output is empty. The problem started when commit
> 546047688e1c was added.
>
> Steps to Reproduce:
> 1. create/assemble /dev/md0 device
> 2. mdadm --manage /dev/md0 -r /dev/vdxx
>
> Actual results:
> Nothing, empty output, nothing happens, the device is still
> connected to the array.
>
> The output should have stated "mdadm: hot remove failed
> for /dev/vdxx: Device or resource busy", if the device was
> still active. Or it should remove the device and print
> a message:
>
> mdadm: set /dev/vdd faulty in /dev/md0
> mdadm: hot removed /dev/vdd from /dev/md0
>
> The following commit should be reverted as it breaks
> mdadm --manage -r.
>
> commit 546047688e1c64638f462147c755b58119cabdc8
> Author: Wu Guanghao <wuguanghao3@huawei.com>
> Date:   Mon Aug 16 15:24:51 2021 +0800
> mdadm: fix coredump of mdadm --monitor -r
>
> -Nigel

Maybe it could be better to remove the above signature?


>
> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>

Acked-by: Coly Li <colyli@suse.de>


Thanks.


Coly Li


> ---
>   ReadMe.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ReadMe.c b/ReadMe.c
> index 8f873c48..bec1be9a 100644
> --- a/ReadMe.c
> +++ b/ReadMe.c
> @@ -81,11 +81,11 @@ 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_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},



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r"
  2022-04-18 17:44 [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r" Nigel Croxon
  2022-05-23 15:43 ` Coly Li
@ 2022-06-14 14:11 ` Nigel Croxon
  2022-06-17 17:09   ` Nigel Croxon
  1 sibling, 1 reply; 8+ messages in thread
From: Nigel Croxon @ 2022-06-14 14:11 UTC (permalink / raw)
  To: jes, linux-raid, mariusz.tkaczyk, wuguanghao3, colyli

On 4/18/22 1:44 PM, Nigel Croxon wrote:
> This reverts commit 546047688e1c64638f462147c755b58119cabdc8.
> 
> The change from commit mdadm: fix coredump of mdadm
> --monitor -r broke the printing of the return message when
> passing -r to mdadm --manage, the removal of a device from
> an array.
> 
> If the current code reverts this commit, both issues are
> still fixed.
> 
> The original problem reported that the fix tried to address
> was:  The --monitor -r option requires a parameter,
> otherwise a null pointer will be manipulated when
> converting to integer data, and a core dump will appear.
> 
> The original problem was really fixed with:
> 60815698c0a Refactor parse_num and use it to parse optarg.
> Which added a check for NULL in 'optarg' before moving it
> to the 'increments' variable.
> 
> New issue: When trying to remove a device using the short
> argument -r, instead of the long argument --remove, the
> output is empty. The problem started when commit
> 546047688e1c was added.
> 
> Steps to Reproduce:
> 1. create/assemble /dev/md0 device
> 2. mdadm --manage /dev/md0 -r /dev/vdxx
> 
> Actual results:
> Nothing, empty output, nothing happens, the device is still
> connected to the array.
> 
> The output should have stated "mdadm: hot remove failed
> for /dev/vdxx: Device or resource busy", if the device was
> still active. Or it should remove the device and print
> a message:
> 
> mdadm: set /dev/vdd faulty in /dev/md0
> mdadm: hot removed /dev/vdd from /dev/md0
> 
> The following commit should be reverted as it breaks
> mdadm --manage -r.
> 
> commit 546047688e1c64638f462147c755b58119cabdc8
> Author: Wu Guanghao <wuguanghao3@huawei.com>
> Date:   Mon Aug 16 15:24:51 2021 +0800
> mdadm: fix coredump of mdadm --monitor -r
> 
> -Nigel
> 
> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
> ---
>   ReadMe.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/ReadMe.c b/ReadMe.c
> index 8f873c48..bec1be9a 100644
> --- a/ReadMe.c
> +++ b/ReadMe.c
> @@ -81,11 +81,11 @@ 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_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},

Jes, That is the status of this patch?

Thanks, Nigel


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r"
  2022-06-14 14:11 ` Nigel Croxon
@ 2022-06-17 17:09   ` Nigel Croxon
  2022-06-17 19:35     ` Jes Sorensen
  0 siblings, 1 reply; 8+ messages in thread
From: Nigel Croxon @ 2022-06-17 17:09 UTC (permalink / raw)
  To: jes, linux-raid, mariusz.tkaczyk, wuguanghao3, colyli

On 6/14/22 10:11 AM, Nigel Croxon wrote:
> On 4/18/22 1:44 PM, Nigel Croxon wrote:
>> This reverts commit 546047688e1c64638f462147c755b58119cabdc8.
>>
>> The change from commit mdadm: fix coredump of mdadm
>> --monitor -r broke the printing of the return message when
>> passing -r to mdadm --manage, the removal of a device from
>> an array.
>>
>> If the current code reverts this commit, both issues are
>> still fixed.
>>
>> The original problem reported that the fix tried to address
>> was:  The --monitor -r option requires a parameter,
>> otherwise a null pointer will be manipulated when
>> converting to integer data, and a core dump will appear.
>>
>> The original problem was really fixed with:
>> 60815698c0a Refactor parse_num and use it to parse optarg.
>> Which added a check for NULL in 'optarg' before moving it
>> to the 'increments' variable.
>>
>> New issue: When trying to remove a device using the short
>> argument -r, instead of the long argument --remove, the
>> output is empty. The problem started when commit
>> 546047688e1c was added.
>>
>> Steps to Reproduce:
>> 1. create/assemble /dev/md0 device
>> 2. mdadm --manage /dev/md0 -r /dev/vdxx
>>
>> Actual results:
>> Nothing, empty output, nothing happens, the device is still
>> connected to the array.
>>
>> The output should have stated "mdadm: hot remove failed
>> for /dev/vdxx: Device or resource busy", if the device was
>> still active. Or it should remove the device and print
>> a message:
>>
>> mdadm: set /dev/vdd faulty in /dev/md0
>> mdadm: hot removed /dev/vdd from /dev/md0
>>
>> The following commit should be reverted as it breaks
>> mdadm --manage -r.
>>
>> commit 546047688e1c64638f462147c755b58119cabdc8
>> Author: Wu Guanghao <wuguanghao3@huawei.com>
>> Date:   Mon Aug 16 15:24:51 2021 +0800
>> mdadm: fix coredump of mdadm --monitor -r
>>
>> -Nigel
>>
>> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
>> ---
>>   ReadMe.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/ReadMe.c b/ReadMe.c
>> index 8f873c48..bec1be9a 100644
>> --- a/ReadMe.c
>> +++ b/ReadMe.c
>> @@ -81,11 +81,11 @@ 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_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},
> 
> Jes, That is the status of this patch?
> 
> Thanks, Nigel


Jes, Is there an issue with reverting this patch?

-Nigel


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r"
  2022-06-17 17:09   ` Nigel Croxon
@ 2022-06-17 19:35     ` Jes Sorensen
  2022-06-20 16:13       ` Coly Li
  0 siblings, 1 reply; 8+ messages in thread
From: Jes Sorensen @ 2022-06-17 19:35 UTC (permalink / raw)
  To: Nigel Croxon, linux-raid, mariusz.tkaczyk, wuguanghao3, colyli

On 6/17/22 13:09, Nigel Croxon wrote:
> On 6/14/22 10:11 AM, Nigel Croxon wrote:
>> On 4/18/22 1:44 PM, Nigel Croxon wrote:
>>> This reverts commit 546047688e1c64638f462147c755b58119cabdc8.
>>>
>>> The change from commit mdadm: fix coredump of mdadm
>>> --monitor -r broke the printing of the return message when
>>> passing -r to mdadm --manage, the removal of a device from
>>> an array.
>>>
>>> If the current code reverts this commit, both issues are
>>> still fixed.
>>>
>>> The original problem reported that the fix tried to address
>>> was:  The --monitor -r option requires a parameter,
>>> otherwise a null pointer will be manipulated when
>>> converting to integer data, and a core dump will appear.
>>>
>>> The original problem was really fixed with:
>>> 60815698c0a Refactor parse_num and use it to parse optarg.
>>> Which added a check for NULL in 'optarg' before moving it
>>> to the 'increments' variable.
>>>
>>> New issue: When trying to remove a device using the short
>>> argument -r, instead of the long argument --remove, the
>>> output is empty. The problem started when commit
>>> 546047688e1c was added.
>>>
>>> Steps to Reproduce:
>>> 1. create/assemble /dev/md0 device
>>> 2. mdadm --manage /dev/md0 -r /dev/vdxx
>>>
>>> Actual results:
>>> Nothing, empty output, nothing happens, the device is still
>>> connected to the array.
>>>
>>> The output should have stated "mdadm: hot remove failed
>>> for /dev/vdxx: Device or resource busy", if the device was
>>> still active. Or it should remove the device and print
>>> a message:
>>>
>>> mdadm: set /dev/vdd faulty in /dev/md0
>>> mdadm: hot removed /dev/vdd from /dev/md0
>>>
>>> The following commit should be reverted as it breaks
>>> mdadm --manage -r.
>>>
>>> commit 546047688e1c64638f462147c755b58119cabdc8
>>> Author: Wu Guanghao <wuguanghao3@huawei.com>
>>> Date:   Mon Aug 16 15:24:51 2021 +0800
>>> mdadm: fix coredump of mdadm --monitor -r
>>>
>>> -Nigel
>>>
>>> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
>>> ---
>>>   ReadMe.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/ReadMe.c b/ReadMe.c
>>> index 8f873c48..bec1be9a 100644
>>> --- a/ReadMe.c
>>> +++ b/ReadMe.c
>>> @@ -81,11 +81,11 @@ 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_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},
>>
>> Jes, That is the status of this patch?
>>
>> Thanks, Nigel
> 
> 
> Jes, Is there an issue with reverting this patch?

The fact that I am swamped with my regular work and it hadn't made it
into patchworks. It's applied now.

Jes



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r"
  2022-06-17 19:35     ` Jes Sorensen
@ 2022-06-20 16:13       ` Coly Li
  2022-06-20 17:13         ` Jes Sorensen
  0 siblings, 1 reply; 8+ messages in thread
From: Coly Li @ 2022-06-20 16:13 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: Nigel Croxon, linux-raid, Mariusz Tkaczyk, wuguanghao3



> 2022年6月18日 03:35,Jes Sorensen <jes@trained-monkey.org> 写道:
> 
> On 6/17/22 13:09, Nigel Croxon wrote:
>> On 6/14/22 10:11 AM, Nigel Croxon wrote:
>>> On 4/18/22 1:44 PM, Nigel Croxon wrote:
>>>> This reverts commit 546047688e1c64638f462147c755b58119cabdc8.
>>>> 
>>>> The change from commit mdadm: fix coredump of mdadm
>>>> --monitor -r broke the printing of the return message when
>>>> passing -r to mdadm --manage, the removal of a device from
>>>> an array.
>>>> 
[snipped]

>>> 
>>> Jes, That is the status of this patch?
>>> 
>>> Thanks, Nigel
>> 
>> 
>> Jes, Is there an issue with reverting this patch?
> 
> The fact that I am swamped with my regular work and it hadn't made it
> into patchworks. It's applied now.

Hi Jes,

Since I don’t see this patch applied, so I send it again in my “mdadm-CI for-jes/20220620: patches for merge” series with my Acked-by.

Just for your information. Thank you in advance for taking care of them.

Coly Li

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r"
  2022-06-20 16:13       ` Coly Li
@ 2022-06-20 17:13         ` Jes Sorensen
  0 siblings, 0 replies; 8+ messages in thread
From: Jes Sorensen @ 2022-06-20 17:13 UTC (permalink / raw)
  To: Coly Li; +Cc: Nigel Croxon, linux-raid, Mariusz Tkaczyk, wuguanghao3

On 6/20/22 12:13, Coly Li wrote:
> 
> 
>> 2022年6月18日 03:35,Jes Sorensen <jes@trained-monkey.org> 写道:
>>
>> On 6/17/22 13:09, Nigel Croxon wrote:
>>> On 6/14/22 10:11 AM, Nigel Croxon wrote:
>>>> On 4/18/22 1:44 PM, Nigel Croxon wrote:
>>>>> This reverts commit 546047688e1c64638f462147c755b58119cabdc8.
>>>>>
>>>>> The change from commit mdadm: fix coredump of mdadm
>>>>> --monitor -r broke the printing of the return message when
>>>>> passing -r to mdadm --manage, the removal of a device from
>>>>> an array.
>>>>>
> [snipped]
> 
>>>>
>>>> Jes, That is the status of this patch?
>>>>
>>>> Thanks, Nigel
>>>
>>>
>>> Jes, Is there an issue with reverting this patch?
>>
>> The fact that I am swamped with my regular work and it hadn't made it
>> into patchworks. It's applied now.
> 
> Hi Jes,
> 
> Since I don’t see this patch applied, so I send it again in my “mdadm-CI for-jes/20220620: patches for merge” series with my Acked-by.
> 
> Just for your information. Thank you in advance for taking care of them.

Hi Coly,

Thanks, I just forgot to push it on Friday. I'll need to look through
the rest soon.

Cheers,
Jes



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r"
@ 2022-02-09 12:42 Nigel Croxon
  0 siblings, 0 replies; 8+ messages in thread
From: Nigel Croxon @ 2022-02-09 12:42 UTC (permalink / raw)
  To: linux-raid, wuguanghao3, mariusz.tkaczyk, jes

This reverts commit 546047688e1c64638f462147c755b58119cabdc8.

mdadm: fix msg when removing a device using the short arg -r

The change from commit mdadm: fix coredump of mdadm
--monitor -r broke the printing of the return message when
passing -r to mdadm --manage, the removal of a device from
an array.

If the current code reverts this commit, both issues are
still fixed.

The original problem reported that the fix tried to address
was:  The --monitor -r option requires a parameter,
otherwise a null pointer will be manipulated when
converting to integer data, and a core dump will appear.

The original problem was really fixed with:
60815698c0a Refactor parse_num and use it to parse optarg.
Which added a check for NULL in 'optarg' before moving it
to the 'increments' variable.

New issue: When trying to remove a device using the short
argument -r, instead of the long argument --remove, the
output is empty. The problem started when commit 
546047688e1c was added.

Steps to Reproduce:
1. create/assemble /dev/md0 device
2. mdadm --manage /dev/md0 -r /dev/vdxx

Actual results:
Nothing, empty output, nothing happens, the device is still
connected to the array.

The output should have stated "mdadm: hot remove failed
for /dev/vdxx: Device or resource busy", if the device was
still active. Or it should remove the device and print
a message:

# mdadm --set-faulty /dev/md0 /dev/vdd
mdadm: set /dev/vdd faulty in /dev/md0
# mdadm --manage /dev/md0 -r /dev/vdd
mdadm: hot removed /dev/vdd from /dev/md0


The following commit should be reverted as it breaks
mdadm --manage -r.

commit 546047688e1c64638f462147c755b58119cabdc8
Author: Wu Guanghao <wuguanghao3@huawei.com>
Date:   Mon Aug 16 15:24:51 2021 +0800
mdadm: fix coredump of mdadm --monitor -r

-Nigel

Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
---
 ReadMe.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ReadMe.c b/ReadMe.c
index 81399765..ee457a54 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -81,11 +81,11 @@ 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_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},
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-06-20 17:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-18 17:44 [PATCH] Revert "mdadm: fix coredump of mdadm --monitor -r" Nigel Croxon
2022-05-23 15:43 ` Coly Li
2022-06-14 14:11 ` Nigel Croxon
2022-06-17 17:09   ` Nigel Croxon
2022-06-17 19:35     ` Jes Sorensen
2022-06-20 16:13       ` Coly Li
2022-06-20 17:13         ` Jes Sorensen
  -- strict thread matches above, loose matches on Subject: below --
2022-02-09 12:42 Nigel Croxon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.