diff mbox series

ndctl/bus:Handling the scrub related command more gracefully

Message ID 20220427153751.190286-1-tsahu@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series ndctl/bus:Handling the scrub related command more gracefully | expand

Commit Message

Tarun Sahu April 27, 2022, 3:37 p.m. UTC
For the buses, that don't have nfit support, they use to
return "No such file or directory" for start-scrub/
wait-scrub command.

Though, non-nfit support buses do not support start-scrub/
wait-scrub operation. I propose this patch to handle these
commands more gracefully by returning "Operation not
supported".

Previously:
$ ./ndctl start-scrub ndbus0
error starting scrub: No such file or directory

Now:
$ ./ndctl start-scrub ndbus0
error starting scrub: Operation not supported

Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 ndctl/lib/libndctl.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Vaibhav Jain May 2, 2022, 5:17 a.m. UTC | #1
Hi Tarun,

Minor review comments below
Tarun Sahu <tsahu@linux.ibm.com> writes:

> For the buses, that don't have nfit support, they use to
> return "No such file or directory" for start-scrub/
> wait-scrub command.
s/they use to//

>
> Though, non-nfit support buses do not support start-scrub/
> wait-scrub operation. I propose this patch to handle these
> commands more gracefully by returning "Operation not
> supported".
>
s/Though/Presently/
s/non-nfit support buses do not support/non-nfit support buses do not support/

> Previously:
> $ ./ndctl start-scrub ndbus0
> error starting scrub: No such file or directory
>
> Now:
> $ ./ndctl start-scrub ndbus0
> error starting scrub: Operation not supported
>
The code changes look good to me though. But it would be better if you
can describe the test setup that resulted in above output.

I was able to test the patch on PPC64LE guest with an nvdimm device with
following expected results:

# Valid ndbus
$ sudo ./ndctl start-scrub ndbus0
error starting scrub: Operation not supported

# Invalid ndbus
$ sudo ./ndctl start-scrub ndbus5
error starting scrub: No such device or address


Hence,

> Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
Reviewed-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Tested-by: Vaibhav Jain <vaibhav@linux.ibm.com>


> ---
>  ndctl/lib/libndctl.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index ccca8b5..8bfad6a 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -938,10 +938,14 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
>  	if (!bus->wait_probe_path)
>  		goto err_read;
>  
> -	sprintf(path, "%s/device/nfit/scrub", ctl_base);
> -	bus->scrub_path = strdup(path);
> -	if (!bus->scrub_path)
> -		goto err_read;
> +	if (ndctl_bus_has_nfit(bus)) {
> +		sprintf(path, "%s/device/nfit/scrub", ctl_base);
> +		bus->scrub_path = strdup(path);
> +		if (!bus->scrub_path)
> +			goto err_read;
> +	} else {
> +		bus->scrub_path = NULL;
> +	}
>  
>  	sprintf(path, "%s/device/firmware/activate", ctl_base);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
> @@ -1377,6 +1381,9 @@ NDCTL_EXPORT int ndctl_bus_start_scrub(struct ndctl_bus *bus)
>  	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
>  	int rc;
>  
> +	if (bus->scrub_path == NULL)
> +		return -EOPNOTSUPP;
> +
>  	rc = sysfs_write_attr(ctx, bus->scrub_path, "1\n");
>  
>  	/*
> @@ -1447,6 +1454,9 @@ NDCTL_EXPORT int ndctl_bus_poll_scrub_completion(struct ndctl_bus *bus,
>  	char in_progress;
>  	int fd = 0, rc;
>  
> +	if (bus->scrub_path == NULL)
> +		return -EOPNOTSUPP;
> +
>  	fd = open(bus->scrub_path, O_RDONLY|O_CLOEXEC);
>  	if (fd < 0)
>  		return -errno;
> -- 
> 2.35.1
>
>
diff mbox series

Patch

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ccca8b5..8bfad6a 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -938,10 +938,14 @@  static void *add_bus(void *parent, int id, const char *ctl_base)
 	if (!bus->wait_probe_path)
 		goto err_read;
 
-	sprintf(path, "%s/device/nfit/scrub", ctl_base);
-	bus->scrub_path = strdup(path);
-	if (!bus->scrub_path)
-		goto err_read;
+	if (ndctl_bus_has_nfit(bus)) {
+		sprintf(path, "%s/device/nfit/scrub", ctl_base);
+		bus->scrub_path = strdup(path);
+		if (!bus->scrub_path)
+			goto err_read;
+	} else {
+		bus->scrub_path = NULL;
+	}
 
 	sprintf(path, "%s/device/firmware/activate", ctl_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
@@ -1377,6 +1381,9 @@  NDCTL_EXPORT int ndctl_bus_start_scrub(struct ndctl_bus *bus)
 	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
 	int rc;
 
+	if (bus->scrub_path == NULL)
+		return -EOPNOTSUPP;
+
 	rc = sysfs_write_attr(ctx, bus->scrub_path, "1\n");
 
 	/*
@@ -1447,6 +1454,9 @@  NDCTL_EXPORT int ndctl_bus_poll_scrub_completion(struct ndctl_bus *bus,
 	char in_progress;
 	int fd = 0, rc;
 
+	if (bus->scrub_path == NULL)
+		return -EOPNOTSUPP;
+
 	fd = open(bus->scrub_path, O_RDONLY|O_CLOEXEC);
 	if (fd < 0)
 		return -errno;