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.
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.
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/
