Re: [PATCH v3 1/2] ndctl/namespace:Fix multiple issues with write-infoblock

[Shivaprasad G Bhat <sbhat@linux.ibm.com>]

On 4/26/22 22:50, Tarun Sahu wrote:
> Write-infoblock command has the below issues,
> 1 - Oerwriting the existing alignment value with the default value when
>        not passed as parameter.
> 2 - Changing the mode of the namespace to fsdax when -m not specified
> 3 - Incorrectly updating the uuid and parent_uuid if corresponding
> parameter is not specified

<snip>

> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index 257b58c..cca9a51 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c

<snip>

> @@ -2026,12 +2105,10 @@ static int namespace_rw_infoblock(struct ndctl_namespace *ndns,
>   		struct read_infoblock_ctx *ri_ctx, int write)
>   {
>   	int rc;
> -	uuid_t uuid;
> -	char str[40];
>   	char path[50];
> -	const char *save;
>   	const char *cmd = write ? "write-infoblock" : "read-infoblock";
>   	const char *devname = ndctl_namespace_get_devname(ndns);
> +	struct ns_info ns_info;
>   
>   	if (ndctl_namespace_is_active(ndns)) {
>   		pr_verbose("%s: %s enabled, must be disabled\n", cmd, devname);
> @@ -2045,21 +2122,22 @@ static int namespace_rw_infoblock(struct ndctl_namespace *ndns,
>   		goto out;

This would lead to calling ns_info_destroy() on uninitialized ns_info, 
and ns_info->ns_sb_buf would have junk value.

With that fixed,

Reviewed-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>

>   	}
>   
> -	save = param.parent_uuid;
> -	if (!param.parent_uuid) {
> -		ndctl_namespace_get_uuid(ndns, uuid);
> -		uuid_unparse(uuid, str);
> -		param.parent_uuid = str;
> -	}
> -
>   	sprintf(path, "/dev/%s", ndctl_namespace_get_block_device(ndns));
> -	if (write) {
> +	if (ns_info_init(&ns_info) != 0)
> +		goto out;
> +
> +	rc = file_read_infoblock(path, ndns, ri_ctx, &ns_info);
> +	if (!rc && write) {
>   		unsigned long long align;
>   		bool align_provided = true;
>   
>   		if (!param.align) {
>   			align = ndctl_get_default_alignment(ndns);
> -
> +			if (ns_info.mode == NDCTL_NS_MODE_FSDAX ||
> +					ns_info.mode == NDCTL_NS_MODE_DEVDAX) {
> +				align = ((struct pfn_sb *)(ns_info.ns_sb_buf + ns_info.offset))->
> +					align;
> +			}
>   			if (asprintf((char **)&param.align, "%llu", align) < 0) {
>   				rc = -EINVAL;
>   				goto out;
> @@ -2078,18 +2156,16 @@ static int namespace_rw_infoblock(struct ndctl_namespace *ndns,
>   				rc = -EINVAL;
>   			}
>   		}
> -
>   		if (!rc)
> -			rc = file_write_infoblock(path);
> +			rc = file_write_infoblock(path, &ns_info);
>   
>   		if (!align_provided) {
>   			free((char *)param.align);
>   			param.align = NULL;
>   		}
> -	} else
> -		rc = file_read_infoblock(path, ndns, ri_ctx);
> -	param.parent_uuid = save;
> +	}
>   out:
> +	ns_info_destroy(&ns_info);
>   	ndctl_namespace_set_raw_mode(ndns, 0);
>   	ndctl_namespace_disable_invalidate(ndns);
>   	return rc;