Re: [PATCH 1/1] mdadm: check posix name before setting name and devname

[Paul Menzel <pmenzel@xxxxxxxxxxxxx>]

Dear Xiao,


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

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.

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.

> 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);
>