Linux-NVDIMM Archive on lore.kernel.org
 help / color / 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	[flat|nested] 8+ messages in thread

* Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
  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
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Moyer @ 2019-08-28 20:13 UTC (permalink / raw)
  To: Vishal Verma; +Cc: Steve Scargal, linux-nvdimm

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

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

* RE: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
  2019-08-28 20:13 ` Jeff Moyer
@ 2019-08-28 20:47   ` Scargall, Steve
  2019-08-28 21:16     ` Verma, Vishal L
  0 siblings, 1 reply; 8+ messages in thread
From: Scargall, Steve @ 2019-08-28 20:47 UTC (permalink / raw)
  To: Jeff Moyer, Verma, Vishal L; +Cc: linux-nvdimm

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:jmoyer@redhat.com] 
Sent: Wednesday, August 28, 2019 2:13 PM
To: Verma, Vishal L <vishal.l.verma@intel.com>
Cc: linux-nvdimm@lists.01.org; Scargall, Steve <steve.scargall@intel.com>; Williams, Dan J <dan.j.williams@intel.com>
Subject: Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily

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

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

* Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
  2019-08-28 20:47   ` Scargall, Steve
@ 2019-08-28 21:16     ` Verma, Vishal L
  2019-08-28 21:22       ` Verma, Vishal L
  0 siblings, 1 reply; 8+ messages in thread
From: Verma, Vishal L @ 2019-08-28 21:16 UTC (permalink / raw)
  To: Scargall, Steve, jmoyer; +Cc: linux-nvdimm

On Wed, 2019-08-28 at 13:47 -0700, Scargall, Steve wrote:
> 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.

A scripted solution can indeed be really simple - e.g.:

# while read -r region; do  ndctl create-namespace --region="$region";
done < <(ndctl list --bus=nfit_test.0 -R  | jq -r '.[].dev')

{
  "dev":"namespace5.0",
  "mode":"fsdax",
  "map":"dev",
  "size":"62.00 MiB (65.01 MB)",
  "uuid":"c8014457-c268-4f22-8eae-6386fbf08ceb",
  "sector_size":512,
  "align":2097152,
  "blockdev":"pmem5"
}
{
  "dev":"namespace4.0",
  "mode":"fsdax",
  "map":"dev",
  "size":"30.00 MiB (31.46 MB)",
  "uuid":"f9498ef6-cdd6-46c7-95f1-86469546ecb9",
  "sector_size":512,
  "align":2097152,
  "blockdev":"pmem4"
}

> 
> 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).
> 

This is true, but unfortunately, the implementation has treated create-
namespace as an exception to this since the start, and I agree with Jeff
that changing its behavior now can cause other Hyrum's law-esqe [1]
breakage.

I think however it should be easy to make a compromise, and retain the
legacy behavior of create-namespace, while creating a new create-
namespace-greedy command with the new semantics.

Thoughts on that?

Thanks,
	-Vishal

[1]: http://www.hyrumslaw.com/
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
  2019-08-28 21:16     ` Verma, Vishal L
@ 2019-08-28 21:22       ` Verma, Vishal L
  2019-08-28 21:32         ` Dan Williams
  0 siblings, 1 reply; 8+ messages in thread
From: Verma, Vishal L @ 2019-08-28 21:22 UTC (permalink / raw)
  To: Scargall, Steve, jmoyer; +Cc: linux-nvdimm

On Wed, 2019-08-28 at 21:16 +0000, Verma, Vishal L wrote:
> On Wed, 2019-08-28 at 13:47 -0700, Scargall, Steve wrote:
> > 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.
> 
> A scripted solution can indeed be really simple - e.g.:
> 
> # while read -r region; do  ndctl create-namespace --region="$region";
> done < <(ndctl list --bus=nfit_test.0 -R  | jq -r '.[].dev')
> 
> {
>   "dev":"namespace5.0",
>   "mode":"fsdax",
>   "map":"dev",
>   "size":"62.00 MiB (65.01 MB)",
>   "uuid":"c8014457-c268-4f22-8eae-6386fbf08ceb",
>   "sector_size":512,
>   "align":2097152,
>   "blockdev":"pmem5"
> }
> {
>   "dev":"namespace4.0",
>   "mode":"fsdax",
>   "map":"dev",
>   "size":"30.00 MiB (31.46 MB)",
>   "uuid":"f9498ef6-cdd6-46c7-95f1-86469546ecb9",
>   "sector_size":512,
>   "align":2097152,
>   "blockdev":"pmem4"
> }
> 
> > 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).
> > 
> 
> This is true, but unfortunately, the implementation has treated create-
> namespace as an exception to this since the start, and I agree with Jeff
> that changing its behavior now can cause other Hyrum's law-esqe [1]
> breakage.
> 
> I think however it should be easy to make a compromise, and retain the
> legacy behavior of create-namespace, while creating a new create-
> namespace-greedy command with the new semantics.
> 
.. And it doesn't even need to be a new command, a simple --greedy
option to create-namespace should be sufficient.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
  2019-08-28 21:22       ` Verma, Vishal L
