linux-nvdimm.lists.01.org archive mirror
 help / color / mirror / Atom feed
* [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
@ 2019-08-28 20:02 Vishal Verma
  2019-08-28 20:13 ` Jeff Moyer
  0 siblings, 1 reply; 8+ messages in thread
From: Vishal Verma @ 2019-08-28 20:02 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Steve Scargal

When a --region=all option is supplied to ndctl-create-namespace, it
happily ignores it, since create-namespace has historically only created
one namespace per invocation.

This can be cumbersome, so change create-namespace to create namespaces
greedily. When --region=all is specified, or if a --region option is
absent (same as 'all' from a filtering standpoint), create namespaces on
all regions.

Note that this does has two important implications:

1. The user-facing behavior of create-namespace changes in a potentially
surprising way. It may be undesirable for an unadorned 'ndctl
create-namespace' command to suddenly start creating lots of namespaces.

2. Error handling becomes a bit inconsistent. As with all commands
accepting an 'all' option, error reporting becomes a bit tenuous. With
this change, we will continue to create namespaces so long as we don't
hit an error, but if we do, we bail and exit. Because of the special
ENOSPC detection and handling around this, it can be easy to construct a
scenario where en existing namespace on the last region in the scan list
happens to report an error exit, but if the existing namespace was in a
region at the start of the scan list, it gets passed over as a "just try
the next region"

RFC comment: Maybe the solution is to keep the create-namespace
semantics unchanged, and introduce a new command - 'create-namespaces'
or 'create-names-ace-greedy' with the new behavior. I'm not sure if
users will care deeply about either of the two points above, hence
sending this as an RFC.

Link: https://github.com/pmem/ndctl/issues/106
Reported-by: Steve Scargal <steve.scargall@intel.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 ndctl/namespace.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index af20a42..856ad82 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -1365,9 +1365,12 @@ static int do_xaction_namespace(const char *namespace,
 				rc = namespace_create(region);
 				if (rc == -EAGAIN)
 					continue;
-				if (rc == 0)
-					*processed = 1;
-				return rc;
+				if (rc == 0) {
+					(*processed)++;
+					continue;
+				} else {
+					return rc;
+				}
 			}
 			ndctl_namespace_foreach_safe(region, ndns, _n) {
 				ndns_name = ndctl_namespace_get_devname(ndns);
@@ -1487,6 +1490,8 @@ int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 		rc = do_xaction_namespace(NULL, ACTION_CREATE, ctx, &created);
 	}
 
+	fprintf(stderr, "created %d namespace%s\n", created,
+			created == 1 ? "" : "s");
 	if (rc < 0 || (!namespace && created < 1)) {
 		fprintf(stderr, "failed to %s namespace: %s\n", namespace
 				? "reconfigure" : "create", strerror(-rc));
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-08-28 23:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-28 20:02 [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily Vishal Verma
2019-08-28 20:13 ` Jeff Moyer
2019-08-28 20:47   ` Scargall, Steve
2019-08-28 21:16     ` Verma, Vishal L
2019-08-28 21:22       ` Verma, Vishal L
2019-08-28 21:32         ` Dan Williams
2019-08-28 22:38           ` Scargall, Steve
2019-08-28 23:17             ` Verma, Vishal L

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).