From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: Re: [ndctl PATCH] ndctl: correctly propagate errors from sysfs_write_attr
Date: Tue, 18 Jul 2017 21:14:36 +0000 [thread overview]
Message-ID: <1500412378.6401.57.camel@intel.com> (raw)
In-Reply-To: <20170718203055.11179-1-vishal.l.verma@intel.com>
On Tue, 2017-07-18 at 14:30 -0600, Vishal Verma wrote:
> In libndctl, if a sysfs_write_attr failed, we simply returned ENXIO,
> which would print a confusing error message ("unsupported
> configuration") for something like EACCESS..
>
> Propagate the actuall return from the sysfs helper in all the call
oops, s/actuall/actual/
> sites. Also use strerror() instead of open-coding a translation of
> error
> messages in create-namespace.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
> ndctl/lib/libndctl.c | 95 +++++++++++++++++++++++++++++------------
> -----------
> ndctl/namespace.c | 13 ++-----
> 2 files changed, 56 insertions(+), 52 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 68d8064..dda1345 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -1050,7 +1050,7 @@ NDCTL_EXPORT int ndctl_region_set_ro(struct
> ndctl_region *region, int ro)
> {
> struct ndctl_ctx *ctx = ndctl_region_get_ctx(region);
> char *path = region->region_buf;
> - int len = region->buf_len;
> + int len = region->buf_len, rc;
>
> if (snprintf(path, len, "%s/read_only", region->region_path)
> >= len) {
> err(ctx, "%s: buffer too small!\n",
> @@ -1059,8 +1059,9 @@ NDCTL_EXPORT int ndctl_region_set_ro(struct
> ndctl_region *region, int ro)
> }
>
> ro = !!ro;
> - if (sysfs_write_attr(ctx, path, ro ? "1\n" : "0\n") < 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, ro ? "1\n" : "0\n");
> + if (rc < 0)
> + return rc;
>
> region->ro = ro;
> return ro;
> @@ -3235,7 +3236,7 @@ NDCTL_EXPORT int
> ndctl_namespace_set_raw_mode(struct ndctl_namespace *ndns,
> {
> struct ndctl_ctx *ctx = ndctl_namespace_get_ctx(ndns);
> char *path = ndns->ndns_buf;
> - int len = ndns->buf_len;
> + int len = ndns->buf_len, rc;
>
> if (snprintf(path, len, "%s/force_raw", ndns->ndns_path) >=
> len) {
> err(ctx, "%s: buffer too small!\n",
> @@ -3244,8 +3245,9 @@ NDCTL_EXPORT int
> ndctl_namespace_set_raw_mode(struct ndctl_namespace *ndns,
> }
>
> raw_mode = !!raw_mode;
> - if (sysfs_write_attr(ctx, path, raw_mode ? "1\n" : "0\n") <
> 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, raw_mode ? "1\n" : "0\n");
> + if (rc < 0)
> + return rc;
>
> ndns->raw_mode = raw_mode;
> return raw_mode;
> @@ -3602,7 +3604,7 @@ NDCTL_EXPORT int ndctl_namespace_set_uuid(struct
> ndctl_namespace *ndns, uuid_t u
> {
> struct ndctl_ctx *ctx = ndctl_namespace_get_ctx(ndns);
> char *path = ndns->ndns_buf;
> - int len = ndns->buf_len;
> + int len = ndns->buf_len, rc;
> char uuid[40];
>
> if (snprintf(path, len, "%s/uuid", ndns->ndns_path) >= len) {
> @@ -3612,8 +3614,9 @@ NDCTL_EXPORT int ndctl_namespace_set_uuid(struct
> ndctl_namespace *ndns, uuid_t u
> }
>
> uuid_unparse(uu, uuid);
> - if (sysfs_write_attr(ctx, path, uuid) != 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, uuid);
> + if (rc != 0)
> + return rc;
> memcpy(ndns->uuid, uu, sizeof(uuid_t));
> return 0;
> }
> @@ -3645,7 +3648,7 @@ NDCTL_EXPORT int
> ndctl_namespace_set_sector_size(struct ndctl_namespace *ndns,
> {
> struct ndctl_ctx *ctx = ndctl_namespace_get_ctx(ndns);
> char *path = ndns->ndns_buf;
> - int len = ndns->buf_len;
> + int len = ndns->buf_len, rc;
> char sector_str[40];
> int i;
>
> @@ -3659,8 +3662,9 @@ NDCTL_EXPORT int
> ndctl_namespace_set_sector_size(struct ndctl_namespace *ndns,
> }
>
> sprintf(sector_str, "%d\n", sector_size);
> - if (sysfs_write_attr(ctx, path, sector_str) != 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, sector_str);
> + if (rc != 0)
> + return rc;
>
> for (i = 0; i < ndns->lbasize.num; i++)
> if (ndns->lbasize.supported[i] == sector_size)
> @@ -3680,7 +3684,7 @@ NDCTL_EXPORT int
> ndctl_namespace_set_alt_name(struct ndctl_namespace *ndns,
> {
> struct ndctl_ctx *ctx = ndctl_namespace_get_ctx(ndns);
> char *path = ndns->ndns_buf;
> - int len = ndns->buf_len;
> + int len = ndns->buf_len, rc;
> char *buf;
>
> if (!ndns->alt_name)
> @@ -3699,9 +3703,10 @@ NDCTL_EXPORT int
> ndctl_namespace_set_alt_name(struct ndctl_namespace *ndns,
> if (!buf)
> return -ENOMEM;
>
> - if (sysfs_write_attr(ctx, path, buf) < 0) {
> + rc = sysfs_write_attr(ctx, path, buf);
> + if (rc < 0) {
> free(buf);
> - return -ENXIO;
> + return rc;
> }
>
> free(ndns->alt_name);
> @@ -3724,7 +3729,7 @@ static int namespace_set_size(struct
> ndctl_namespace *ndns,
> {
> struct ndctl_ctx *ctx = ndctl_namespace_get_ctx(ndns);
> char *path = ndns->ndns_buf;
> - int len = ndns->buf_len;
> + int len = ndns->buf_len, rc;
> char buf[SYSFS_ATTR_SIZE];
>
> if (snprintf(path, len, "%s/size", ndns->ndns_path) >= len) {
> @@ -3734,8 +3739,9 @@ static int namespace_set_size(struct
> ndctl_namespace *ndns,
> }
>
> sprintf(buf, "%#llx\n", size);
> - if (sysfs_write_attr(ctx, path, buf) < 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, buf);
> + if (rc < 0)
> + return rc;
>
> ndns->size = size;
> return 0;
> @@ -4008,7 +4014,7 @@ NDCTL_EXPORT int ndctl_btt_set_uuid(struct
> ndctl_btt *btt, uuid_t uu)
> {
> struct ndctl_ctx *ctx = ndctl_btt_get_ctx(btt);
> char *path = btt->btt_buf;
> - int len = btt->buf_len;
> + int len = btt->buf_len, rc;
> char uuid[40];
>
> if (snprintf(path, len, "%s/uuid", btt->btt_path) >= len) {
> @@ -4018,8 +4024,9 @@ NDCTL_EXPORT int ndctl_btt_set_uuid(struct
> ndctl_btt *btt, uuid_t uu)
> }
>
> uuid_unparse(uu, uuid);
> - if (sysfs_write_attr(ctx, path, uuid) != 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, uuid);
> + if (rc != 0)
> + return rc;
> memcpy(btt->uuid, uu, sizeof(uuid_t));
> return 0;
> }
> @@ -4029,7 +4036,7 @@ NDCTL_EXPORT int
> ndctl_btt_set_sector_size(struct ndctl_btt *btt,
> {
> struct ndctl_ctx *ctx = ndctl_btt_get_ctx(btt);
> char *path = btt->btt_buf;
> - int len = btt->buf_len;
> + int len = btt->buf_len, rc;
> char sector_str[40];
> int i;
>
> @@ -4040,8 +4047,9 @@ NDCTL_EXPORT int
> ndctl_btt_set_sector_size(struct ndctl_btt *btt,
> }
>
> sprintf(sector_str, "%d\n", sector_size);
> - if (sysfs_write_attr(ctx, path, sector_str) != 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, sector_str);
> + if (rc != 0)
> + return rc;
>
> for (i = 0; i < btt->lbasize.num; i++)
> if (btt->lbasize.supported[i] == sector_size)
> @@ -4053,8 +4061,8 @@ NDCTL_EXPORT int ndctl_btt_set_namespace(struct
> ndctl_btt *btt,
> struct ndctl_namespace *ndns)
> {
> struct ndctl_ctx *ctx = ndctl_btt_get_ctx(btt);
> + int len = btt->buf_len, rc;
> char *path = btt->btt_buf;
> - int len = btt->buf_len;
>
> if (snprintf(path, len, "%s/namespace", btt->btt_path) >=
> len) {
> err(ctx, "%s: buffer too small!\n",
> @@ -4062,9 +4070,10 @@ NDCTL_EXPORT int ndctl_btt_set_namespace(struct
> ndctl_btt *btt,
> return -ENXIO;
> }
>
> - if (sysfs_write_attr(ctx, path, ndns
> - ? ndctl_namespace_get_devname(ndns) :
> "\n"))
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, ndns
> + ? ndctl_namespace_get_devname(ndns) :
> "\n");
> + if (rc != 0)
> + return rc;
>
> btt->ndns = ndns;
> return 0;
> @@ -4407,8 +4416,8 @@ NDCTL_EXPORT unsigned long long
> ndctl_pfn_get_resource(struct ndctl_pfn *pfn)
> NDCTL_EXPORT int ndctl_pfn_set_uuid(struct ndctl_pfn *pfn, uuid_t uu)
> {
> struct ndctl_ctx *ctx = ndctl_pfn_get_ctx(pfn);
> + int len = pfn->buf_len, rc;
> char *path = pfn->pfn_buf;
> - int len = pfn->buf_len;
> char uuid[40];
>
> if (snprintf(path, len, "%s/uuid", pfn->pfn_path) >= len) {
> @@ -4418,8 +4427,9 @@ NDCTL_EXPORT int ndctl_pfn_set_uuid(struct
> ndctl_pfn *pfn, uuid_t uu)
> }
>
> uuid_unparse(uu, uuid);
> - if (sysfs_write_attr(ctx, path, uuid) != 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, uuid);
> + if (rc != 0)
> + return rc;
> memcpy(pfn->uuid, uu, sizeof(uuid_t));
> return 0;
> }
> @@ -4433,8 +4443,8 @@ NDCTL_EXPORT int ndctl_pfn_set_location(struct
> ndctl_pfn *pfn,
> enum ndctl_pfn_loc loc)
> {
> struct ndctl_ctx *ctx = ndctl_pfn_get_ctx(pfn);
> + int len = pfn->buf_len, rc;
> char *path = pfn->pfn_buf;
> - int len = pfn->buf_len;
> const char *locations[] = {
> [NDCTL_PFN_LOC_NONE] = "none",
> [NDCTL_PFN_LOC_RAM] = "ram",
> @@ -4456,8 +4466,9 @@ NDCTL_EXPORT int ndctl_pfn_set_location(struct
> ndctl_pfn *pfn,
> return -ENXIO;
> }
>
> - if (sysfs_write_attr(ctx, path, locations[loc]) != 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, locations[loc]);
> + if (rc != 0)
> + return rc;
> pfn->loc = loc;
> return 0;
> }
> @@ -4486,8 +4497,8 @@ NDCTL_EXPORT int ndctl_pfn_has_align(struct
> ndctl_pfn *pfn)
> NDCTL_EXPORT int ndctl_pfn_set_align(struct ndctl_pfn *pfn, unsigned
> long align)
> {
> struct ndctl_ctx *ctx = ndctl_pfn_get_ctx(pfn);
> + int len = pfn->buf_len, rc;
> char *path = pfn->pfn_buf;
> - int len = pfn->buf_len;
> char align_str[40];
>
> if (snprintf(path, len, "%s/align", pfn->pfn_path) >= len) {
> @@ -4497,8 +4508,9 @@ NDCTL_EXPORT int ndctl_pfn_set_align(struct
> ndctl_pfn *pfn, unsigned long align)
> }
>
> sprintf(align_str, "%lu\n", align);
> - if (sysfs_write_attr(ctx, path, align_str) != 0)
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, align_str);
> + if (rc != 0)
> + return rc;
> pfn->align = align;
> return 0;
> }
> @@ -4507,8 +4519,8 @@ NDCTL_EXPORT int ndctl_pfn_set_namespace(struct
> ndctl_pfn *pfn,
> struct ndctl_namespace *ndns)
> {
> struct ndctl_ctx *ctx = ndctl_pfn_get_ctx(pfn);
> + int len = pfn->buf_len, rc;
> char *path = pfn->pfn_buf;
> - int len = pfn->buf_len;
>
> if (snprintf(path, len, "%s/namespace", pfn->pfn_path) >=
> len) {
> err(ctx, "%s: buffer too small!\n",
> @@ -4516,9 +4528,10 @@ NDCTL_EXPORT int ndctl_pfn_set_namespace(struct
> ndctl_pfn *pfn,
> return -ENXIO;
> }
>
> - if (sysfs_write_attr(ctx, path, ndns
> - ? ndctl_namespace_get_devname(ndns) :
> "\n"))
> - return -ENXIO;
> + rc = sysfs_write_attr(ctx, path, ndns
> + ? ndctl_namespace_get_devname(ndns) :
> "\n");
> + if (rc != 0)
> + return rc;
>
> pfn->ndns = ndns;
> return 0;
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index a9576ce..e4ae31a 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -978,17 +978,8 @@ int cmd_create_namespace(int argc, const char
> **argv, void *ctx)
> }
>
> if (created < 0 || (!namespace && created < 1)) {
> - char *reason = "";
> -
> - if (created == -EAGAIN)
> - reason = ": no suitable capacity found";
> - else if (created == -ENXIO)
> - reason = ": unsupported configuration";
> - else if (created == -EINVAL)
> - reason = ": invalid argument";
> -
> - fprintf(stderr, "failed to %s namespace%s\n",
> namespace
> - ? "reconfigure" : "create", reason);
> + fprintf(stderr, "failed to %s namespace: %s\n",
> namespace
> + ? "reconfigure" : "create",
> strerror(-created));
> if (!namespace)
> created = -ENODEV;
> }
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
prev parent reply other threads:[~2017-07-18 21:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-18 20:30 [ndctl PATCH] ndctl: correctly propagate errors from sysfs_write_attr Vishal Verma
2017-07-18 21:14 ` Verma, Vishal L [this message]
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=1500412378.6401.57.camel@intel.com \
--to=vishal.l.verma@intel.com \
--cc=linux-nvdimm@lists.01.org \
/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.