[ndctl PATCH 0/2] ndctl: Cross-arch compatible namespace alignment

[ndctl PATCH 0/2] ndctl: Cross-arch compatible namespace alignment

[Dan Williams]

A follow-up to the region-align kernel enabling, [1], update ndctl to
enumerate region alignment, and update the unit tests to account for the
kernel's default compatible alignment.

Given that changing the default is a surefire way to create incompatible
namespaces I do not think it makes sense to plumb region alignment
through the create-namespace interface. Only expert users would be
expected to trawl through sysfs and set a non-default alignment. I.e.
the lack of documentation and enabling for setting this value in the
ndctl man pages is deliberate.

---

Dan Williams (2):
      ndctl/region: Support ndctl_region_{get,set}_align()
      ndctl/namespace: Improve namespace action failure messages


 ndctl/lib/libndctl.c   |   35 ++++++++++++++++++++
 ndctl/lib/libndctl.sym |    2 +
 ndctl/libndctl.h       |    2 +
 ndctl/list.c           |    5 +++
 ndctl/namespace.c      |   82 ++++++++++++++++++++++++++++++++----------------
 test/blk_namespaces.c  |    1 +
 test/dpa-alloc.c       |   10 +++++-
 test/dsm-fail.c        |    5 ++-
 test/libndctl.c        |   10 +++++-
 test/parent-uuid.c     |    1 +
 10 files changed, 121 insertions(+), 32 deletions(-)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[ndctl PATCH 1/2] ndctl/region: Support ndctl_region_{get, set}_align()

[Dan Williams]

Add support for the new kernel facility to set space alignment
constraints at the region level. Update the unit tests to bypass the
default 16MiB alignment constraint. Add the new parameter to the default
region listing given how central it is to understanding the valid values
for "create-namespace --size=...".

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/lib/libndctl.c   |   35 +++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym |    2 ++
 ndctl/libndctl.h       |    2 ++
 ndctl/list.c           |    5 +++++
 test/blk_namespaces.c  |    1 +
 test/dpa-alloc.c       |   10 ++++++++--
 test/dsm-fail.c        |    5 ++++-
 test/libndctl.c        |   10 ++++++++--
 test/parent-uuid.c     |    1 +
 9 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 469815a8f04b..1046db2bc1af 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -150,6 +150,7 @@ struct ndctl_region {
 	struct kmod_module *module;
 	struct ndctl_bus *bus;
 	int id, num_mappings, nstype, range_index, ro;
+	unsigned long align;
 	int mappings_init;
 	int namespaces_init;
 	int btts_init;
@@ -1109,6 +1110,34 @@ NDCTL_EXPORT int ndctl_region_set_ro(struct ndctl_region *region, int ro)
 	return ro;
 }
 
