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

[Shivaprasad G Bhat]

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 <sb...@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;