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

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

On 4/13/22 09:22, Tarun Sahu wrote:
> Write-infoblock command-
> 1. update the alignment to default value if not passed as parameter
> 2. convert any type of namespace to fsdax if parameter -m is not
> specified
> 3. Incorrectly updating the uuid and parent_uuid if corresponding
> parameter is not specified
> 

May be rephrased to,

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

> Considering the above three issues, we first needed to read the
> original infoblock if available, and update the align, uuid, parent_uuid
> to its original value while writing the infoblock if corresponding
> parameter is not passed.
> 

<snip>

> 
> This patch change the declaration of following functions to pass
> ns_info,
> 
> file_read_infoblock()
> file_write_infoblock()
> parse_namespace_infoblock()
> write_pfn_sb()
> 
> Before and after the patch results:

The test results bloat up the commit log, just keep the "after" OR may 
be move it below "---". Few of the implementation explanations from the 
  description summary can be moved down too. The commit log that way 
will have crisp summary, and details being moved to the bottom of "---".


<snip>
> +		ns_info_destroy(&ns_info);
>   		return rc;
>   	}
>   
> @@ -2447,3 +2530,5 @@ int cmd_write_infoblock(int argc, const char **argv, struct ndctl_ctx *ctx)
>   	fprintf(stderr, "wrote %d infoblock%s\n", write, write == 1 ? "" : "s");
>   	return rc;
>   }
> +
> +

Blank line insertions

> diff --git a/ndctl/namespace.h b/ndctl/namespace.h
> index 57735eb..a406c18 100644
> --- a/ndctl/namespace.h
> +++ b/ndctl/namespace.h
> @@ -8,6 +8,7 @@
>   #include <util/fletcher.h>
>   #include <ccan/endian/endian.h>
>   #include <ccan/short_types/short_types.h>
> +#include <ndctl/libndctl.h>
>   
>   enum {
>   	NSINDEX_SIG_LEN = 16,
> @@ -233,6 +234,12 @@ union info_block {
>   	struct btt_sb btt_sb;
>   };
>   
> +struct ns_info {
> +	void *ns_sb_buf;
> +	enum ndctl_namespace_mode mode;
> +	size_t offset;
> +};
> +

This structure is used only inside the namespace.c. Can be defined
in the namaespace.c itself instead of the header.

>   static inline bool verify_infoblock_checksum(union info_block *sb)
>   {
>   	uint64_t sum;