On Fri, Apr 19, 2024 at 3:20 PM Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx> wrote: > > On Thu, 18 Apr 2024 18:23:18 +0800 > Xiao Ni <xni@xxxxxxxxxx> wrote: > > > Now 00createnames doesn't check Create names=yes config. Without this > > config, mdadm creates /dev/md127 device node when mdadm --create > > /dev/md/test. With this config, it creates /dev/md_test. This patch > > only adds the check. If it has this config, it returns directly > > without error. > > Hi Xiao, > Thanks for patches I will review them all later (probably next week). > > About this one: > > The proposed change is not complete as config may be read from both > /etc/mdadm.conf and /etc/mdadm/mdadm.conf. Ideally, you should check them > both in the approach you proposed. > > There is also possible to have /etc/mdadm.d/ directory - it is always checked > and read if exists and it cannot be disabled. See load_conffile() and > CONFFILEFLAGS in makefile for details. Hi Mariusz Yes, you're right. Thanks for this. > > Test relies on the global configuration and user may forgot that it is set. > That will give us positive test result because test was not run due to > configuration issue. This is risky, I would prefer fail to indicate > that something is wrong. User can skip this test. Ok, we can keep it the way it is. > > What about adding empty mdadm config to the command `-c ./mdadm_empty.conf`? I > see it as the best option for now. That save use from checking 2 config > locations and any user defined behaviors. Do you see any disadvantages? You mean specifying config file in test case when creating raid? > > As config directory is not configurable we have to accept the risk that > something could be there. > Ideally, you can propose patch with confdir customization to apply same > solution as for conffile (just set it to empty directory) but as it is probably > rarely used we can accept risk here for now (unless somebody reported). I give > it up to you as it not having confdir customization is more like new > feature. I'll remove the adding check in the case. I think few people use the config too. > > Another possible solution would be to learn mdadm print it's configuration and > print it before running test and fail if not compatible setting detected. > > I did not realize that it would be a problem, that for catching! Regards Xiao > > Thanks, > Mariusz >