@ 2019-08-28 21:32         ` Dan Williams
  2019-08-28 22:38           ` Scargall, Steve
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Williams @ 2019-08-28 21:32 UTC (permalink / raw)
  To: Verma, Vishal L; +Cc: Scargall, Steve, linux-nvdimm

On Wed, Aug 28, 2019 at 2:22 PM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Wed, 2019-08-28 at 21:16 +0000, Verma, Vishal L wrote:
> > On Wed, 2019-08-28 at 13:47 -0700, Scargall, Steve wrote:
> > > 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.
> >
> > A scripted solution can indeed be really simple - e.g.:
> >
> > # while read -r region; do  ndctl create-namespace --region="$region";
> > done < <(ndctl list --bus=nfit_test.0 -R  | jq -r '.[].dev')
> >
> > {
> >   "dev":"namespace5.0",
> >   "mode":"fsdax",
> >   "map":"dev",
> >   "size":"62.00 MiB (65.01 MB)",
> >   "uuid":"c8014457-c268-4f22-8eae-6386fbf08ceb",
> >   "sector_size":512,
> >   "align":2097152,
> >   "blockdev":"pmem5"
> > }
> > {
> >   "dev":"namespace4.0",
> >   "mode":"fsdax",
> >   "map":"dev",
> >   "size":"30.00 MiB (31.46 MB)",
> >   "uuid":"f9498ef6-cdd6-46c7-95f1-86469546ecb9",
> >   "sector_size":512,
> >   "align":2097152,
> >   "blockdev":"pmem4"
> > }
> >
> > > 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).
> > >
> >
> > This is true, but unfortunately, the implementation has treated create-
> > namespace as an exception to this since the start, and I agree with Jeff
> > that changing its behavior now can cause other Hyrum's law-esqe [1]
> > breakage.
> >
> > I think however it should be easy to make a compromise, and retain the
> > legacy behavior of create-namespace, while creating a new create-
> > namespace-greedy command with the new semantics.
> >
> .. And it doesn't even need to be a new command, a simple --greedy
> option to create-namespace should be sufficient.

