On Thu, 2024-10-17 at 13:26 +0200, Mariusz Tkaczyk wrote:
> On Thu, 17 Oct 2024 17:24:59 +0800
> Xiao Ni <xni@xxxxxxxxxx> wrote:
>
> > On Wed, Oct 16, 2024 at 9:11 PM Laurence Oberman
> > <loberman@xxxxxxxxxx> wrote:
> > >
> > > On Wed, 2024-10-16 at 10:14 +0200, Mariusz Tkaczyk wrote:
> > > > On Tue, 15 Oct 2024 13:35:24 -0400
> > > > Laurence Oberman <loberman@xxxxxxxxxx> wrote:
> > > >
> > > > Hello Laurence,
> > > > Thanks for the patch.
> > > >
> > > > ":" is used internally by native metadata name, we have
> > > > "hostname:name". We are
> > > > searching for it specifically, for that reason I think that I
> > > > cannot
> > > > accept it.
> > > > Name must stay simple.
> > > >
> > > > If you want this again, I need to full set of mdadm tests that
> > > > is
> > > > covering every
> > > > scenario and is confirming that we are able to determine
> > > > hostname (if
> > > > exists)
> > > > and name in every case correctly.
> > > >
> > > > There are some workaround in code for that, I can see that we
> > > > are not
> > > > appending
> > > > homehost if ":" is in the name. It is not user friendly. I
> > > > prefer to
> > > > not
> > > > allow ":" to keep in simpler unless you have really good reason
> > > > to
> > > > have it back
> > > > - there is no reason in commit message.
> > > >
> > > >
> > > > > Signed-off-by: Laurence Oberman <loberman@xxxxxxxxxx>
> > > > > ---
> > > > > lib.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/lib.c b/lib.c
> > > > > index f36ae03a..cb4b6a0f 100644
> > > > > --- a/lib.c
> > > > > +++ b/lib.c
> > > > > @@ -485,7 +485,7 @@ bool is_name_posix_compatible(const char
> > > > > *
> > > > > const name)
> > > > > {
> > > > > assert(name);
> > > > >
> > > > > - char allowed_symbols[] = "-_.";
> > > > > + char allowed_symbols[] = "-_.:";
> > > >
> > > > Because the function has been made to follow POSIX, this cannot
> > > > be
> > > > simply added
> > > > here. Please at least explain that in description.
> > > >
> > > > It is not POSIX compatible with this change.
> > > >
> > > >
> > > > > const char *n = name;
> > > > >
> > > > > if (!is_string_lq(name, NAME_MAX))
> > > >
> > > > Thanks,
> > > > Mariusz
> > > >
> > >
> > > Hello
> > > Apologies Christoph and Mariusz, this could have definitely done
> > > with
> > > more of an explanation.
> > >
> > > We have customers complaining about a regression in mdadm since
> > > these
> > > changes happened. They have 1000's of raid devices that can no
> > > longer
> > > be started because they have ":" in the name. Example
> > > mdadm: Value "tbz:pv1" cannot be set as devname. Reason: Not
> > > POSIX
> > > compatible.
> > >
> > > Should we add a --legacy flag where the original behavior is
> > > still an
> > > option, what are your thoughts ?
> > >
> > > Regards
> > > Laurence
> > >
> > >
> >
> > Hi Laurence and Mariusz
> >
> > Can we allow assemble an array whose devname has ":"? So the users
> > can
> > assemble the arrays which were created before patch e2eb503bd797
> > (mdadm: Follow POSIX Portable Character Set).
> >
> > For creating an array, we can still follow the POSIX policy. So it
> > can
> > keep the naming rule simpler.
> >
>
> Thanks Xiao,
>
> Please give me some time to reproduce and understand this case. I
> will find some
> time on monday, maybe tomorrow.
>
> I hope that works for you guys.
>
> Mariusz
>
Hello
Yes, Thank you, if we can have the option to start already created
arrays but make customers aware of the POSIX limitation in creating new
arrays that would be fine with me.
Regards
Laurence
