From: Jeff Moyer <jmoyer@redhat.com>
To: Vishal Verma <vishal.l.verma@intel.com>
Cc: Steve Scargal <steve.scargall@intel.com>, linux-nvdimm@lists.01.org
Subject: Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
Date: Wed, 28 Aug 2019 16:13:27 -0400 [thread overview]
Message-ID: <x49y2zd6obc.fsf@segfault.boston.devel.redhat.com> (raw)
In-Reply-To: <20190828200204.21750-1-vishal.l.verma@intel.com> (Vishal Verma's message of "Wed, 28 Aug 2019 14:02:04 -0600")
Vishal Verma <vishal.l.verma@intel.com> writes:
> 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.
Cumbersome? Like, in the same way partitioning a disk is cumbersome? I
don't understand what the problem is, I guess. If you want N namespaces
of the same type/size/align, then script it. Why does there have to be
a command for that? I definitely think that changing the behavior of
create-namespace is a non-starter.
Cheers,
Jeff
>
> 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));
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
next prev parent reply other threads:[~2019-08-28 20:15 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-28 20:02 [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily Vishal Verma
2019-08-28 20:13 ` Jeff Moyer [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=x49y2zd6obc.fsf@segfault.boston.devel.redhat.com \
--to=jmoyer@redhat.com \
--cc=linux-nvdimm@lists.01.org \
--cc=steve.scargall@intel.com \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).