diff mbox series

[ndctl,4/7] cxl/region: accept user-supplied UUIDs for pmem regions

Message ID 20230120-vv-volatile-regions-v1-4-b42b21ee8d0b@intel.com
State Superseded
Headers show
Series cxl: add support for listing and creating volatile regions | expand

Commit Message

Verma, Vishal L Feb. 7, 2023, 7:16 p.m. UTC 
Attempting to add additional checking around user-supplied UUIDs against
'ram' type regions revealed that commit 21b089025178 ("cxl: add a 'create-region' command")
completely neglected to add the requisite support for accepting
user-supplied UUIDs, even though the man page for cxl-create-region
advertised the option.

Fix this by actually adding this option now, and add checks to validate
the user-supplied UUID, and refuse it for ram regions.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 cxl/region.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

Comments

Ira Weiny Feb. 8, 2023, 3:56 a.m. UTC | #1 
Vishal Verma wrote:
> Attempting to add additional checking around user-supplied UUIDs against
> 'ram' type regions revealed that commit 21b089025178 ("cxl: add a 'create-region' command")
> completely neglected to add the requisite support for accepting
> user-supplied UUIDs, even though the man page for cxl-create-region
> advertised the option.
> 
> Fix this by actually adding this option now, and add checks to validate
> the user-supplied UUID, and refuse it for ram regions.
> 
> Cc: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  cxl/region.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 0945a14..9079b2d 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -22,6 +22,7 @@ static struct region_params {
>  	const char *bus;
>  	const char *size;
>  	const char *type;
> +	const char *uuid;
>  	const char *root_decoder;
>  	const char *region;
>  	int ways;
> @@ -40,6 +41,7 @@ struct parsed_params {
>  	u64 ep_min_size;
>  	int ways;
>  	int granularity;
> +	uuid_t uuid;
>  	struct json_object *memdevs;
>  	int num_memdevs;
>  	int argc;
> @@ -74,6 +76,8 @@ OPT_INTEGER('g', "granularity", &param.granularity,  \
>  	    "granularity of the interleave set"), \
>  OPT_STRING('t', "type", &param.type, \
>  	   "region type", "region type - 'pmem' or 'ram'"), \
> +OPT_STRING('U', "uuid", &param.uuid, \
> +	   "region uuid", "uuid for the new region (default: autogenerate)"), \
>  OPT_BOOLEAN('m', "memdevs", &param.memdevs, \
>  	    "non-option arguments are memdevs"), \
>  OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats")
> @@ -293,6 +297,11 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
>  
>  	if (param.type) {
>  		p->mode = cxl_decoder_mode_from_ident(param.type);
> +		if (p->mode == CXL_DECODER_MODE_RAM && param.uuid) {
> +			log_err(&rl,
> +				"can't set UUID for ram / volatile regions");
> +			return -EINVAL;
> +		}
>  		if (p->mode == CXL_DECODER_MODE_NONE) {
>  			log_err(&rl, "unsupported type: %s\n", param.type);
>  			return -EINVAL;
> @@ -341,6 +350,13 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
>  		}
>  	}
>  
> +	if (param.uuid) {
> +		if (uuid_parse(param.uuid, p->uuid)) {
> +			error("failed to parse uuid: '%s'\n", param.uuid);
> +			return -EINVAL;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -566,7 +582,6 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  	int i, rc, granularity;
>  	u64 size, max_extent;
>  	const char *devname;
> -	uuid_t uuid;
>  
>  	rc = create_region_validate_config(ctx, p);
>  	if (rc)
> @@ -627,8 +642,9 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>  	try(cxl_region, set_interleave_granularity, region, granularity);
>  	try(cxl_region, set_interleave_ways, region, p->ways);
>  	if (p->mode == CXL_DECODER_MODE_PMEM) {
> -		uuid_generate(uuid);
> -		try(cxl_region, set_uuid, region, uuid);
> +		if (!param.uuid)
> +			uuid_generate(p->uuid);
> +		try(cxl_region, set_uuid, region, p->uuid);
>  	}
>  	try(cxl_region, set_size, region, size);
>  
> 
> -- 
> 2.39.1
> 
>
Dan Williams Feb. 8, 2023, 5:51 a.m. UTC | #2 
Vishal Verma wrote:
> Attempting to add additional checking around user-supplied UUIDs against
> 'ram' type regions revealed that commit 21b089025178 ("cxl: add a 'create-region' command")
> completely neglected to add the requisite support for accepting
> user-supplied UUIDs, even though the man page for cxl-create-region
> advertised the option.
> 
> Fix this by actually adding this option now, and add checks to validate
> the user-supplied UUID, and refuse it for ram regions.
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---

Looks good,

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
diff mbox series

Patch

diff --git a/cxl/region.c b/cxl/region.c
index 0945a14..9079b2d 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -22,6 +22,7 @@  static struct region_params {
 	const char *bus;
 	const char *size;
 	const char *type;
+	const char *uuid;
 	const char *root_decoder;
 	const char *region;
 	int ways;
@@ -40,6 +41,7 @@  struct parsed_params {
 	u64 ep_min_size;
 	int ways;
 	int granularity;
+	uuid_t uuid;
 	struct json_object *memdevs;
 	int num_memdevs;
 	int argc;
@@ -74,6 +76,8 @@  OPT_INTEGER('g', "granularity", &param.granularity,  \
 	    "granularity of the interleave set"), \
 OPT_STRING('t', "type", &param.type, \
 	   "region type", "region type - 'pmem' or 'ram'"), \
+OPT_STRING('U', "uuid", &param.uuid, \
+	   "region uuid", "uuid for the new region (default: autogenerate)"), \
 OPT_BOOLEAN('m', "memdevs", &param.memdevs, \
 	    "non-option arguments are memdevs"), \
 OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats")
@@ -293,6 +297,11 @@  static int parse_create_options(struct cxl_ctx *ctx, int count,
 
 	if (param.type) {
 		p->mode = cxl_decoder_mode_from_ident(param.type);
+		if (p->mode == CXL_DECODER_MODE_RAM && param.uuid) {
+			log_err(&rl,
+				"can't set UUID for ram / volatile regions");
+			return -EINVAL;
+		}
 		if (p->mode == CXL_DECODER_MODE_NONE) {
 			log_err(&rl, "unsupported type: %s\n", param.type);
 			return -EINVAL;
@@ -341,6 +350,13 @@  static int parse_create_options(struct cxl_ctx *ctx, int count,
 		}
 	}
 
+	if (param.uuid) {
+		if (uuid_parse(param.uuid, p->uuid)) {
+			error("failed to parse uuid: '%s'\n", param.uuid);
+			return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 
@@ -566,7 +582,6 @@  static int create_region(struct cxl_ctx *ctx, int *count,
 	int i, rc, granularity;
 	u64 size, max_extent;
 	const char *devname;
-	uuid_t uuid;
 
 	rc = create_region_validate_config(ctx, p);
 	if (rc)
@@ -627,8 +642,9 @@  static int create_region(struct cxl_ctx *ctx, int *count,
 	try(cxl_region, set_interleave_granularity, region, granularity);
 	try(cxl_region, set_interleave_ways, region, p->ways);
 	if (p->mode == CXL_DECODER_MODE_PMEM) {
-		uuid_generate(uuid);
-		try(cxl_region, set_uuid, region, uuid);
+		if (!param.uuid)
+			uuid_generate(p->uuid);
+		try(cxl_region, set_uuid, region, p->uuid);
 	}
 	try(cxl_region, set_size, region, size);