All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ndctl/bus:Handling the scrub related command more gracefully
@ 2022-04-27 15:37 Tarun Sahu
  2022-05-02  5:17 ` Vaibhav Jain
  0 siblings, 1 reply; 2+ messages in thread
From: Tarun Sahu @ 2022-04-27 15:37 UTC (permalink / raw)
  To: nvdimm
  Cc: tsahu, dan.j.williams, vishal.l.verma, aneesh.kumar, sbhat, vaibhav

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(-)

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ndctl/bus:Handling the scrub related command more gracefully
  2022-04-27 15:37 [PATCH] ndctl/bus:Handling the scrub related command more gracefully Tarun Sahu
@ 2022-05-02  5:17 ` Vaibhav Jain
  0 siblings, 0 replies; 2+ messages in thread
From: Vaibhav Jain @ 2022-05-02  5:17 UTC (permalink / raw)
  To: Tarun Sahu, nvdimm
  Cc: tsahu, dan.j.williams, vishal.l.verma, aneesh.kumar, sbhat

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
>
>

-- 
Cheers
~ Vaibhav

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-02  5:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-27 15:37 [PATCH] ndctl/bus:Handling the scrub related command more gracefully Tarun Sahu
2022-05-02  5:17 ` Vaibhav Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.