All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Scargall, Steve" <steve.scargall@intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>,
	"Verma, Vishal L" <vishal.l.verma@intel.com>
Cc: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: RE: [ndctl RFC PATCH] ndctl/namespace: create namespaces greedily
Date: Wed, 28 Aug 2019 22:38:02 +0000	[thread overview]
Message-ID: <507921D13093A64EAF066075F3F6ED13076E4922@ORSMSX111.amr.corp.intel.com> (raw)
In-Reply-To: <CAPcyv4gnfRv3uKAFj7gctLpZHphCc_3gPc7MzjbYmndK0aZE_w@mail.gmail.com>

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

  reply	other threads:[~2019-08-28 22:40 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
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 [this message]
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=507921D13093A64EAF066075F3F6ED13076E4922@ORSMSX111.amr.corp.intel.com \
    --to=steve.scargall@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=linux-nvdimm@lists.01.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.