diff mbox

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

Message ID 20180418063242.9221-1-qi.fuli@jp.fujitsu.com (mailing list archive)
State Accepted
Commit c70adc3
Headers show

Commit Message

QI Fuli April 18, 2018, 6:32 a.m. UTC 
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(-)

Comments

Dan Williams April 18, 2018, 2:31 p.m. UTC | #1 
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>
Verma, Vishal L April 18, 2018, 4:06 p.m. UTC | #2 
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
QI Fuli April 19, 2018, 2:10 a.m. UTC | #3 
> 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
QI Fuli April 25, 2018, 2:39 a.m. UTC | #4 
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
Dan Williams April 25, 2018, 7:32 a.m. UTC | #5 
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 mbox

Patch

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