* [PATCH] libndctl, dimm: Don't require an xlat function
@ 2019-01-30 10:26 Oliver O'Halloran
2019-01-30 16:38 ` Dan Williams
0 siblings, 1 reply; 5+ messages in thread
From: Oliver O'Halloran @ 2019-01-30 10:26 UTC (permalink / raw)
To: linux-nvdimm
commit 62bbfce3cb62 ("libndctl, intel: Add infrastructure for
firmware_status translation") has the unfortunate side effect of making
all NDCTL commands fail with -ENOMSG unless an xlat_firmware_status
function is defined for the DIMM family. This means that none of the
DIMM label manipulation commands work anymore, unless you happen to be
using an Intel DIMM.
Cc: Vishal Verma <vishal.l.verma@intel.com>
Fixes: 62bbfce3cb62 ("libndctl, intel: Add infrastructure for firmware_status translation")
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
ndctl/lib/libndctl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 06f835d76117..80d107394a74 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2846,6 +2846,9 @@ NDCTL_EXPORT int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd)
* useful), then the xlat function is available separately as well.
*/
xlat_rc = ndctl_cmd_xlat_firmware_status(cmd);
+ if (xlat_rc == -ENOMSG)
+ return rc;
+
return (xlat_rc == 0) ? rc : xlat_rc;
}
--
2.20.1
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] libndctl, dimm: Don't require an xlat function
2019-01-30 10:26 [PATCH] libndctl, dimm: Don't require an xlat function Oliver O'Halloran
@ 2019-01-30 16:38 ` Dan Williams
2019-01-30 19:05 ` Verma, Vishal L
0 siblings, 1 reply; 5+ messages in thread
From: Dan Williams @ 2019-01-30 16:38 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linux-nvdimm
On Wed, Jan 30, 2019 at 2:27 AM Oliver O'Halloran <oohall@gmail.com> wrote:
>
> commit 62bbfce3cb62 ("libndctl, intel: Add infrastructure for
> firmware_status translation") has the unfortunate side effect of making
> all NDCTL commands fail with -ENOMSG unless an xlat_firmware_status
> function is defined for the DIMM family. This means that none of the
> DIMM label manipulation commands work anymore, unless you happen to be
> using an Intel DIMM.
>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Fixes: 62bbfce3cb62 ("libndctl, intel: Add infrastructure for firmware_status translation")
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> ndctl/lib/libndctl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 06f835d76117..80d107394a74 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -2846,6 +2846,9 @@ NDCTL_EXPORT int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd)
> * useful), then the xlat function is available separately as well.
> */
> xlat_rc = ndctl_cmd_xlat_firmware_status(cmd);
> + if (xlat_rc == -ENOMSG)
> + return rc;
> +
Ah, good point, however I think we should do this instead:
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 830b791339d2..2fa89162e05e 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2710,7 +2710,7 @@ NDCTL_EXPORT int
ndctl_cmd_xlat_firmware_status(struct ndctl_cmd *cmd)
struct ndctl_dimm_ops *ops = dimm ? dimm->ops : NULL;
if (!dimm || !ops || !ops->xlat_firmware_status)
- return -ENOMSG;
+ return 0;
return ops->xlat_firmware_status(cmd);
}
A failure to translate just means proceed with the information that we
*do* have, not a failure.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] libndctl, dimm: Don't require an xlat function
2019-01-30 16:38 ` Dan Williams
@ 2019-01-30 19:05 ` Verma, Vishal L
2019-01-30 19:07 ` Dan Williams
0 siblings, 1 reply; 5+ messages in thread
From: Verma, Vishal L @ 2019-01-30 19:05 UTC (permalink / raw)
To: Williams, Dan J, oohall; +Cc: linux-nvdimm
On Wed, 2019-01-30 at 08:38 -0800, Dan Williams wrote:
> On Wed, Jan 30, 2019 at 2:27 AM Oliver O'Halloran <oohall@gmail.com> wrote:
> > commit 62bbfce3cb62 ("libndctl, intel: Add infrastructure for
> > firmware_status translation") has the unfortunate side effect of making
> > all NDCTL commands fail with -ENOMSG unless an xlat_firmware_status
> > function is defined for the DIMM family. This means that none of the
> > DIMM label manipulation commands work anymore, unless you happen to be
> > using an Intel DIMM.
> >
> > Cc: Vishal Verma <vishal.l.verma@intel.com>
> > Fixes: 62bbfce3cb62 ("libndctl, intel: Add infrastructure for firmware_status translation")
> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> > ---
> > ndctl/lib/libndctl.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > index 06f835d76117..80d107394a74 100644
> > --- a/ndctl/lib/libndctl.c
> > +++ b/ndctl/lib/libndctl.c
> > @@ -2846,6 +2846,9 @@ NDCTL_EXPORT int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd)
> > * useful), then the xlat function is available separately as well.
> > */
> > xlat_rc = ndctl_cmd_xlat_firmware_status(cmd);
> > + if (xlat_rc == -ENOMSG)
> > + return rc;
> > +
>
> Ah, good point, however I think we should do this instead:
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 830b791339d2..2fa89162e05e 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -2710,7 +2710,7 @@ NDCTL_EXPORT int
> ndctl_cmd_xlat_firmware_status(struct ndctl_cmd *cmd)
> struct ndctl_dimm_ops *ops = dimm ? dimm->ops : NULL;
>
> if (!dimm || !ops || !ops->xlat_firmware_status)
> - return -ENOMSG;
> + return 0;
> return ops->xlat_firmware_status(cmd);
> }
>
>
> A failure to translate just means proceed with the information that we
> *do* have, not a failure.
Ah good catch and I agree. Dan, will you send a proper patch for this
or shall I?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libndctl, dimm: Don't require an xlat function
2019-01-30 19:05 ` Verma, Vishal L
@ 2019-01-30 19:07 ` Dan Williams
[not found] ` <CAPcyv4jGYVNbbNZ5Kaw7SpwhH=BW8bgDGcj-80m2XmAKD5-ZEg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Dan Williams @ 2019-01-30 19:07 UTC (permalink / raw)
To: Verma, Vishal L; +Cc: linux-nvdimm
On Wed, Jan 30, 2019 at 11:05 AM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
>
> On Wed, 2019-01-30 at 08:38 -0800, Dan Williams wrote:
> > On Wed, Jan 30, 2019 at 2:27 AM Oliver O'Halloran <oohall@gmail.com> wrote:
> > > commit 62bbfce3cb62 ("libndctl, intel: Add infrastructure for
> > > firmware_status translation") has the unfortunate side effect of making
> > > all NDCTL commands fail with -ENOMSG unless an xlat_firmware_status
> > > function is defined for the DIMM family. This means that none of the
> > > DIMM label manipulation commands work anymore, unless you happen to be
> > > using an Intel DIMM.
> > >
> > > Cc: Vishal Verma <vishal.l.verma@intel.com>
> > > Fixes: 62bbfce3cb62 ("libndctl, intel: Add infrastructure for firmware_status translation")
> > > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> > > ---
> > > ndctl/lib/libndctl.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > > index 06f835d76117..80d107394a74 100644
> > > --- a/ndctl/lib/libndctl.c
> > > +++ b/ndctl/lib/libndctl.c
> > > @@ -2846,6 +2846,9 @@ NDCTL_EXPORT int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd)
> > > * useful), then the xlat function is available separately as well.
> > > */
> > > xlat_rc = ndctl_cmd_xlat_firmware_status(cmd);
> > > + if (xlat_rc == -ENOMSG)
> > > + return rc;
> > > +
> >
> > Ah, good point, however I think we should do this instead:
> >
> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > index 830b791339d2..2fa89162e05e 100644
> > --- a/ndctl/lib/libndctl.c
> > +++ b/ndctl/lib/libndctl.c
> > @@ -2710,7 +2710,7 @@ NDCTL_EXPORT int
> > ndctl_cmd_xlat_firmware_status(struct ndctl_cmd *cmd)
> > struct ndctl_dimm_ops *ops = dimm ? dimm->ops : NULL;
> >
> > if (!dimm || !ops || !ops->xlat_firmware_status)
> > - return -ENOMSG;
> > + return 0;
> > return ops->xlat_firmware_status(cmd);
> > }
> >
> >
> > A failure to translate just means proceed with the information that we
> > *do* have, not a failure.
>
> Ah good catch and I agree. Dan, will you send a proper patch for this
> or shall I?
Go for it :)
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] libndctl, dimm: Don't require an xlat function
[not found] ` <CAPcyv4jGYVNbbNZ5Kaw7SpwhH=BW8bgDGcj-80m2XmAKD5-ZEg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2019-01-30 22:54 ` Dexuan Cui
0 siblings, 0 replies; 5+ messages in thread
From: Dexuan Cui @ 2019-01-30 22:54 UTC (permalink / raw)
To: Dan Williams, Verma, Vishal L; +Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw
> From: Linux-nvdimm <linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org> On Behalf Of Dan
> Williams
> Sent: Wednesday, January 30, 2019 11:08 AM
> To: Verma, Vishal L <vishal.l.verma-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org
> Subject: Re: [PATCH] libndctl, dimm: Don't require an xlat function
>
> On Wed, Jan 30, 2019 at 11:05 AM Verma, Vishal L
> <vishal.l.verma-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> >
> >
> > On Wed, 2019-01-30 at 08:38 -0800, Dan Williams wrote:
> > > On Wed, Jan 30, 2019 at 2:27 AM Oliver O'Halloran <oohall-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> wrote:
> > > > commit 62bbfce3cb62 ("libndctl, intel: Add infrastructure for
> > > > firmware_status translation") has the unfortunate side effect of making
> > > > all NDCTL commands fail with -ENOMSG unless an xlat_firmware_status
> > > > function is defined for the DIMM family. This means that none of the
> > > > DIMM label manipulation commands work anymore, unless you happen
> to be
> > > > using an Intel DIMM.
> > > >
> > > > Cc: Vishal Verma <vishal.l.verma-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > Fixes: 62bbfce3cb62 ("libndctl, intel: Add infrastructure for
> firmware_status translation")
> > > > Signed-off-by: Oliver O'Halloran <oohall-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > ---
> > > > ndctl/lib/libndctl.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > > > index 06f835d76117..80d107394a74 100644
> > > > --- a/ndctl/lib/libndctl.c
> > > > +++ b/ndctl/lib/libndctl.c
> > > > @@ -2846,6 +2846,9 @@ NDCTL_EXPORT int
> ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd)
> > > > * useful), then the xlat function is available separately as well.
> > > > */
> > > > xlat_rc = ndctl_cmd_xlat_firmware_status(cmd);
> > > > + if (xlat_rc == -ENOMSG)
> > > > + return rc;
> > > > +
> > >
> > > Ah, good point, however I think we should do this instead:
> > >
> > > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > > index 830b791339d2..2fa89162e05e 100644
> > > --- a/ndctl/lib/libndctl.c
> > > +++ b/ndctl/lib/libndctl.c
> > > @@ -2710,7 +2710,7 @@ NDCTL_EXPORT int
> > > ndctl_cmd_xlat_firmware_status(struct ndctl_cmd *cmd)
> > > struct ndctl_dimm_ops *ops = dimm ? dimm->ops : NULL;
> > >
> > > if (!dimm || !ops || !ops->xlat_firmware_status)
> > > - return -ENOMSG;
> > > + return 0;
> > > return ops->xlat_firmware_status(cmd);
> > > }
> > >
> > >
> > > A failure to translate just means proceed with the information that we
> > > *do* have, not a failure.
> >
> > Ah good catch and I agree. Dan, will you send a proper patch for this
> > or shall I?
>
> Go for it :)
Glad to know this will be fixed soon.
Thanks,
-- Dexuan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-30 22:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30 10:26 [PATCH] libndctl, dimm: Don't require an xlat function Oliver O'Halloran
2019-01-30 16:38 ` Dan Williams
2019-01-30 19:05 ` Verma, Vishal L
2019-01-30 19:07 ` Dan Williams
[not found] ` <CAPcyv4jGYVNbbNZ5Kaw7SpwhH=BW8bgDGcj-80m2XmAKD5-ZEg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2019-01-30 22:54 ` Dexuan Cui
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.