+NDCTL_EXPORT unsigned long ndctl_region_get_align(struct ndctl_region *region)
+{
+	return region->align;
+}
+
+NDCTL_EXPORT int ndctl_region_set_align(struct ndctl_region *region,
+		unsigned long align)
+{
+	struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
+	char *path = region->region_buf;
+	int len = region->buf_len, rc;
+	char buf[SYSFS_ATTR_SIZE];
+
+	if (snprintf(path, len, "%s/align", region->region_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				ndctl_region_get_devname(region));
+		return -ENXIO;
+	}
+
+	sprintf(buf, "%#lx\n", align);
+	rc = sysfs_write_attr(ctx, path, buf);
+	if (rc < 0)
+		return rc;
+
+	region->align = align;
+	return 0;
+}
+
 NDCTL_EXPORT unsigned long long ndctl_region_get_resource(struct ndctl_region *region)
 {
 	struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
@@ -2158,6 +2187,12 @@ static void *add_region(void *parent, int id, const char *region_base)
 	else
 		region->target_node = -1;
 
+	sprintf(path, "%s/align", region_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0)
+		region->align = strtoul(buf, NULL, 0);
+	else
+		region->align = ULONG_MAX;
+
 	if (region_set_type(region, path) < 0)
 		goto err_read;
 
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index bf049af1393a..ac575a23d035 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -428,4 +428,6 @@ LIBNDCTL_23 {
 	ndctl_namespace_is_configuration_idle;
 	ndctl_namespace_get_target_node;
 	ndctl_region_get_target_node;
+	ndctl_region_get_align;
+	ndctl_region_set_align;
 } LIBNDCTL_22;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 208240b20aee..076c34583b7d 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -372,6 +372,8 @@ struct ndctl_namespace *ndctl_region_get_namespace_seed(
 		struct ndctl_region *region);
 int ndctl_region_get_ro(struct ndctl_region *region);
 int ndctl_region_set_ro(struct ndctl_region *region, int ro);
+unsigned long ndctl_region_get_align(struct ndctl_region *region);
+int ndctl_region_set_align(struct ndctl_region *region, unsigned long align);
 unsigned long long ndctl_region_get_resource(struct ndctl_region *region);
 struct ndctl_btt *ndctl_region_get_btt_seed(struct ndctl_region *region);
 struct ndctl_pfn *ndctl_region_get_pfn_seed(struct ndctl_region *region);
diff --git a/ndctl/list.c b/ndctl/list.c
index 125a9fe34cb8..3d3ca40d56fe 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -95,6 +95,11 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 		goto err;
 	json_object_object_add(jregion, "size", jobj);
 
+	jobj = util_json_object_size(ndctl_region_get_align(region), flags);
+	if (!jobj)
+		goto err;
+	json_object_object_add(jregion, "align", jobj);
+
 	jobj = util_json_object_size(ndctl_region_get_available_size(region),
 			flags);
 	if (!jobj)
diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
index b587ab93fbb8..437fcad0a8f5 100644
--- a/test/blk_namespaces.c
+++ b/test/blk_namespaces.c
@@ -54,6 +54,7 @@ static struct ndctl_namespace *create_blk_namespace(int region_fraction,
 	unsigned long long size;
 	uuid_t uuid;
 
+	ndctl_region_set_align(region, sysconf(_SC_PAGESIZE));
 	ndctl_namespace_foreach(region, ndns)
 		if (ndctl_namespace_get_size(ndns) == 0) {
 			seed_ns = ndns;
diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
index 9a9c6b64c504..b757b9ad9c2c 100644
--- a/test/dpa-alloc.c
+++ b/test/dpa-alloc.c
@@ -58,15 +58,21 @@ static int do_test(struct ndctl_ctx *ctx, struct ndctl_test *test)
 	bus = ndctl_bus_get_by_provider(ctx, NFIT_PROVIDER1);
 	if (!bus)
 		return -ENXIO;
-	ndctl_region_foreach(bus, region)
+	ndctl_region_foreach(bus, region) {
 		ndctl_region_disable_invalidate(region);
+		ndctl_region_set_align(region, sysconf(_SC_PAGESIZE)
+				* ndctl_region_get_interleave_ways(region));
+	}
 
 	/* init nfit_test.0 */
 	bus = ndctl_bus_get_by_provider(ctx, NFIT_PROVIDER0);
 	if (!bus)
 		return -ENXIO;
-	ndctl_region_foreach(bus, region)
+	ndctl_region_foreach(bus, region) {
 		ndctl_region_disable_invalidate(region);
+		ndctl_region_set_align(region, sysconf(_SC_PAGESIZE)
+				* ndctl_region_get_interleave_ways(region));
+	}
 
 	ndctl_dimm_foreach(bus, dimm) {
 		rc = ndctl_dimm_zero_labels(dimm);
diff --git a/test/dsm-fail.c b/test/dsm-fail.c
index 6e812aec008f..b2c51db4aa3a 100644
--- a/test/dsm-fail.c
+++ b/test/dsm-fail.c
@@ -48,8 +48,11 @@ static int reset_bus(struct ndctl_bus *bus)
 	}
 
 	/* set regions back to their default state */
-	ndctl_region_foreach(bus, region)
+	ndctl_region_foreach(bus, region) {
 		ndctl_region_enable(region);
+		ndctl_region_set_align(region, sysconf(_SC_PAGESIZE)
+				* ndctl_region_get_interleave_ways(region));
+	}
 	return 0;
 }
 
diff --git a/test/libndctl.c b/test/libndctl.c
index 02bb9ccaa465..9ad8f87b92dc 100644
--- a/test/libndctl.c
+++ b/test/libndctl.c
@@ -2622,9 +2622,15 @@ static int do_test0(struct ndctl_ctx *ctx, struct ndctl_test *test)
 		}
 	}
 
