> From: Linux-nvdimm <[email protected]> On Behalf Of Dan
> Williams
> Sent: Wednesday, January 30, 2019 11:08 AM
> To: Verma, Vishal L <[email protected]>
> Cc: [email protected]
> Subject: Re: [PATCH] libndctl, dimm: Don't require an xlat function
> 
> 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 :)

Glad to know this will be fixed soon. 

Thanks,
-- Dexuan
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to