On Wed, Jan 30, 2019 at 11:05 AM Verma, Vishal L
<[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > Fixes: 62bbfce3cb62 ("libndctl, intel: Add infrastructure for 
> > > firmware_status translation")
> > > Signed-off-by: Oliver O'Halloran <[email protected]>
> > > ---
> > >  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
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to