On Tue, Mar 18, 2025 at 4:24 PM Paul Menzel <pmenzel@xxxxxxxxxxxxx> wrote:
>
> Dear Xiao,
>
>
>
> Am 18.03.25 um 09:18 schrieb Xiao Ni:
> > On Tue, Mar 18, 2025 at 3:38 PM Paul Menzel <pmenzel@xxxxxxxxxxxxx> wrote:
>
> >> Thank you for the patch. Wasn’t a similar patch sent to the list already
> >> months ago?
> >>
> >> For the commit message subject/title I suggest:
> >>
> >> mdadm: Allow to assemble existing arrays with non-POSIX names
>
> > Thanks for reminding me about this. I forgot about this patch. I can't
> > find the patch. Do you still have the link for it?
>
> I searched for *e2eb503bd797* in the linux-raid archive [1], I found the
> patch and discussion *[PATCH] Add the ":" to the allowed_symbols list to
> work with the latest POSIX changes* [2]. So, similar, and I think only
> the topic of already created arrays came up, your patch solves. Sorry
> for the confusion.
Thanks for finding it. It doesn't resolve the problem. So this patch
tries to resolve the problem.
>
> >> Am 18.03.25 um 06:46 schrieb Xiao Ni:
> >>> It's good to has limitation for name when creating an array. But the arrays
> >>
> >> … to have limitations …
> >>
> >>> which were created before patch e2eb503bd797 (mdadm: Follow POSIX Portable
> >>> Character Set) can't be assembled. In this patch, it removes the posix
> >>> check for assemble mode.
> >>
> >> “In this patch” is redundant in the commit message. Maybe:
> >>
> >> So, remove the POSIX check for assemble mode.
> >
> > good to me.
> >
> >> Maybe add how to reproduce this? Is there a way to create a non-POSIX
> >> name with current mdadm, or should such a file be provided for tests.
> >
> > For example:
> >
> > * build mdadm without patch e2eb503bd797
> > * mdadm -CR /dev/md/tbz:pv1 -l0 -n2 /dev/loop0 /dev/loop1
> > * mdadm -Ss
> > * build with latest mdadm, and try to assemble it.
> > * mdadm -A /dev/md/tbz:pv1 --name tbz:pv1
> > mdadm: Value "tbz:pv1" cannot be set as name. Reason: Not POSIX compatible.
>
> Thank you. For running tests, re-building mdadm might not be feasible.
> But having these instructions would be great to have in the commit
> message nevertheless.
It's right. I'll add them into the commit message.
Regards
Xiao
>
> >>> Fixes: e2eb503bd797 (mdadm: Follow POSIX Portable Character Set)
> >>> Signed-off-by: Xiao Ni <xni@xxxxxxxxxx>
> >>> ---
> >>> config.c | 8 ++------
> >>> mdadm.c | 11 +++++++++++
> >>> 2 files changed, 13 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/config.c b/config.c
> >>> index 8a8ae5e48c41..ef7dbc4eb29f 100644
> >>> --- a/config.c
> >>> +++ b/config.c
> >>> @@ -208,11 +208,6 @@ static mdadm_status_t ident_check_name(const char *name, const char *prop_name,
> >>> return MDADM_STATUS_ERROR;
> >>> }
> >>>
> >>> - if (!is_name_posix_compatible(name)) {
> >>> - ident_log(prop_name, name, "Not POSIX compatible", cmdline);
> >>> - return MDADM_STATUS_ERROR;
> >>> - }
> >>> -
> >>> return MDADM_STATUS_SUCCESS;
> >>> }
> >>>
> >>> @@ -512,7 +507,8 @@ void arrayline(char *line)
> >>>
> >>> for (w = dl_next(line); w != line; w = dl_next(w)) {
> >>> if (w[0] == '/' || strchr(w, '=') == NULL) {
> >>> - _ident_set_devname(&mis, w, false);
> >>> + if (is_name_posix_compatible(w))
> >>> + _ident_set_devname(&mis, w, false);
> >>> } else if (strncasecmp(w, "uuid=", 5) == 0) {
> >>> if (mis.uuid_set)
> >>> pr_err("only specify uuid once, %s ignored.\n",
> >>> diff --git a/mdadm.c b/mdadm.c
> >>> index 6200cd0e7f9b..9d5b0e567799 100644
> >>> --- a/mdadm.c
> >>> +++ b/mdadm.c
> >>> @@ -732,6 +732,11 @@ int main(int argc, char *argv[])
> >>> exit(2);
> >>> }
> >>>
> >>> + if (mode != ASSEMBLE && !is_name_posix_compatible(optarg)) {
> >>> + pr_err("%s Not POSIX compatible\n", optarg);
> >>> + exit(2);
> >>> + }
> >>> +
> >>> if (ident_set_name(&ident, optarg) != MDADM_STATUS_SUCCESS)
> >>> exit(2);
> >>>
> >>> @@ -1289,6 +1294,12 @@ int main(int argc, char *argv[])
> >>> pr_err("an md device must be given in this mode\n");
> >>> exit(2);
> >>> }
> >>> +
> >>> + if (mode != ASSEMBLE && !is_name_posix_compatible(devlist->devname)) {
> >>> + pr_err("%s Not POSIX compatible\n", devlist->devname);
> >>> + exit(2);
> >>> + }
> >>> +
> >>> if (ident_set_devname(&ident, devlist->devname) != MDADM_STATUS_SUCCESS)
> >>> exit(1);
>
> Kind regards,
>
> Paul
>
>
> [1]: https://lore.kernel.org/linux-raid/?q=e2eb503bd797
> [2]:
> https://lore.kernel.org/linux-raid/20241015173553.276546-1-loberman@xxxxxxxxxx/
>
