Hi Jeff,

The issue is more of repetition.   On an 8-socket system,  should a user really 
be expected to type 'ndctl create-namespace' eight times vs. running 'ndctl 
create-namespace --region=all' once?   SAP HANA is an example app the requires 
one namespace per region.  Scripting is a viable solution, but that requires 
somebody to write the script and do all the error checking & handling.  Each 
OEM/ISV/SysAdmin would have their own script.  Pushing the logic to ndctl seems 
to be a reasonable approach and let the user handle any errors returned by 
ndctl.

The ndctl-man-page implies the 'ndctl create-namespace --region=all' feature 
exists today:

       -r, --region=

           A regionX device name, or a region id number. The keyword all can be 
specified to carry out the operation on every region in the system, optionally 
filtered by bus id (see --bus=  option).

-Steve

-----Original Message-----
From: Jeff Moyer [mailto:jmo...@redhat.com] 
Sent: Wednesday, August 28, 2019 2:13 PM
To: Verma, Vishal L <vishal.l.ve...@intel.com>
Cc: linux-nvdimm@lists.01.org; Scargall, Steve <steve.scarg...@intel.com>; 
Williams, Dan J <dan.j.willi...@intel.com>
Subject: Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily

Vishal Verma <vishal.l.ve...@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.scarg...@intel.com>
> Cc: Jeff Moyer <jmo...@redhat.com>
> Cc: Dan Williams <dan.j.willi...@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.ve...@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

Reply via email to