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