| Message ID | 147033593728.33374.15172970808845415226.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 08/05/2016 02:42 AM, Dan Williams wrote: > While the kernel will prevent invalid configurations, the user > experience of a kernel error message and disabling of the namespace is > too harsh. Trap and report these attempts to make create-namespace more > user friendly. > > # ndctl create-namespace -e namespace0.0 -m sector -l 513 -f -v > validate_namespace_options:437: region0: does not support btt sector_size 513 > failed to reconfigure namespace > > Reported-by: Yi Zhang <yizhan@redhat.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > Changes in v2 to address unit test breakage: > > 1/ stop setting a default sector size for cases where create-namespace > is operating in pmem mode. > > 2/ don't set ndns in validate_namespace(), use a separate 'seed' > variable, otherwise we mistakenly change a 'create' to a 'reconfigure' > operation. > > ndctl/builtin-xaction-namespace.c | 51 +++++++++++++++++++++++++++++++------ > 1 file changed, 43 insertions(+), 8 deletions(-) > > diff --git a/ndctl/builtin-xaction-namespace.c b/ndctl/builtin-xaction-namespace.c > index 8304203750d8..76d06bbb53ba 100644 > --- a/ndctl/builtin-xaction-namespace.c > +++ b/ndctl/builtin-xaction-namespace.c > @@ -210,8 +210,13 @@ static int set_defaults(enum namespace_action mode) > error("'pmem' namespaces do not support setting 'sector size'\n"); > rc = -EINVAL; > } > - } else if (!param.reconfig) > - param.sector_size = "4096"; > + } else if (!param.reconfig > + && (param.type && strcmp(param.type, "blk") == 0 Got below warning when compiling with this patch builtin-xaction-namespace.c: In function ‘set_defaults’: builtin-xaction-namespace.c:214:19: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] && (param.type && strcmp(param.type, "blk") == 0 ^ CC builtin-xable-region.o CC builtin-list.o CC builtin-test.o CC builtin-help.o CC builtin-zero-labels.o CC builtin-read-labels.o CC util/json.o CC util/json-smart.o CCLD ndctl GEN lib/libndctl.pc > + || (param.mode > + && strcmp(param.mode, "safe") == 0))) { > + /* default sector size for blk-type or safe-mode */ > + param.sector_size = "4096"; > + } > > return rc; > } > @@ -415,14 +420,44 @@ static int validate_namespace_options(struct ndctl_region *region, > p->mode = ndctl_namespace_get_mode(ndns); > > if (param.sector_size) { > - p->sector_size = parse_size64(param.sector_size); > + struct ndctl_btt *btt; > + int num, i; > > - if (ndns && p->mode != NDCTL_NS_MODE_SAFE > - && ndctl_namespace_get_type(ndns) > - == ND_DEVICE_NAMESPACE_PMEM) { > - debug("%s: does not support sector_size modification\n", > - ndctl_namespace_get_devname(ndns)); > + p->sector_size = parse_size64(param.sector_size); > + btt = ndctl_region_get_btt_seed(region); > + if (p->mode == NDCTL_NS_MODE_SAFE) { > + if (!btt) { > + debug("%s: does not support 'sector' mode\n", > + ndctl_region_get_devname(region)); > + return -EINVAL; > + } > + num = ndctl_btt_get_num_sector_sizes(btt); > + for (i = 0; i < num; i++) > + if (ndctl_btt_get_supported_sector_size(btt, i) > + == p->sector_size) > + break; > + if (i >= num) { > + debug("%s: does not support btt sector_size %lu\n", > + ndctl_region_get_devname(region), > + p->sector_size); > + return -EINVAL; > + } > + } else { > + struct ndctl_namespace *seed = ndns; > + > + if (!seed) > + seed = ndctl_region_get_namespace_seed(region); > + num = ndctl_namespace_get_num_sector_sizes(seed); > + for (i = 0; i < num; i++) > + if (ndctl_namespace_get_supported_sector_size(seed, i) > + == p->sector_size) > + break; > + if (i >= num) { > + debug("%s: does not support namespace sector_size %lu\n", > + ndctl_region_get_devname(region), > + p->sector_size); > return -EINVAL; > + } > } > } else if (ndns) { > struct ndctl_btt *btt = ndctl_namespace_get_btt(ndns); >
diff --git a/ndctl/builtin-xaction-namespace.c b/ndctl/builtin-xaction-namespace.c index 8304203750d8..76d06bbb53ba 100644 --- a/ndctl/builtin-xaction-namespace.c +++ b/ndctl/builtin-xaction-namespace.c @@ -210,8 +210,13 @@ static int set_defaults(enum namespace_action mode) error("'pmem' namespaces do not support setting 'sector size'\n"); rc = -EINVAL; } - } else if (!param.reconfig) - param.sector_size = "4096"; + } else if (!param.reconfig + && (param.type && strcmp(param.type, "blk") == 0 + || (param.mode + && strcmp(param.mode, "safe") == 0))) { + /* default sector size for blk-type or safe-mode */ + param.sector_size = "4096"; + } return rc; } @@ -415,14 +420,44 @@ static int validate_namespace_options(struct ndctl_region *region, p->mode = ndctl_namespace_get_mode(ndns); if (param.sector_size) { - p->sector_size = parse_size64(param.sector_size); + struct ndctl_btt *btt; + int num, i; - if (ndns && p->mode != NDCTL_NS_MODE_SAFE - && ndctl_namespace_get_type(ndns) - == ND_DEVICE_NAMESPACE_PMEM) { - debug("%s: does not support sector_size modification\n", - ndctl_namespace_get_devname(ndns)); + p->sector_size = parse_size64(param.sector_size); + btt = ndctl_region_get_btt_seed(region); + if (p->mode == NDCTL_NS_MODE_SAFE) { + if (!btt) { + debug("%s: does not support 'sector' mode\n", + ndctl_region_get_devname(region)); + return -EINVAL; + } + num = ndctl_btt_get_num_sector_sizes(btt); + for (i = 0; i < num; i++) + if (ndctl_btt_get_supported_sector_size(btt, i) + == p->sector_size) + break; + if (i >= num) { + debug("%s: does not support btt sector_size %lu\n", + ndctl_region_get_devname(region), + p->sector_size); + return -EINVAL; + } + } else { + struct ndctl_namespace *seed = ndns; + + if (!seed) + seed = ndctl_region_get_namespace_seed(region); + num = ndctl_namespace_get_num_sector_sizes(seed); + for (i = 0; i < num; i++) + if (ndctl_namespace_get_supported_sector_size(seed, i) + == p->sector_size) + break; + if (i >= num) { + debug("%s: does not support namespace sector_size %lu\n", + ndctl_region_get_devname(region), + p->sector_size); return -EINVAL; + } } } else if (ndns) { struct ndctl_btt *btt = ndctl_namespace_get_btt(ndns);
While the kernel will prevent invalid configurations, the user experience of a kernel error message and disabling of the namespace is too harsh. Trap and report these attempts to make create-namespace more user friendly. # ndctl create-namespace -e namespace0.0 -m sector -l 513 -f -v validate_namespace_options:437: region0: does not support btt sector_size 513 failed to reconfigure namespace Reported-by: Yi Zhang <yizhan@redhat.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- Changes in v2 to address unit test breakage: 1/ stop setting a default sector size for cases where create-namespace is operating in pmem mode. 2/ don't set ndns in validate_namespace(), use a separate 'seed' variable, otherwise we mistakenly change a 'create' to a 'reconfigure' operation. ndctl/builtin-xaction-namespace.c | 51 +++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-)