Re: [PATCH - mdadm] mdopen: always try create_named_array()

["NeilBrown" <neilb@suse.de>]

On Wed, 22 Mar 2023, Xiao Ni wrote:
> On Tue, Mar 14, 2023 at 8:08 AM NeilBrown <neilb@suse.de> wrote:
> >
> >
> > mdopen() will use create_named_array() to ask the kernel to create the
> > given md array, but only if it is given a number or name.
> > If it is NOT given a name and is required to choose one itself using
> > find_free_devnm() it does NOT use create_named_array().
> >
> > On kernels with CONFIG_BLOCK_LEGACY_AUTOLOAD not set, this can result in
> > failure to assemble an array.  This can particularly seen when the
> > "name" of the array begins with a host name different to the name of the
> > host running the command.
> >
> > So add the missing call to create_named_array().
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217074
> 
> Hi Neil
> 
> I have two questions, hope you can help to understand the function
> create_mddev better.
> 
> Frist, from the comment7 of the bug you mentioned:
> 
> There are two different sorts names.  Note that you almost
> acknowledged this by writing "name for my md device node" while the
> documentation only talks about names for "md devices", not for "md
> device nodes".
> 
> There are
> 1/ there are names in /dev or /dev/md/ (device nodes)
> 2/ there are names that appear in /proc/mdstat and in /sys/block/ (devices)
> 
> Thanks for the clarification. But it looks like it doesn't work like
> what you said.
> For example:
> mdadm -CR /dev/md/root -l0 -n2 /dev/sda /dev/sdc --name=test
> cat /proc/mdstat
> Personalities : [raid0]
> md127 : active raid0 sdc[1] sda[0]
>       3906764800 blocks super 1.2 512k chunks
> cd /sys/block/md127/md/
> 
> In /proc/mdstat and /sys/block, they all use md127 rather than the name(root)

Try again with "CREATE names=yes" in /etc/mdadm.conf.

mdadm generally tries to keep:
  - the names in /dev/
  - the names in /dev/md/
  - the names in /proc/mdstat
  - the names stored in the metadata

in sync.  It can only do this when:
 - you enabled "names=yes"
 - you don't confuse it by specifying a device name (/dev/md/root) that
   is different from the metadata names "test".

If you don't have "names=yes" then the name in /proc/mdstat and the name
in /dev/md* will be numeric.  The name in /dev/md/ and the name in the
metadata can be different and will usually be the same.

If you explicitly give a different name with --name= than the device
name then obviously they will be different.  If you then stop the array
and restart with "mdadm -As" or "mdadm -I /dev/sda; mdadm -I /dev/sdb"
then mdadm will create a name in /dev/md/ that matches the name in the
metadata.

> 
> Before this patch,  it creates a symbol link with the name root rather than test
> ll /dev/md/root
> lrwxrwxrwx. 1 root root 8 Mar 21 22:35 /dev/md/root -> ../md127

That is what you asked it to do.

> 
> So "test" which is specified by --name looks like it has little usage.
> 

It is stored in the metadata.  You can see it in --examine output.  If
you reassemble the array without specifying a device name, it will use
the name "test".

> 
> By the way, after this patch, the symbol link /dev/md/root can't be
> created anymore.
> Is it a regression problem?

I cannot reproduce any problem like that.  Please provide a sequence of
steps so that I can try to duplicate it.

> 
> Second, are there possibilities that the arguments "dev" and "name" of
> function create_mddev
> are null at the same time?

No.  For Build or Create, dev is never NULL.  For Assemble and
Incremental, name is never NULL.


> After some tests, I found dev can't be null when creating a raid
> device. It can be checked before
> calling create_mddev. And we must get a name after creating a raid
> device. So when assembling
> a raid device, the name must not be null. So the dev and name can't be
> null at the same time, right?

Correct.

NeilBrown


> 
> Best Regards
> Xiao
> 
> 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  mdopen.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/mdopen.c b/mdopen.c
> > index d18c931996d2..810f79a3d19a 100644
> > --- a/mdopen.c
> > +++ b/mdopen.c
> > @@ -370,6 +370,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> >                 }
> >                 if (block_udev)
> >                         udev_block(devnm);
> > +               create_named_array(devnm);
> >         }
> >
> >         sprintf(devname, "/dev/%s", devnm);
> > --
> > 2.39.2
> >
> 
>