On Thu, 2024-10-17 at 08:23 -0400, Laurence Oberman wrote:
> 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
>
Hello Mariusz and Xiao
I re-ran all the tests in house and I can actually start the arrays
that have the ":" in the names, just not create new ones. So I think we
leave this as is and we keep the adherence to POSIX for newly created
arrays only.
My patch or request for a change for creation can be denied, its fine.
Regards
Laurence
