* [PATCH v4 1/4] ndctl: convert namespace actions to use util_filter_params
2018-04-27 18:18 [PATCH v4 0/4] ndctl: convert actions to use util_filter_walk Dave Jiang
@ 2018-04-27 18:18 ` Dave Jiang
2018-04-27 18:18 ` [PATCH v4 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Dave Jiang @ 2018-04-27 18:18 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
In preparation of moving to using util_filter_walk, moving parts of
namespace params to util_filter_params.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
ndctl/namespace.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index fe86d826..e61f500c 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -40,14 +40,11 @@ static struct parameters {
bool mode_default;
bool align_default;
bool autolabel;
- const char *bus;
const char *map;
- const char *type;
const char *uuid;
const char *name;
const char *size;
const char *mode;
- const char *region;
const char *reconfig;
const char *sector_size;
const char *align;
@@ -55,6 +52,8 @@ static struct parameters {
.autolabel = true,
};
+struct util_filter_params uf_param;
+
void builtin_xaction_namespace_reset(void)
{
/*
@@ -87,9 +86,9 @@ struct parsed_parameters {
}})
#define BASE_OPTIONS() \
-OPT_STRING('b', "bus", ¶m.bus, "bus-id", \
+OPT_STRING('b', "bus", &uf_param.bus, "bus-id", \
"limit namespace to a bus with an id or provider of <bus-id>"), \
-OPT_STRING('r', "region", ¶m.region, "region-id", \
+OPT_STRING('r', "region", &uf_param.region, "region-id", \
"limit namespace to a region with an id or name of <region-id>"), \
OPT_BOOLEAN('v', "verbose", &verbose, "emit extra debug messages to stderr")
@@ -108,7 +107,7 @@ OPT_STRING('M', "map", ¶m.map, "memmap-location", \
"specify 'mem' or 'dev' for the location of the memmap"), \
OPT_STRING('l', "sector-size", ¶m.sector_size, "lba-size", \
"specify the logical sector size in bytes"), \
-OPT_STRING('t', "type", ¶m.type, "type", \
+OPT_STRING('t', "type", &uf_param.type, "type", \
"specify the type of namespace to create 'pmem' or 'blk'"), \
OPT_STRING('a', "align", ¶m.align, "align", \
"specify the namespace alignment in bytes (default: 2M)"), \
@@ -148,18 +147,18 @@ static int set_defaults(enum device_action mode)
{
int rc = 0;
- if (param.type) {
- if (strcmp(param.type, "pmem") == 0)
+ if (uf_param.type) {
+ if (strcmp(uf_param.type, "pmem") == 0)
/* pass */;
- else if (strcmp(param.type, "blk") == 0)
+ else if (strcmp(uf_param.type, "blk") == 0)
/* pass */;
else {
error("invalid type '%s', must be either 'pmem' or 'blk'\n",
- param.type);
+ uf_param.type);
rc = -EINVAL;
}
} else if (!param.reconfig && mode == ACTION_CREATE)
- param.type = "pmem";
+ uf_param.type = "pmem";
if (param.mode) {
if (strcmp(param.mode, "safe") == 0)
@@ -180,8 +179,8 @@ static int set_defaults(enum device_action mode)
error("invalid mode '%s'\n", param.mode);
rc = -EINVAL;
}
- } else if (!param.reconfig && param.type) {
- if (strcmp(param.type, "pmem") == 0)
+ } else if (!param.reconfig && uf_param.type) {
+ if (strcmp(uf_param.type, "pmem") == 0)
param.mode = "memory";
else
param.mode = "safe";
@@ -208,7 +207,7 @@ static int set_defaults(enum device_action mode)
param.map = "dev";
/* check for incompatible mode and type combinations */
- if (param.type && param.mode && strcmp(param.type, "blk") == 0
+ if (uf_param.type && param.mode && strcmp(uf_param.type, "blk") == 0
&& (strcmp(param.mode, "memory") == 0
|| strcmp(param.mode, "dax") == 0)) {
error("only 'pmem' namespaces support dax operation\n");
@@ -244,7 +243,7 @@ static int set_defaults(enum device_action mode)
error("invalid sector size: %s\n", param.sector_size);
rc = -EINVAL;
}
- } else if (((param.type && strcmp(param.type, "blk") == 0)
+ } else if (((uf_param.type && strcmp(uf_param.type, "blk") == 0)
|| (param.mode && strcmp(param.mode, "safe") == 0))) {
/* default sector size for blk-type or safe-mode */
param.sector_size = "4096";
@@ -1000,19 +999,19 @@ static int do_xaction_namespace(const char *namespace,
ndctl_set_log_priority(ctx, LOG_DEBUG);
ndctl_bus_foreach(ctx, bus) {
- if (!util_bus_filter(bus, param.bus))
+ if (!util_bus_filter(bus, uf_param.bus))
continue;
ndctl_region_foreach(bus, region) {
- if (!util_region_filter(region, param.region))
+ if (!util_region_filter(region, uf_param.region))
continue;
- if (param.type) {
- if (strcmp(param.type, "pmem") == 0
+ if (uf_param.type) {
+ if (strcmp(uf_param.type, "pmem") == 0
&& ndctl_region_get_type(region)
== ND_DEVICE_REGION_PMEM)
/* pass */;
- else if (strcmp(param.type, "blk") == 0
+ else if (strcmp(uf_param.type, "blk") == 0
&& ndctl_region_get_type(region)
== ND_DEVICE_REGION_BLK)
/* pass */;
@@ -1125,7 +1124,7 @@ int cmd_create_namespace(int argc, const char **argv, void *ctx)
* fallback to blk before giving up.
*/
memset(¶m, 0, sizeof(param));
- param.type = "blk";
+ uf_param.type = "blk";
set_defaults(ACTION_CREATE);
created = do_xaction_namespace(NULL, ACTION_CREATE, ctx);
}
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/4] ndctl: convert namespace actions to use util_filter_walk()
2018-04-27 18:18 [PATCH v4 0/4] ndctl: convert actions to use util_filter_walk Dave Jiang
2018-04-27 18:18 ` [PATCH v4 1/4] ndctl: convert namespace actions to use util_filter_params Dave Jiang
@ 2018-04-27 18:18 ` Dave Jiang
2018-04-27 19:25 ` Dan Williams
2018-04-27 18:18 ` [PATCH v4 3/4] ndctl: convert region " Dave Jiang
2018-04-27 18:18 ` [PATCH v4 4/4] ndctl: convert dimm " Dave Jiang
3 siblings, 1 reply; 7+ messages in thread
From: Dave Jiang @ 2018-04-27 18:18 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
util_filter_walk() does the looping through of busses and regions. Removing
duplicate code in namespace ops and provide filter functions so we can
utilize util_filter_walk() and share common code.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
ndctl/namespace.c | 164 ++++++++++++++++++++++++++++++-----------------------
test/btt-check.sh | 2 -
util/filter.c | 5 +-
util/filter.h | 8 +++
4 files changed, 105 insertions(+), 74 deletions(-)
diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index e61f500c..404d6761 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -983,14 +983,92 @@ static int namespace_reconfig(struct ndctl_region *region,
int namespace_check(struct ndctl_namespace *ndns, bool verbose, bool force,
bool repair, bool logfix);
+static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
+{
+ return true;
+}
+
+static bool filter_region(struct ndctl_region *region,
+ struct util_filter_ctx *ctx)
+{
+ struct nsaction_filter_arg *nfa = ctx->nsaction;
+ int rc = 0;
+ bool out = true;
+
+ if (nfa->action == ACTION_CREATE && !nfa->namespace) {
+ if (nfa->rc == 1)
+ return false;
+
+ rc = namespace_create(region);
+ if (rc != -EAGAIN) {
+ if (rc == 0)
+ rc = 1;
+ /* don't proceed in the filter loop */
+ out = false;
+ }
+ nfa->rc = rc;
+ }
+
+ return out;
+}
+
+static void filter_namespace(struct ndctl_namespace *ndns,
+ struct util_filter_ctx *ctx)
+{
+ struct ndctl_region *region = ndctl_namespace_get_region(ndns);
+ struct nsaction_filter_arg *nfa = ctx->nsaction;
+ const char *ndns_name;
+ int rc;
+
+ /* we have an error, don't do anything else */
+ if (nfa->rc < 0)
+ return;
+
+ ndns_name = ndctl_namespace_get_devname(ndns);
+ if (strcmp(nfa->namespace, "all") != 0
+ && strcmp(nfa->namespace, ndns_name) != 0) {
+ return;
+ }
+
+ switch (nfa->action) {
+ case ACTION_DISABLE:
+ rc = ndctl_namespace_disable_safe(ndns);
+ break;
+ case ACTION_ENABLE:
+ rc = ndctl_namespace_enable(ndns);
+ break;
+ case ACTION_DESTROY:
+ rc = namespace_destroy(region, ndns);
+ break;
+ case ACTION_CHECK:
+ rc = namespace_check(ndns, verbose, force, repair, logfix);
+ if (rc < 0) {
+ nfa->rc = rc;
+ return;
+ }
+ break;
+ case ACTION_CREATE:
+ rc = namespace_reconfig(region, ndns);
+ if (rc < 0) {
+ nfa->rc = rc;
+ return;
+ }
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ if (rc >= 0)
+ nfa->rc++;
+}
+
static int do_xaction_namespace(const char *namespace,
enum device_action action, struct ndctl_ctx *ctx)
{
- struct ndctl_namespace *ndns, *_n;
- int rc = -ENXIO, success = 0;
- struct ndctl_region *region;
- const char *ndns_name;
- struct ndctl_bus *bus;
+ int rc = -ENXIO;
+ struct util_filter_ctx fctx = { 0 };
+ struct nsaction_filter_arg nfa = { 0 };
if (!namespace && action != ACTION_CREATE)
return rc;
@@ -998,74 +1076,18 @@ static int do_xaction_namespace(const char *namespace,
if (verbose)
ndctl_set_log_priority(ctx, LOG_DEBUG);
- ndctl_bus_foreach(ctx, bus) {
- if (!util_bus_filter(bus, uf_param.bus))
- continue;
-
- ndctl_region_foreach(bus, region) {
- if (!util_region_filter(region, uf_param.region))
- continue;
-
- if (uf_param.type) {
- if (strcmp(uf_param.type, "pmem") == 0
- && ndctl_region_get_type(region)
- == ND_DEVICE_REGION_PMEM)
- /* pass */;
- else if (strcmp(uf_param.type, "blk") == 0
- && ndctl_region_get_type(region)
- == ND_DEVICE_REGION_BLK)
- /* pass */;
- else
- continue;
- }
+ fctx.filter_bus = filter_bus;
+ fctx.filter_region = filter_region;
+ fctx.filter_namespace = filter_namespace;
+ fctx.nsaction = &nfa;
+ fctx.nsaction->action = action;
+ fctx.nsaction->namespace = namespace;
- if (action == ACTION_CREATE && !namespace) {
- rc = namespace_create(region);
- if (rc == -EAGAIN)
- continue;
- if (rc == 0)
- rc = 1;
- return rc;
- }
- ndctl_namespace_foreach_safe(region, ndns, _n) {
- ndns_name = ndctl_namespace_get_devname(ndns);
-
- if (strcmp(namespace, "all") != 0
- && strcmp(namespace, ndns_name) != 0)
- continue;
- switch (action) {
- case ACTION_DISABLE:
- rc = ndctl_namespace_disable_safe(ndns);
- break;
- case ACTION_ENABLE:
- rc = ndctl_namespace_enable(ndns);
- break;
- case ACTION_DESTROY:
- rc = namespace_destroy(region, ndns);
- break;
- case ACTION_CHECK:
- rc = namespace_check(ndns, verbose,
- force, repair, logfix);
- if (rc < 0)
- return rc;
- break;
- case ACTION_CREATE:
- rc = namespace_reconfig(region, ndns);
- if (rc < 0)
- return rc;
- return 1;
- default:
- rc = -EINVAL;
- break;
- }
- if (rc >= 0)
- success++;
- }
- }
- }
+ rc = util_filter_walk(ctx, &fctx, &uf_param);
+ if (rc < 0)
+ return rc;
- if (success)
- return success;
+ rc = nfa.rc;
return rc;
}
diff --git a/test/btt-check.sh b/test/btt-check.sh
index 353d4375..50c1b592 100755
--- a/test/btt-check.sh
+++ b/test/btt-check.sh
@@ -1,4 +1,4 @@
-#!/bin/bash -E
+#!/bin/bash -Ex
# Copyright(c) 2015-2017 Intel Corporation. All rights reserved.
#
diff --git a/util/filter.c b/util/filter.c
index 0d3cc02f..b7a4fbd6 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -18,6 +18,7 @@
#include <sys/stat.h>
#include <util/util.h>
#include <sys/types.h>
+#include <ndctl/action.h>
#include <ndctl/ndctl.h>
#include <util/filter.h>
#include <ndctl/libndctl.h>
@@ -418,7 +419,7 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
}
ndctl_region_foreach(bus, region) {
- struct ndctl_namespace *ndns;
+ struct ndctl_namespace *ndns, *_n;
if (!util_region_filter(region, param->region)
|| !util_region_filter_by_dimm(region,
@@ -437,7 +438,7 @@ int util_filter_walk(struct ndctl_ctx *ctx, struct util_filter_ctx *fctx,
if (!fctx->filter_region(region, fctx))
continue;
- ndctl_namespace_foreach(region, ndns) {
+ ndctl_namespace_foreach_safe(region, ndns, _n) {
enum ndctl_namespace_mode mode;
if (!fctx->filter_namespace)
diff --git a/util/filter.h b/util/filter.h
index effda24b..9f3786a9 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -13,6 +13,7 @@
#ifndef _UTIL_FILTER_H_
#define _UTIL_FILTER_H_
#include <stdbool.h>
+#include <ndctl/action.h>
struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char *ident);
struct ndctl_region *util_region_filter(struct ndctl_region *region,
@@ -50,6 +51,12 @@ struct list_filter_arg {
unsigned long flags;
};
+struct nsaction_filter_arg {
+ enum device_action action;
+ const char *namespace;
+ int rc;
+};
+
/*
* struct util_filter_ctx - control and callbacks for util_filter_walk()
* ->filter_bus() and ->filter_region() return bool because the
@@ -67,6 +74,7 @@ struct util_filter_ctx {
union {
void *arg;
struct list_filter_arg *list;
+ struct nsaction_filter_arg *nsaction;
};
};
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/4] ndctl: convert namespace actions to use util_filter_walk()
2018-04-27 18:18 ` [PATCH v4 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
@ 2018-04-27 19:25 ` Dan Williams
2018-04-27 19:50 ` Dave Jiang
0 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2018-04-27 19:25 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Fri, Apr 27, 2018 at 11:18 AM, Dave Jiang <dave.jiang@intel.com> wrote:
> util_filter_walk() does the looping through of busses and regions. Removing
> duplicate code in namespace ops and provide filter functions so we can
> utilize util_filter_walk() and share common code.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> ndctl/namespace.c | 164 ++++++++++++++++++++++++++++++-----------------------
> test/btt-check.sh | 2 -
> util/filter.c | 5 +-
> util/filter.h | 8 +++
> 4 files changed, 105 insertions(+), 74 deletions(-)
>
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index e61f500c..404d6761 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -983,14 +983,92 @@ static int namespace_reconfig(struct ndctl_region *region,
> int namespace_check(struct ndctl_namespace *ndns, bool verbose, bool force,
> bool repair, bool logfix);
>
> +static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
> +{
> + return true;
> +}
> +
> +static bool filter_region(struct ndctl_region *region,
> + struct util_filter_ctx *ctx)
> +{
> + struct nsaction_filter_arg *nfa = ctx->nsaction;
> + int rc = 0;
> + bool out = true;
> +
> + if (nfa->action == ACTION_CREATE && !nfa->namespace) {
> + if (nfa->rc == 1)
> + return false;
> +
> + rc = namespace_create(region);
> + if (rc != -EAGAIN) {
> + if (rc == 0)
> + rc = 1;
> + /* don't proceed in the filter loop */
> + out = false;
> + }
> + nfa->rc = rc;
> + }
> +
> + return out;
> +}
> +
> +static void filter_namespace(struct ndctl_namespace *ndns,
> + struct util_filter_ctx *ctx)
> +{
> + struct ndctl_region *region = ndctl_namespace_get_region(ndns);
> + struct nsaction_filter_arg *nfa = ctx->nsaction;
> + const char *ndns_name;
> + int rc;
> +
> + /* we have an error, don't do anything else */
> + if (nfa->rc < 0)
> + return;
Why do we need to stop here? This seems like a behavior regression.
For example:
ndctl disable-namespace all
Should continue to disable subsequent namespaces even if a previous
namespace attempt fails.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/4] ndctl: convert namespace actions to use util_filter_walk()
2018-04-27 19:25 ` Dan Williams
@ 2018-04-27 19:50 ` Dave Jiang
0 siblings, 0 replies; 7+ messages in thread
From: Dave Jiang @ 2018-04-27 19:50 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm
On 04/27/2018 12:25 PM, Dan Williams wrote:
> On Fri, Apr 27, 2018 at 11:18 AM, Dave Jiang <dave.jiang@intel.com> wrote:
>> util_filter_walk() does the looping through of busses and regions. Removing
>> duplicate code in namespace ops and provide filter functions so we can
>> utilize util_filter_walk() and share common code.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> ndctl/namespace.c | 164 ++++++++++++++++++++++++++++++-----------------------
>> test/btt-check.sh | 2 -
>> util/filter.c | 5 +-
>> util/filter.h | 8 +++
>> 4 files changed, 105 insertions(+), 74 deletions(-)
>>
>> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
>> index e61f500c..404d6761 100644
>> --- a/ndctl/namespace.c
>> +++ b/ndctl/namespace.c
>> @@ -983,14 +983,92 @@ static int namespace_reconfig(struct ndctl_region *region,
>> int namespace_check(struct ndctl_namespace *ndns, bool verbose, bool force,
>> bool repair, bool logfix);
>>
>> +static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
>> +{
>> + return true;
>> +}
>> +
>> +static bool filter_region(struct ndctl_region *region,
>> + struct util_filter_ctx *ctx)
>> +{
>> + struct nsaction_filter_arg *nfa = ctx->nsaction;
>> + int rc = 0;
>> + bool out = true;
>> +
>> + if (nfa->action == ACTION_CREATE && !nfa->namespace) {
>> + if (nfa->rc == 1)
>> + return false;
>> +
>> + rc = namespace_create(region);
>> + if (rc != -EAGAIN) {
>> + if (rc == 0)
>> + rc = 1;
>> + /* don't proceed in the filter loop */
>> + out = false;
>> + }
>> + nfa->rc = rc;
>> + }
>> +
>> + return out;
>> +}
>> +
>> +static void filter_namespace(struct ndctl_namespace *ndns,
>> + struct util_filter_ctx *ctx)
>> +{
>> + struct ndctl_region *region = ndctl_namespace_get_region(ndns);
>> + struct nsaction_filter_arg *nfa = ctx->nsaction;
>> + const char *ndns_name;
>> + int rc;
>> +
>> + /* we have an error, don't do anything else */
>> + if (nfa->rc < 0)
>> + return;
>
> Why do we need to stop here? This seems like a behavior regression.
>
> For example:
>
> ndctl disable-namespace all
>
> Should continue to disable subsequent namespaces even if a previous
> namespace attempt fails.
>
Yes I misread the original flow. I need to replace that with:
if (!nfa->namespace)
return;
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 3/4] ndctl: convert region actions to use util_filter_walk()
2018-04-27 18:18 [PATCH v4 0/4] ndctl: convert actions to use util_filter_walk Dave Jiang
2018-04-27 18:18 ` [PATCH v4 1/4] ndctl: convert namespace actions to use util_filter_params Dave Jiang
2018-04-27 18:18 ` [PATCH v4 2/4] ndctl: convert namespace actions to use util_filter_walk() Dave Jiang
@ 2018-04-27 18:18 ` Dave Jiang
2018-04-27 18:18 ` [PATCH v4 4/4] ndctl: convert dimm " Dave Jiang
3 siblings, 0 replies; 7+ messages in thread
From: Dave Jiang @ 2018-04-27 18:18 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
util_filter_walk() does the looping through of busses and regions. Removing
duplicate code in region ops and provide filter functions so we can
utilize util_filter_walk() and share common code.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
ndctl/region.c | 59 ++++++++++++++++++++++++++++++++++----------------------
util/filter.h | 6 ++++++
2 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/ndctl/region.c b/ndctl/region.c
index 9fc90808..9fd07af6 100644
--- a/ndctl/region.c
+++ b/ndctl/region.c
@@ -19,10 +19,7 @@
#include <util/parse-options.h>
#include <ndctl/libndctl.h>
-static struct {
- const char *bus;
- const char *type;
-} param;
+struct util_filter_params param;
static const struct option region_options[] = {
OPT_STRING('b', "bus", ¶m.bus, "bus-id",
@@ -92,33 +89,49 @@ static int region_action(struct ndctl_region *region, enum device_action mode)
return 0;
}
+static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
+{
+ return true;
+}
+
+static bool filter_region(struct ndctl_region *region,
+ struct util_filter_ctx *ctx)
+{
+ struct rgaction_filter_arg *rfa = ctx->rgaction;
+ int rc;
+
+ if (rfa->rc < 0)
+ return false;
+
+ rc = region_action(region, rfa->action);
+
+ if (rc == 0)
+ rfa->rc++;
+ else
+ rfa->rc = 0;
+
+ /* we don't need to fall through, can continue the loop */
+ return false;
+}
+
static int do_xable_region(const char *region_arg, enum device_action mode,
struct ndctl_ctx *ctx)
{
- int rc = -ENXIO, success = 0;
- struct ndctl_region *region;
- struct ndctl_bus *bus;
+ int rc = -ENXIO;
+ struct util_filter_ctx fctx = { 0 };
+ struct rgaction_filter_arg rfa = { 0 };
if (!region_arg)
goto out;
- ndctl_bus_foreach(ctx, bus) {
- if (!util_bus_filter(bus, param.bus))
- continue;
-
- ndctl_region_foreach(bus, region) {
- const char *type = ndctl_region_get_type_name(region);
-
- if (param.type && strcmp(param.type, type) != 0)
- continue;
- if (!util_region_filter(region, region_arg))
- continue;
- if (region_action(region, mode) == 0)
- success++;
- }
- }
+ fctx.filter_bus = filter_bus;
+ fctx.filter_region = filter_region;
+ fctx.rgaction = &rfa;
+ fctx.rgaction->action = mode;
+ rc = util_filter_walk(ctx, &fctx, ¶m);
+ if (rc == 0)
+ rc = fctx.rgaction->rc;
- rc = success;
out:
param.bus = NULL;
return rc;
diff --git a/util/filter.h b/util/filter.h
index 9f3786a9..0e0e3bec 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -57,6 +57,11 @@ struct nsaction_filter_arg {
int rc;
};
+struct rgaction_filter_arg {
+ enum device_action action;
+ int rc;
+};
+
/*
* struct util_filter_ctx - control and callbacks for util_filter_walk()
* ->filter_bus() and ->filter_region() return bool because the
@@ -75,6 +80,7 @@ struct util_filter_ctx {
void *arg;
struct list_filter_arg *list;
struct nsaction_filter_arg *nsaction;
+ struct rgaction_filter_arg *rgaction;
};
};
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 4/4] ndctl: convert dimm actions to use util_filter_walk()
2018-04-27 18:18 [PATCH v4 0/4] ndctl: convert actions to use util_filter_walk Dave Jiang
` (2 preceding siblings ...)
2018-04-27 18:18 ` [PATCH v4 3/4] ndctl: convert region " Dave Jiang
@ 2018-04-27 18:18 ` Dave Jiang
3 siblings, 0 replies; 7+ messages in thread
From: Dave Jiang @ 2018-04-27 18:18 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
util_filter_walk() does the looping through of busses and dimms. Removing
duplicate code for dimm actions and provide filter functions so we can
utilize util_filter_walk() and share common code.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
ndctl/dimm.c | 83 ++++++++++++++++++++++++++++++++++-----------------------
util/filter.h | 9 ++++++
2 files changed, 59 insertions(+), 33 deletions(-)
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 1c168997..d652be69 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -822,7 +822,6 @@ static int action_update(struct ndctl_dimm *dimm, struct action_context *actx)
}
static struct parameters {
- const char *bus;
const char *outfile;
const char *infile;
const char *labelversion;
@@ -833,6 +832,8 @@ static struct parameters {
.labelversion = "1.1",
};
+struct util_filter_params uf_param;
+
static int __action_init(struct ndctl_dimm *dimm,
enum ndctl_namespace_version version, int chk_only)
{
@@ -899,7 +900,7 @@ static int action_check(struct ndctl_dimm *dimm, struct action_context *actx)
#define BASE_OPTIONS() \
-OPT_STRING('b', "bus", ¶m.bus, "bus-id", \
+OPT_STRING('b', "bus", &uf_param.bus, "bus-id", \
"<nmem> must be on a bus with an id/provider of <bus-id>"), \
OPT_BOOLEAN('v',"verbose", ¶m.verbose, "turn on debug")
@@ -951,18 +952,48 @@ static const struct option init_options[] = {
OPT_END(),
};
+static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
+{
+ return true;
+}
+
+static void filter_dimm(struct ndctl_dimm *dimm, struct util_filter_ctx *ctx)
+{
+ struct dmaction_filter_arg *dfa = ctx->dmaction;
+ int rc;
+
+ if (dfa->action == action_write) {
+ dfa->dimm = dimm;
+ rc = 0;
+ } else
+ rc = dfa->action(dimm, dfa->actx);
+
+ if (rc == 0)
+ dfa->count++;
+ else if (rc && !dfa->rc)
+ dfa->rc = rc;
+}
+
+/* dummy filter function to skip the loop */
+static bool filter_region(struct ndctl_region *region,
+ struct util_filter_ctx *ctx)
+{
+ return false;
+}
+
static int dimm_action(int argc, const char **argv, void *ctx,
int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
const struct option *options, const char *usage)
{
struct action_context actx = { 0 };
- int i, rc = 0, count = 0, err = 0;
- struct ndctl_dimm *single = NULL;
+ int i, rc = 0, err = 0;
const char * const u[] = {
usage,
NULL
};
unsigned long id;
+ struct util_filter_ctx fctx = { 0 };
+ struct dmaction_filter_arg dfa = { 0 };
argc = parse_options(argc, argv, options, u, 0);
@@ -1039,45 +1070,31 @@ static int dimm_action(int argc, const char **argv, void *ctx,
goto out;
}
+ fctx.filter_bus = filter_bus;
+ fctx.filter_dimm = filter_dimm;
+ fctx.filter_region = filter_region;
+ fctx.dmaction = &dfa;
+ dfa.action = action;
+ dfa.actx = &actx;
+
rc = 0;
- err = 0;
- count = 0;
for (i = 0; i < argc; i++) {
- struct ndctl_dimm *dimm;
- struct ndctl_bus *bus;
-
if (sscanf(argv[i], "nmem%lu", &id) != 1
&& strcmp(argv[i], "all") != 0)
continue;
- ndctl_bus_foreach(ctx, bus) {
- if (!util_bus_filter(bus, param.bus))
- continue;
- ndctl_dimm_foreach(bus, dimm) {
- if (!util_dimm_filter(dimm, argv[i]))
- continue;
- if (action == action_write) {
- single = dimm;
- rc = 0;
- } else
- rc = action(dimm, &actx);
-
- if (rc == 0)
- count++;
- else if (rc && !err)
- err = rc;
- }
- }
+ uf_param.dimm = argv[i];
+ rc = util_filter_walk(ctx, &fctx, &uf_param);
}
- rc = err;
+ rc = dfa.rc;
if (action == action_write) {
- if (count > 1) {
+ if (dfa.count > 1) {
error("write-labels only supports writing a single dimm\n");
usage_with_options(u, options);
return -EINVAL;
- } else if (single)
- rc = action(single, &actx);
+ } else if (dfa.dimm)
+ rc = action(dfa.dimm, &actx);
}
if (actx.jdimms)
@@ -1095,8 +1112,8 @@ static int dimm_action(int argc, const char **argv, void *ctx,
* count if some actions succeeded, 0 if none were attempted,
* negative error code otherwise.
*/
- if (count > 0)
- return count;
+ if (dfa.count > 0)
+ return dfa.count;
return rc;
}
diff --git a/util/filter.h b/util/filter.h
index 0e0e3bec..2bef9310 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -62,6 +62,14 @@ struct rgaction_filter_arg {
int rc;
};
+struct dmaction_filter_arg {
+ struct action_context *actx;
+ struct ndctl_dimm *dimm;
+ int (*action)(struct ndctl_dimm *dimm, struct action_context *actx);
+ int count;
+ int rc;
+};
+
/*
* struct util_filter_ctx - control and callbacks for util_filter_walk()
* ->filter_bus() and ->filter_region() return bool because the
@@ -81,6 +89,7 @@ struct util_filter_ctx {
struct list_filter_arg *list;
struct nsaction_filter_arg *nsaction;
struct rgaction_filter_arg *rgaction;
+ struct dmaction_filter_arg *dmaction;
};
};
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 7+ messages in thread