On Tue, 8 Feb 2022 10:14:50 +0100 Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx> wrote: > On Mon, 7 Feb 2022 17:15:19 -0500 > Nigel Croxon <ncroxon@xxxxxxxxxx> wrote: > > > 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@xxxxxxxxxx> > > Date: Mon Aug 16 15:24:51 2021 +0800 > > mdadm: fix coredump of mdadm --monitor -r > > > > -Nigel > > > > Signed-off-by: Nigel Croxon <ncroxon@xxxxxxxxxx> > > > > > Hi, > Thanks for the reminder. Revert is obviously correct. Jes could you > merge it? > > In systemd world mdmonitor is rarely started from cmdline. If > "increment" option is not settable via config then it is probably > unused. I consider this option as deprecated and I suspect that no one > is using it now. Can we remove it completely? > > If you disagree with that, then we should add support for it in config > file to make it usable. Additionally, I suggest to change "-r" for > "increments" to short opt always used with parameter. > Hi Coly, Could you take a look? Thanks, Mariusz