-	/* set regions back to their default state */
-	ndctl_region_foreach(bus, region)
+	/*
+	 * Enable regions and adjust the space-align to drop the default
+	 * alignment constraints
+	 */
+	ndctl_region_foreach(bus, region) {
 		ndctl_region_enable(region);
+		ndctl_region_set_align(region, sysconf(_SC_PAGESIZE)
+				* ndctl_region_get_interleave_ways(region));
+	}
 
 	/* pfn and dax tests require vmalloc-enabled nfit_test */
 	if (ndctl_test_attempt(test, KERNEL_VERSION(4, 8, 0))) {
diff --git a/test/parent-uuid.c b/test/parent-uuid.c
index 3a63f7244e21..f41ca2c7bd75 100644
--- a/test/parent-uuid.c
+++ b/test/parent-uuid.c
@@ -61,6 +61,7 @@ static struct ndctl_namespace *create_blk_namespace(int region_fraction,
 	struct ndctl_namespace *ndns, *seed_ns = NULL;
 	unsigned long long size;
 
+	ndctl_region_set_align(region, sysconf(_SC_PAGESIZE));
 	ndctl_namespace_foreach(region, ndns)
 		if (ndctl_namespace_get_size(ndns) == 0) {
 			seed_ns = ndns;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[ndctl PATCH 2/2] ndctl/namespace: Improve namespace action failure messages

[Dan Williams]

With the new kernel updates to enforce wider alignment constraints by
default, it is increasingly important to report why provisioning failed.
Move some debug messages to error messages when they indicate a hard
failure, and drop the generic message when something more precise has
been emitted.

Before:
#  ndctl create-namespace -m fsdax -s 1073750016 -a 4k
failed to create namespace: Invalid argument

After:
#  ndctl create-namespace -m fsdax -s 1073750016 -a 4k
  Error: create namespace: region2: align setting is 0x1000000 size 0x40002000 is misaligned

Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ndctl/namespace.c |   82 ++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 27 deletions(-)

diff --git a/ndctl/namespace.c b/ndctl/namespace.c
index 994b4e8791ea..1de189d8e69a 100644
--- a/ndctl/namespace.c
+++ b/ndctl/namespace.c
@@ -57,6 +57,8 @@ static struct parameters {
 	.autolabel = true,
 };
 
+const char *cmd_name = "namespace";
+
 void builtin_xaction_namespace_reset(void)
 {
 	/*
@@ -88,6 +90,10 @@ struct parsed_parameters {
 		do { } while (0); \
 	}})
 
+static int err_count;
+#define err(fmt, ...) \
+	({ err_count++; error("%s: " fmt, cmd_name, ##__VA_ARGS__);})
+
 #define BASE_OPTIONS() \
 OPT_STRING('b', "bus", &param.bus, "bus-id", \
 	"limit namespace to a bus with an id or provider of <bus-id>"), \
@@ -325,7 +331,7 @@ static const char *parse_namespace_options(int argc, const char **argv,
 do { \
 	int __rc = prefix##_##op(dev, p); \
 	if (__rc) { \
-		debug("%s: " #op " failed: %s\n", \
+		err("%s: " #op " failed: %s\n", \
 				prefix##_get_devname(dev), \
 				strerror(abs(__rc))); \
 		return __rc; \
@@ -475,6 +481,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 	unsigned long long size_align, units = 1, resource;
 	struct ndctl_pfn *pfn = NULL;
 	struct ndctl_dax *dax = NULL;
+	unsigned long region_align;
 	unsigned int ways;
 	int rc = 0;
 
@@ -492,7 +499,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 
 	if (param.uuid) {
 		if (uuid_parse(param.uuid, p->uuid) != 0) {
-			debug("%s: invalid uuid\n", __func__);
+			err("%s: invalid uuid\n", __func__);
 			return -EINVAL;
 		}
 	} else
@@ -504,7 +511,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		rc = snprintf(p->name, sizeof(p->name), "%s",
 				ndctl_namespace_get_alt_name(ndns));
 	if (rc >= (int) sizeof(p->name)) {
-		debug("%s: alt name overflow\n", __func__);
+		err("%s: alt name overflow\n", __func__);
 		return -EINVAL;
 	}
 
@@ -523,7 +530,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		if (ndctl_region_get_type(region) != ND_DEVICE_REGION_PMEM
 				&& (p->mode == NDCTL_NS_MODE_MEMORY
 					|| p->mode == NDCTL_NS_MODE_DAX)) {
-			debug("blk %s does not support %s mode\n", region_name,
+			err("blk %s does not support %s mode\n", region_name,
 					p->mode == NDCTL_NS_MODE_MEMORY
 					? "fsdax" : "devdax");
 			return -EAGAIN;
@@ -534,7 +541,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 	if (p->mode == NDCTL_NS_MODE_MEMORY) {
 		pfn = ndctl_region_get_pfn_seed(region);
 		if (!pfn && param.mode_default) {
-			debug("%s fsdax mode not available\n", region_name);
+			err("%s fsdax mode not available\n", region_name);
 			p->mode = NDCTL_NS_MODE_RAW;
 		}
 		/*
@@ -645,6 +652,14 @@ static int validate_namespace_options(struct ndctl_region *region,
 		}
 	}
 
+	region_align = ndctl_region_get_align(region);
+	if (region_align < ULONG_MAX && p->size % region_align) {
+		err("%s: align setting is %#lx size %#llx is misaligned\n",
+				ndctl_region_get_devname(region), region_align,
+				p->size);
+		return -EINVAL;
+	}
+
 	size_align = p->align;
 
 	/* (re-)validate that the size satisfies the alignment */
@@ -671,7 +686,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		p->size++;
 		p->size *= size_align;
 		p->size /= units;
-		error("'--size=' must align to interleave-width: %d and alignment: %ld\n"
+		err("'--size=' must align to interleave-width: %d and alignment: %ld\n"
 				"did you intend --size=%lld%s?\n",
 				ways, p->align, p->size, suffix);
 		return -EINVAL;
@@ -685,7 +700,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 		btt = ndctl_region_get_btt_seed(region);
 		if (p->mode == NDCTL_NS_MODE_SAFE) {
 			if (!btt) {
-				debug("%s: does not support 'sector' mode\n",
+				err("%s: does not support 'sector' mode\n",
 						region_name);
 				return -EINVAL;
 			}
@@ -695,7 +710,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 						== p->sector_size)
 					break;
 			if (i >= num) {
-				debug("%s: does not support btt sector_size %lu\n",
+				err("%s: does not support btt sector_size %lu\n",
 						region_name, p->sector_size);
 				return -EINVAL;
 			}
@@ -710,7 +725,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 						== p->sector_size)
 					break;
 			if (i >= num) {
-				debug("%s: does not support namespace sector_size %lu\n",
+				err("%s: does not support namespace sector_size %lu\n",
 						region_name, p->sector_size);
 				return -EINVAL;
 			}
@@ -752,7 +767,7 @@ static int validate_namespace_options(struct ndctl_region *region,
 
 		if (ndns && p->mode != NDCTL_NS_MODE_MEMORY
 			&& p->mode != NDCTL_NS_MODE_DAX) {
-			debug("%s: --map= only valid for fsdax mode namespace\n",
+			err("%s: --map= only valid for fsdax mode namespace\n",
 				ndctl_namespace_get_devname(ndns));
 			return -EINVAL;
 		}
@@ -850,7 +865,7 @@ static int zero_info_block(struct ndctl_namespace *ndns)
 	ndctl_namespace_set_raw_mode(ndns, 1);
 	rc = ndctl_namespace_enable(ndns);
 	if (rc < 0) {
-		debug("%s failed to enable for zeroing, continuing\n", devname);
+		err("%s failed to enable for zeroing, continuing\n", devname);
 		rc = 1;
 		goto out;
 	}
@@ -867,7 +882,7 @@ static int zero_info_block(struct ndctl_namespace *ndns)
 	sprintf(path, "/dev/%s", ndctl_namespace_get_block_device(ndns));
 	fd = open(path, O_RDWR|O_DIRECT|O_EXCL);
 	if (fd < 0) {
-		debug("%s: failed to open %s to zero info block\n",
+		err("%s: failed to open %s to zero info block\n",
 				devname, path);
 		goto out;
 	}
@@ -875,7 +890,7 @@ static int zero_info_block(struct ndctl_namespace *ndns)
 	memset(buf, 0, info_size);
 	rc = pread(fd, read_buf, info_size, 0);
 	if (rc < info_size) {
-		debug("%s: failed to read info block, continuing\n",
+		err("%s: failed to read info block, continuing\n",
 			devname);
 	}
 	if (memcmp(buf, read_buf, info_size) == 0) {
@@ -885,7 +900,7 @@ static int zero_info_block(struct ndctl_namespace *ndns)
 
 	rc = pwrite(fd, buf, info_size, 0);
 	if (rc < info_size) {
-		debug("%s: failed to zero info block %s\n",
+		err("%s: failed to zero info block %s\n",
 				devname, path);
 		rc = -ENXIO;
 	} else
@@ -1046,7 +1061,7 @@ retry:
 out:
 	ndctl_region_enable(region);
 	if (ndctl_region_get_nstype(region) != ND_DEVICE_NAMESPACE_PMEM) {
-		debug("%s: failed to initialize labels\n",
+		err("%s: failed to initialize labels\n",
 				ndctl_region_get_devname(region));
 		return -EBUSY;
 	}
@@ -1101,19 +1116,19 @@ static int bus_send_clear(struct ndctl_bus *bus, unsigned long long start,
 	/* get ars_cap */
 	cmd_cap = ndctl_bus_cmd_new_ars_cap(bus, start, size);
 	if (!cmd_cap) {
-		debug("bus: %s failed to create cmd\n", busname);
+		err("bus: %s failed to create cmd\n", busname);
 		return -ENOTTY;
 	}
 
 	rc = ndctl_cmd_submit_xlat(cmd_cap);
 	if (rc < 0) {
-		debug("bus: %s failed to submit cmd: %d\n", busname, rc);
+		err("bus: %s failed to submit cmd: %d\n", busname, rc);
 		goto out_cap;
 	}
 
 	/* send clear_error */
 	if (ndctl_cmd_ars_cap_get_range(cmd_cap, &range)) {
-		debug("bus: %s failed to get ars_cap range\n", busname);
+		err("bus: %s failed to get ars_cap range\n", busname);
 		rc = -ENXIO;
 		goto out_cap;
 	}
@@ -1121,20 +1136,20 @@ static int bus_send_clear(struct ndctl_bus *bus, unsigned long long start,
 	cmd_clear = ndctl_bus_cmd_new_clear_error(range.address,
 					range.length, cmd_cap);
 	if (!cmd_clear) {
-		debug("bus: %s failed to create cmd\n", busname);
+		err("bus: %s failed to create cmd\n", busname);
 		rc = -ENOTTY;
 		goto out_cap;
 	}
 
 	rc = ndctl_cmd_submit_xlat(cmd_clear);
 	if (rc < 0) {
-		debug("bus: %s failed to submit cmd: %d\n", busname, rc);
+		err("bus: %s failed to submit cmd: %d\n", busname, rc);
 		goto out_clr;
 	}
 
 	cleared = ndctl_cmd_clear_error_get_cleared(cmd_clear);
 	if (cleared != range.length) {
-		debug("bus: %s expected to clear: %lld actual: %lld\n",
+		err("bus: %s expected to clear: %lld actual: %lld\n",
 				busname, range.length, cleared);
 		rc = -ENXIO;
 	}
@@ -1356,6 +1371,19 @@ static int do_xaction_namespace(const char *namespace,
 	if (verbose)
 		ndctl_set_log_priority(ctx, LOG_DEBUG);
 
+	if (action == ACTION_ENABLE)
+		cmd_name = "enable namespace";
+	else if (action == ACTION_DISABLE)
+		cmd_name = "disable namespace";
+	else if (action == ACTION_CREATE)
+		cmd_name = "create namespace";
+	else if (action == ACTION_DESTROY)
+		cmd_name = "destroy namespace";
+	else if (action == ACTION_CHECK)
+		cmd_name = "check namespace";
+	else if (action == ACTION_CLEAR)
+		cmd_name = "clear errors namespace";
+
         ndctl_bus_foreach(ctx, bus) {
 		bool do_scrub;
 
@@ -1478,7 +1506,7 @@ int cmd_disable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 	int disabled, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_DISABLE, ctx, &disabled);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error disabling namespaces: %s\n",
 				strerror(-rc));
 
@@ -1495,7 +1523,7 @@ int cmd_enable_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 	int enabled, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_ENABLE, ctx, &enabled);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error enabling namespaces: %s\n",
 				strerror(-rc));
 	fprintf(stderr, "enabled %d namespace%s\n", enabled,
@@ -1525,7 +1553,7 @@ int cmd_create_namespace(int argc, const char **argv, struct ndctl_ctx *ctx)
 	if (param.greedy)
 		fprintf(stderr, "created %d namespace%s\n", created,
 			created == 1 ? "" : "s");
-	if (rc < 0 || (!namespace && created < 1)) {
+	if ((rc < 0 || (!namespace && created < 1)) && !err_count) {
 		fprintf(stderr, "failed to %s namespace: %s\n", namespace
 				? "reconfigure" : "create", strerror(-rc));
 		if (!namespace)
@@ -1543,7 +1571,7 @@ int cmd_destroy_namespace(int argc , const char **argv, struct ndctl_ctx *ctx)
 	int destroyed, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_DESTROY, ctx, &destroyed);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error destroying namespaces: %s\n",
 				strerror(-rc));
 	fprintf(stderr, "destroyed %d namespace%s\n", destroyed,
@@ -1559,7 +1587,7 @@ int cmd_check_namespace(int argc , const char **argv, struct ndctl_ctx *ctx)
 	int checked, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_CHECK, ctx, &checked);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error checking namespaces: %s\n",
 				strerror(-rc));
 	fprintf(stderr, "checked %d namespace%s\n", checked,
@@ -1575,7 +1603,7 @@ int cmd_clear_errors(int argc , const char **argv, struct ndctl_ctx *ctx)
 	int cleared, rc;
 
 	rc = do_xaction_namespace(namespace, ACTION_CLEAR, ctx, &cleared);
-	if (rc < 0)
+	if (rc < 0 && !err_count)
 		fprintf(stderr, "error clearing namespaces: %s\n",
 				strerror(-rc));
 	fprintf(stderr, "cleared %d namespace%s\n", cleared,
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [ndctl PATCH 1/2] ndctl/region: Support ndctl_region_{get, set}_align()

[Jeff Moyer]

Dan Williams <dan.j.williams@intel.com> writes:

>  
> +NDCTL_EXPORT unsigned long ndctl_region_get_align(struct ndctl_region *region)
> +{
> +	return region->align;
> +}
> +
> +NDCTL_EXPORT int ndctl_region_set_align(struct ndctl_region *region,
> +		unsigned long align)
> +{
> +	struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
> +	char *path = region->region_buf;
> +	int len = region->buf_len, rc;
> +	char buf[SYSFS_ATTR_SIZE];
> +
> +	if (snprintf(path, len, "%s/align", region->region_path) >= len) {
> +		err(ctx, "%s: buffer too small!\n",
> +				ndctl_region_get_devname(region));
> +		return -ENXIO;
> +	}
> +
> +	sprintf(buf, "%#lx\n", align);
> +	rc = sysfs_write_attr(ctx, path, buf);
> +	if (rc < 0)
> +		return rc;
> +
> +	region->align = align;
> +	return 0;
> +}
> +

Missing doctext.  Specifically, there should be a big, fat warning
against changing the region alignment.

-Jeff
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [ndctl PATCH 1/2] ndctl/region: Support ndctl_region_{get, set}_align()

[Dan Williams]

On Wed, Feb 19, 2020 at 10:04 AM Jeff Moyer <jmoyer@redhat.com> wrote:
>
> Dan Williams <dan.j.williams@intel.com> writes:
>
> >
> > +NDCTL_EXPORT unsigned long ndctl_region_get_align(struct ndctl_region *region)
> > +{
> > +     return region->align;
> > +}
> > +
> > +NDCTL_EXPORT int ndctl_region_set_align(struct ndctl_region *region,
> > +             unsigned long align)
> > +{
> > +     struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
> > +     char *path = region->region_buf;
> > +     int len = region->buf_len, rc;
> > +     char buf[SYSFS_ATTR_SIZE];
> > +
> > +     if (snprintf(path, len, "%s/align", region->region_path) >= len) {
> > +             err(ctx, "%s: buffer too small!\n",
> > +                             ndctl_region_get_devname(region));
> > +             return -ENXIO;
> > +     }
> > +
> > +     sprintf(buf, "%#lx\n", align);
> > +     rc = sysfs_write_attr(ctx, path, buf);
> > +     if (rc < 0)
> > +             return rc;
> > +
> > +     region->align = align;
> > +     return 0;
> > +}
> > +
>
> Missing doctext.  Specifically, there should be a big, fat warning
> against changing the region alignment.

I don't mind adding one, but is this the right place to document an
API warning? If the audience is future ndctl developers that should be
warned to keep the status quo of not plumbing this capability into
"create-namespace" that's one message. If it's to stop other libndctl
application developers, they'll likely never see this source file.

As it stands there is
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [ndctl PATCH 1/2] ndctl/region: Support ndctl_region_{get, set}_align()

[Dan Williams]

On Tue, Feb 25, 2020 at 3:23 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Wed, Feb 19, 2020 at 10:04 AM Jeff Moyer <jmoyer@redhat.com> wrote:
> >
> > Dan Williams <dan.j.williams@intel.com> writes:
> >
> > >
> > > +NDCTL_EXPORT unsigned long ndctl_region_get_align(struct ndctl_region *region)
> > > +{
> > > +     return region->align;
> > > +}
> > > +
> > > +NDCTL_EXPORT int ndctl_region_set_align(struct ndctl_region *region,
> > > +             unsigned long align)
> > > +{
> > > +     struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
> > > +     char *path = region->region_buf;
> > > +     int len = region->buf_len, rc;
> > > +     char buf[SYSFS_ATTR_SIZE];
> > > +
> > > +     if (snprintf(path, len, "%s/align", region->region_path) >= len) {
> > > +             err(ctx, "%s: buffer too small!\n",
> > > +                             ndctl_region_get_devname(region));
> > > +             return -ENXIO;
> > > +     }
> > > +
> > > +     sprintf(buf, "%#lx\n", align);
> > > +     rc = sysfs_write_attr(ctx, path, buf);
> > > +     if (rc < 0)
> > > +             return rc;
> > > +
> > > +     region->align = align;
> > > +     return 0;
> > > +}
> > > +
> >
> > Missing doctext.  Specifically, there should be a big, fat warning
> > against changing the region alignment.
>
> I don't mind adding one, but is this the right place to document an
> API warning? If the audience is future ndctl developers that should be
> warned to keep the status quo of not plumbing this capability into
> "create-namespace" that's one message. If it's to stop other libndctl
> application developers, they'll likely never see this source file.
>
> As it stands there is

Whoops, meant to delete this before sending...
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [ndctl PATCH 1/2] ndctl/region: Support ndctl_region_{get, set}_align()

[Jeff Moyer]

Dan Williams <dan.j.williams@intel.com> writes:

>> Missing doctext.  Specifically, there should be a big, fat warning
>> against changing the region alignment.
>
> I don't mind adding one, but is this the right place to document an
> API warning? If the audience is future ndctl developers that should be
> warned to keep the status quo of not plumbing this capability into
> "create-namespace" that's one message. If it's to stop other libndctl
> application developers, they'll likely never see this source file.

I meant to target users of the library (not ndctl developers).  I
thought that was the reason for the doctext on exported interfaces.  No?

I admit, I don't know how users of libndctl figure *anything* out about
how to use it.  :)

-Jeff
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [ndctl PATCH 1/2] ndctl/region: Support ndctl_region_{get, set}_align()

[Dan Williams]

On Wed, Feb 26, 2020 at 1:52 PM Jeff Moyer <jmoyer@redhat.com> wrote:
>
> Dan Williams <dan.j.williams@intel.com> writes:
>
> >> Missing doctext.  Specifically, there should be a big, fat warning
> >> against changing the region alignment.
> >
> > I don't mind adding one, but is this the right place to document an
> > API warning? If the audience is future ndctl developers that should be
> > warned to keep the status quo of not plumbing this capability into
> > "create-namespace" that's one message. If it's to stop other libndctl
> > application developers, they'll likely never see this source file.
>
> I meant to target users of the library (not ndctl developers).  I
> thought that was the reason for the doctext on exported interfaces.  No?
>
> I admit, I don't know how users of libndctl figure *anything* out about
> how to use it.  :)
>

Right, that's why I was confused about what you were asking. We
haven't yet formalized a library documentation system, which is bad.
I'll add kernel-doc for this function, and add an item to the backlog
to figure out how to build library-documentation from those
annotations. The developer's guide to date has unfortunately been "go
review how ndctl uses it".
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [ndctl PATCH 1/2] ndctl/region: Support ndctl_region_{get, set}_align()

[Jeff Moyer]

Dan Williams <dan.j.williams@intel.com> writes:

> On Wed, Feb 26, 2020 at 1:52 PM Jeff Moyer <jmoyer@redhat.com> wrote:
>>
>> Dan Williams <dan.j.williams@intel.com> writes:
>>
>> >> Missing doctext.  Specifically, there should be a big, fat warning
>> >> against changing the region alignment.
>> >
>> > I don't mind adding one, but is this the right place to document an
>> > API warning? If the audience is future ndctl developers that should be
>> > warned to keep the status quo of not plumbing this capability into
>> > "create-namespace" that's one message. If it's to stop other libndctl
>> > application developers, they'll likely never see this source file.
>>
>> I meant to target users of the library (not ndctl developers).  I
>> thought that was the reason for the doctext on exported interfaces.  No?
>>
>> I admit, I don't know how users of libndctl figure *anything* out about
>> how to use it.  :)
>>
>
> Right, that's why I was confused about what you were asking. We
> haven't yet formalized a library documentation system, which is bad.
> I'll add kernel-doc for this function, and add an item to the backlog
> to figure out how to build library-documentation from those
> annotations. The developer's guide to date has unfortunately been "go
> review how ndctl uses it".

OK, thanks a lot!

-Jeff
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org