All of lore.kernel.org
 help / color / mirror / Atom feed
From: minwoo.im.dev@gmail.com (Minwoo Im)
Subject: [PATCH V2 3/4] nvme: Return errno mapped for nvme error status
Date: Mon, 20 May 2019 02:53:16 +0900	[thread overview]
Message-ID: <20190519175315.GA10876@minwooim-desktop> (raw)
In-Reply-To: <BYAPR04MB574936E1FDF464243691513286050@BYAPR04MB5749.namprd04.prod.outlook.com>

> > @@ -364,6 +367,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
> >   		perror("get-telemetry-log");
> >   	else if (err > 0) {
> >   		show_nvme_status(err);
> > +		err = nvme_status_to_errno(err, false);
> >   		fprintf(stderr, "Failed to acquire telemetry header %d!\n", err);
> 
> Following line to nvme_status_to_errno() call above assumes that err has 
> a return value from nvme_get_telemetry_log() and we are overwriting it.
> 
> We need to avoid such scenarios going forward. Also, since each command 

Can you please explain why we need to avoid such overwriting scenario?

> assumes that err holds the NVMe status. Instead of having to call 
> nvme_status_errno() along with nvme_show_status() we should call 
> nvme_status_to_errno(err) at the end of the function in the return 
> statement. This approach will not break the assumption that code is 
> having now and less lines of code changes, obviously 
> nvme_status_to_errno() will need some modifications but that
> is fine, untested patch following  :-

Makes sense.  If we are going to convert the positive nvme status and
negative linux internal errno status to an errno inside of
nvme_st4atus_to_errno(), it will much more simpler for this patch.

Anyway, I will prepare V3 patch by updating the conversion at the end of
the each subcommandh handlers.

  reply	other threads:[~2019-05-19 17:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-13 17:03 [PATCH V2 0/4] nvme-cli: Introduce nvme-status mapping with errno Minwoo Im
2019-05-13 17:03 ` [PATCH V2 1/4] nvme.h: Fix typos in status code values Minwoo Im
2019-05-19 16:48   ` Chaitanya Kulkarni
2019-05-13 17:03 ` [PATCH V2 2/4] nvme-status: Introduce nvme status module to map errno Minwoo Im
2019-05-19 17:42   ` Chaitanya Kulkarni
2019-05-19 17:56     ` Minwoo Im
2019-05-13 17:03 ` [PATCH V2 3/4] nvme: Return errno mapped for nvme error status Minwoo Im
2019-05-19 17:41   ` Chaitanya Kulkarni
2019-05-19 17:53     ` Minwoo Im [this message]
2019-05-19 18:00       ` Chaitanya Kulkarni
2019-05-19 19:03         ` Minwoo Im
2019-05-13 17:03 ` [PATCH V2 4/4] fabrics: Return errno mapped for fabrics " Minwoo Im
2019-05-18  2:11 ` [PATCH V2 0/4] nvme-cli: Introduce nvme-status mapping with errno Minwoo Im

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=20190519175315.GA10876@minwooim-desktop \
    --to=minwoo.im.dev@gmail.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.