Perhaps "--continue" to proceed with creating namespace after the
first successful invocation. I agree with Jeff that changing the
default semantics would be surprising. The man page can also be fixed
up to make it clear that it's the "singleton region capacity search",
not "all possible namespaces" that "-r all" implies.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* RE: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
  2019-08-28 21:32         ` Dan Williams
@ 2019-08-28 22:38           ` Scargall, Steve
  2019-08-28 23:17             ` Verma, Vishal L
  0 siblings, 1 reply; 8+ messages in thread
From: Scargall, Steve @ 2019-08-28 22:38 UTC (permalink / raw)
  To: Williams, Dan J, Verma, Vishal L; +Cc: linux-nvdimm

Thanks for the clarification.  I have a much better understanding now. 

Updating the ndctl-create-namespace man page to clarify what '--region=all' does and does not do in combination with other arguments and options would be beneficial.   This would actually provide a more immediate solution to the problem.  It sounds like the general statement used in the other man pages can remain?  Or are there other exceptions?

With the proposed implementation that bails out on first error, `--continue` seems reasonable vs. `--greedy`.  What do you think about `--all-regions` instead?  It is more meaningful towards its intended action or use-case, but would it cause confusion with `--regions=all`?   Documentation could provide the solution here.  We would have to choose an arbitrary short option since `-a` and `-r` are already taken.  

Whatever the final decision, we should test for and return a usage error for a mutually exclusive set of inputs, i.e. `ndctl create-namespace --all-regions --region=region0` doesn't make sense - either you want to perform the create operation on all regions or only in region0.

- Steve

-----Original Message-----
From: Dan Williams [mailto:dan.j.williams@intel.com] 
Sent: Wednesday, August 28, 2019 3:32 PM
To: Verma, Vishal L <vishal.l.verma@intel.com>
Cc: Scargall, Steve <steve.scargall@intel.com>; jmoyer@redhat.com; linux-nvdimm@lists.01.org
Subject: Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily

On Wed, Aug 28, 2019 at 2:22 PM Verma, Vishal L <vishal.l.verma@intel.com> wrote:
>
> On Wed, 2019-08-28 at 21:16 +0000, Verma, Vishal L wrote:
> > On Wed, 2019-08-28 at 13:47 -0700, Scargall, Steve wrote:
> > > 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.
> >
> > A scripted solution can indeed be really simple - e.g.:
> >
> > # while read -r region; do  ndctl create-namespace 
> > --region="$region"; done < <(ndctl list --bus=nfit_test.0 -R  | jq 
> > -r '.[].dev')
> >
> > {
> >   "dev":"namespace5.0",
> >   "mode":"fsdax",
> >   "map":"dev",
> >   "size":"62.00 MiB (65.01 MB)",
> >   "uuid":"c8014457-c268-4f22-8eae-6386fbf08ceb",
> >   "sector_size":512,
> >   "align":2097152,
> >   "blockdev":"pmem5"
> > }
> > {
> >   "dev":"namespace4.0",
> >   "mode":"fsdax",
> >   "map":"dev",
> >   "size":"30.00 MiB (31.46 MB)",
> >   "uuid":"f9498ef6-cdd6-46c7-95f1-86469546ecb9",
> >   "sector_size":512,
> >   "align":2097152,
> >   "blockdev":"pmem4"
> > }
> >
> > > 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).
> > >
> >
> > This is true, but unfortunately, the implementation has treated 
> > create- namespace as an exception to this since the start, and I 
> > agree with Jeff that changing its behavior now can cause other 
> > Hyrum's law-esqe [1] breakage.
> >
> > I think however it should be easy to make a compromise, and retain 
> > the legacy behavior of create-namespace, while creating a new 
> > create- namespace-greedy command with the new semantics.
> >
> .. And it doesn't even need to be a new command, a simple --greedy 
> option to create-namespace should be sufficient.

Perhaps "--continue" to proceed with creating namespace after the first successful invocation. I agree with Jeff that changing the default semantics would be surprising. The man page can also be fixed up to make it clear that it's the "singleton region capacity search", not "all possible namespaces" that "-r all" implies.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
  2019-08-28 22:38           ` Scargall, Steve
@ 2019-08-28 23:17             ` Verma, Vishal L
  0 siblings, 0 replies; 8+ messages in thread
From: Verma, Vishal L @ 2019-08-28 23:17 UTC (permalink / raw)
  To: Williams, Dan J, Scargall, Steve; +Cc: linux-nvdimm

On Wed, 2019-08-28 at 15:38 -0700, Scargall, Steve wrote:
> Thanks for the clarification.  I have a much better understanding
> now. 
> 
> Updating the ndctl-create-namespace man page to clarify what '
> --region=all' does and does not do in combination with other arguments
> and options would be beneficial.   This would actually provide a more
> immediate solution to the problem.  It sounds like the general
> statement used in the other man pages can remain?  Or are there other
> exceptions?

Yep I have a patch clarifying the --region option for create-namespace.
'all' is used in two ways, --<object>=all  or ndctl-<action> all.

The former acts as a filter, where as the latter acts as a "act on all"
directive.

So 'ndctl disable-namespace --region=region0 all' disables all
namespaces on region0, where as
'ndctl disable-namespace --region=all all' is the same as omitting the
--region option (while still acceptable syntax), and acts on all
namespaces regardless of the region.

> 
> With the proposed implementation that bails out on first error, `
> --continue` seems reasonable vs. `--greedy`.  What do you think about
> `--all-regions` instead?  It is more meaningful towards its intended
> action or use-case, but would it cause confusion with `
> --regions=all`?   Documentation could provide the solution here.  We
> would have to choose an arbitrary short option since `-a` and `-r` are
> already taken.  

I prefer --continue/-c since it automatically extends to buses as well.
As a result, with these changes, 'ndctl create-namespace -c' will create
namespaces on all possible regions on all possible buses.

> 
> Whatever the final decision, we should test for and return a usage
> error for a mutually exclusive set of inputs, i.e. `ndctl create-
> namespace --all-regions --region=region0` doesn't make sense - either
> you want to perform the create operation on all regions or only in
> region0.

With --continue this isn't a big problem as the --region restriction
will render the --continue meaningless.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

end of thread, back to index

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

Linux-NVDIMM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvdimm/0 linux-nvdimm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvdimm linux-nvdimm/ https://lore.kernel.org/linux-nvdimm \
		linux-nvdimm@lists.01.org linux-nvdimm@archiver.kernel.org
	public-inbox-index linux-nvdimm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.01.lists.linux-nvdimm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox