[PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple space-seperated arguments

[PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple space-seperated arguments

[QI Fuli]

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

---
 util/filter.c | 141 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 92 insertions(+), 49 deletions(-)

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", &region_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", &region_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;
 }
 
-- 
2.17.0.140.g0b0cc9f86


_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple space-seperated arguments

[Dan Williams]

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>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple space-seperated arguments

[Verma, Vishal L]

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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: [PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple space-seperated arguments

[Qi, Fuli]

> 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

RE: [PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple space-seperated arguments

[Qi, Fuli]

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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2] ndctl, filter: refacor util_<obj>_filter() to support multiple space-seperated arguments

[Dan Williams]

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.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm