Message ID | 20180418063242.9221-1-qi.fuli@jp.fujitsu.com (mailing list archive) |
---|---|
State | Accepted |
Commit | c70adc3 |
Headers | show |
On Tue, Apr 17, 2018 at 11:32 PM, QI Fuli <qi.fuli@jp.fujitsu.com> wrote: > This patch refactors util_<obj>_filter to support multiple space-seperated > arguments. Currently, only one <object> can be filtered by <object>'s name > in util_<obj>_filter(). As a result, when users want to moniotr multiple > dimms, they have to run multiple monitor processes. This feature teachs the > util_dimm_filter() that the "ident" argument maybe a space-seperatd string > which includes multiple dimm_names. Therefore the monitor can filter > multiple dimms in one process. > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > Change log since v1: > - Removing OPT_STRING_LIST from parse-option Looks good: Reviewed-by: Dan Williams <dan.j.williams@intel.com>
On Wed, 2018-04-18 at 07:31 -0700, Dan Williams wrote: > On Tue, Apr 17, 2018 at 11:32 PM, QI Fuli <qi.fuli@jp.fujitsu.com> wrote: > > This patch refactors util_<obj>_filter to support multiple space- > > seperated > > arguments. Currently, only one <object> can be filtered by <object>'s > > name > > in util_<obj>_filter(). As a result, when users want to moniotr > > multiple > > dimms, they have to run multiple monitor processes. This feature teachs > > the > > util_dimm_filter() that the "ident" argument maybe a space-seperatd > > string > > which includes multiple dimm_names. Therefore the monitor can filter > > multiple dimms in one process. > > > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> I'll fix up the tags like so: Based-on-a-patch-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> Thanks, -Vishal > > > > Change log since v1: > > - Removing OPT_STRING_LIST from parse-option > > Looks good: > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > _______________________________________________ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm
> From: Verma, Vishal L [mailto:vishal.l.verma@intel.com] > Sent: Thursday, April 19, 2018 1:06 AM > To: Williams, Dan J <dan.j.williams@intel.com>; Qi, Fuli/斉 福利 > <qi.fuli@jp.fujitsu.com> > Cc: linux-nvdimm@lists.01.org > Subject: Re: [PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple > space-seperated arguments > > On Wed, 2018-04-18 at 07:31 -0700, Dan Williams wrote: > > On Tue, Apr 17, 2018 at 11:32 PM, QI Fuli <qi.fuli@jp.fujitsu.com> wrote: > > > This patch refactors util_<obj>_filter to support multiple space- > > > seperated arguments. Currently, only one <object> can be filtered by > > > <object>'s name in util_<obj>_filter(). As a result, when users want > > > to moniotr multiple dimms, they have to run multiple monitor > > > processes. This feature teachs the > > > util_dimm_filter() that the "ident" argument maybe a space-seperatd > > > string which includes multiple dimm_names. Therefore the monitor can > > > filter multiple dimms in one process. > > > > > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > I'll fix up the tags like so: > > Based-on-a-patch-by: Dan Williams <dan.j.williams@intel.com> > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > > > Thanks, > -Vishal > > > > > > > Change log since v1: > > > - Removing OPT_STRING_LIST from parse-option > > > > Looks good: > > > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> Thank you very much. - Qi
When I ran the command # ndctl list --dimm="all nmem1 nmem2", I found only "nmem1" and "nmem2" can be listed, the keyword "all" was ignored. I want to confirm is this result expected? If not, I will make a follow up patch to fix this bug. > -----Original Message----- > From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of Qi, > Fuli > Sent: Thursday, April 19, 2018 11:10 AM > To: 'Verma, Vishal L' <vishal.l.verma@intel.com>; Williams, Dan J > <dan.j.williams@intel.com> > Cc: linux-nvdimm@lists.01.org > Subject: RE: [PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple > space-seperated arguments > > > From: Verma, Vishal L [mailto:vishal.l.verma@intel.com] > > Sent: Thursday, April 19, 2018 1:06 AM > > To: Williams, Dan J <dan.j.williams@intel.com>; Qi, Fuli/斉 福利 > > <qi.fuli@jp.fujitsu.com> > > Cc: linux-nvdimm@lists.01.org > > Subject: Re: [PATCH v2] ndctl, filter: refacor util_<obj>_filter() to > > support multiple space-seperated arguments > > > > On Wed, 2018-04-18 at 07:31 -0700, Dan Williams wrote: > > > On Tue, Apr 17, 2018 at 11:32 PM, QI Fuli <qi.fuli@jp.fujitsu.com> wrote: > > > > This patch refactors util_<obj>_filter to support multiple space- > > > > seperated arguments. Currently, only one <object> can be filtered > > > > by <object>'s name in util_<obj>_filter(). As a result, when users > > > > want to moniotr multiple dimms, they have to run multiple monitor > > > > processes. This feature teachs the > > > > util_dimm_filter() that the "ident" argument maybe a > > > > space-seperatd string which includes multiple dimm_names. > > > > Therefore the monitor can filter multiple dimms in one process. > > > > > > > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > > > I'll fix up the tags like so: > > > > Based-on-a-patch-by: Dan Williams <dan.j.williams@intel.com> > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > > > > > > Thanks, > > -Vishal > > > > > > > > > > Change log since v1: > > > > - Removing OPT_STRING_LIST from parse-option > > > > > > Looks good: > > > > > > Reviewed-by: Dan Williams <dan.j.williams@intel.com> > > Thank you very much. > - Qi > _______________________________________________ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm
On Tue, Apr 24, 2018 at 7:39 PM, Qi, Fuli <qi.fuli@jp.fujitsu.com> wrote: > When I ran the command # ndctl list --dimm="all nmem1 nmem2", I found > only "nmem1" and "nmem2" can be listed, the keyword "all" was ignored. > I want to confirm is this result expected? > If not, I will make a follow up patch to fix this bug. Yes, good catch, that's a bug.
diff --git a/util/filter.c b/util/filter.c index 6ab391a..0d3cc02 100644 --- a/util/filter.c +++ b/util/filter.c @@ -25,101 +25,144 @@ #define NUMA_NO_NODE (-1) -struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char *ident) +struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char *__ident) { - char *end = NULL; + char *end = NULL, *ident, *save; unsigned long bus_id, id; - const char *provider, *devname; + const char *provider, *devname, *name; - if (!ident || strcmp(ident, "all") == 0) + if (!__ident || strcmp(__ident, "all") == 0) return bus; - bus_id = strtoul(ident, &end, 0); - if (end == ident || end[0]) - bus_id = ULONG_MAX; + ident = strdup(__ident); + if (!ident) + return NULL; - provider = ndctl_bus_get_provider(bus); - devname = ndctl_bus_get_devname(bus); - id = ndctl_bus_get_id(bus); + for (name = strtok_r(ident, " ", &save); name; + name = strtok_r(NULL, " ", &save)) { + bus_id = strtoul(ident, &end, 0); + if (end == ident || end[0]) + bus_id = ULONG_MAX; - if (bus_id < ULONG_MAX && bus_id == id) - return bus; + provider = ndctl_bus_get_provider(bus); + devname = ndctl_bus_get_devname(bus); + id = ndctl_bus_get_id(bus); - if (bus_id == ULONG_MAX && (strcmp(ident, provider) == 0 - || strcmp(ident, devname) == 0)) - return bus; + if (bus_id < ULONG_MAX && bus_id == id) + break; + if (bus_id == ULONG_MAX && (strcmp(provider, name) == 0 + || strcmp(devname, name) == 0)) + break; + } + free(ident); + + if (name) + return bus; return NULL; } struct ndctl_region *util_region_filter(struct ndctl_region *region, - const char *ident) + const char *__ident) { - char *end = NULL; - const char *name; + char *end = NULL, *ident, *save; + const char *name, *region_name; unsigned long region_id, id; - if (!ident || strcmp(ident, "all") == 0) + if (!__ident || strcmp(__ident, "all") == 0) return region; - region_id = strtoul(ident, &end, 0); - if (end == ident || end[0]) - region_id = ULONG_MAX; + ident = strdup(__ident); + if (!ident) + return NULL; - name = ndctl_region_get_devname(region); - id = ndctl_region_get_id(region); + for (name = strtok_r(ident, " ", &save); name; + name = strtok_r(NULL, " ", &save)) { + region_id = strtoul(ident, &end, 0); + if (end == ident || end[0]) + region_id = ULONG_MAX; - if (region_id < ULONG_MAX && region_id == id) - return region; + region_name = ndctl_region_get_devname(region); + id = ndctl_region_get_id(region); - if (region_id == ULONG_MAX && strcmp(ident, name) == 0) - return region; + if (region_id < ULONG_MAX && region_id == id) + break; + + if (region_id == ULONG_MAX && strcmp(region_name, name) == 0) + break; + } + free(ident); + if (name) + return region; return NULL; } struct ndctl_namespace *util_namespace_filter(struct ndctl_namespace *ndns, - const char *ident) + const char *__ident) { struct ndctl_region *region = ndctl_namespace_get_region(ndns); unsigned long region_id, ndns_id; + const char *name; + char *ident, *save; - if (!ident || strcmp(ident, "all") == 0) + if (!__ident || strcmp(__ident, "all") == 0) return ndns; - if (strcmp(ident, ndctl_namespace_get_devname(ndns)) == 0) - return ndns; + ident = strdup(__ident); + if (!ident) + return NULL; - if (sscanf(ident, "%ld.%ld", ®ion_id, &ndns_id) == 2 - && ndctl_region_get_id(region) == region_id - && ndctl_namespace_get_id(ndns) == ndns_id) - return ndns; + for (name = strtok_r(ident, " ", &save); name; + name = strtok_r(NULL, " ", &save)) { + if (strcmp(name, ndctl_namespace_get_devname(ndns)) == 0) + break; + + if (sscanf(name, "%ld.%ld", ®ion_id, &ndns_id) == 2 + && ndctl_region_get_id(region) == region_id + && ndctl_namespace_get_id(ndns) == ndns_id) + break; + } + free(ident); + if (name) + return ndns; return NULL; } -struct ndctl_dimm *util_dimm_filter(struct ndctl_dimm *dimm, const char *ident) +struct ndctl_dimm *util_dimm_filter(struct ndctl_dimm *dimm, + const char *__ident) { - char *end = NULL; - const char *name; + char *end = NULL, *ident, *save; + const char *name, *dimm_name; unsigned long dimm_id, id; - if (!ident || strcmp(ident, "all") == 0) + if (!__ident || strcmp(__ident, "all") == 0) return dimm; - dimm_id = strtoul(ident, &end, 0); - if (end == ident || end[0]) - dimm_id = ULONG_MAX; + ident = strdup(__ident); + if (!ident) + return NULL; - name = ndctl_dimm_get_devname(dimm); - id = ndctl_dimm_get_id(dimm); + for (name = strtok_r(ident, " ", &save); name; + name = strtok_r(NULL, " ", &save)) { + dimm_id = strtoul(ident, &end, 0); + if (end == ident || end[0]) + dimm_id = ULONG_MAX; - if (dimm_id < ULONG_MAX && dimm_id == id) - return dimm; + dimm_name = ndctl_dimm_get_devname(dimm); + id = ndctl_dimm_get_id(dimm); - if (dimm_id == ULONG_MAX && strcmp(ident, name) == 0) - return dimm; + if (dimm_id < ULONG_MAX && dimm_id == id) + break; + + if (dimm_id == ULONG_MAX && strcmp(dimm_name, name) == 0) + break; + } + free(ident); + if (name) + return dimm; return NULL; }