* [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.