[parted-devel] [PATCH] Read NVMe model names from sysfs

[dann frazier]

On Fri, Sep 7, 2018 at 5:38 AM Sebastian Parschauer <sparschauer at suse.de> wrote:
>
> On 05.09.2018 00:19, dann frazier wrote:
> > parted currently shows the same generic model name for all NVMe devices:
> >
> >   # parted /dev/nvme0n1 -s print | grep Model
> >   Model: NVMe Device (nvme)
> >
> > If the model information is available in sysfs, display that instead:
> >
> >   # parted /dev/nvme0n1 -s print | grep Model
> >   Model: THNSN5512GPU7 NVMe TOSHIBA 512GB (nvme)
> > ---
> >  libparted/arch/linux.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> > index 02d7a52c..b3c71edb 100644
> > --- a/libparted/arch/linux.c
> > +++ b/libparted/arch/linux.c
> > @@ -1489,9 +1489,19 @@ linux_new (const char* path)
> >                  break;
> >
> >          case PED_DEVICE_NVME:
> > -                if (!init_generic (dev, _("NVMe Device")))
> > -                        goto error_free_arch_specific;
> > -                break;
> > +                {
> > +                  char* model;
> > +                  model = read_device_sysfs_file (dev, "model");
> > +                  if (!model)
> > +                          model = strdup (_("NVMe Device"));
> > +                  if (!init_generic (dev, model))
> > +                          {
> > +                            free (model);
> > +                            goto error_free_arch_specific;
> > +                          }
> > +                  free (model);
> > +                  break;
> > +                }
> >
> >          case PED_DEVICE_PMEM:
> >                  if (!init_generic (dev, _("NVDIMM Device")))
> >
>
> The idea is good. Thanks. But the style in that file is so inconsistent.
> Often "type* var", sometimes "type *var", how many spaces where, where
> to put '{', ... .

Sebastian,
  Thanks for the review. Yeah, agreed - lots of inconsistencies in this file.

> Would be nicer if this would be put into a separate function init_nvme()
> as linux_new() is quite long already. Looks like the double allocation
> of the model name cannot be easily avoided here. So init_sdmmc() would
> be a similar function.

OK, I'll do that. I also noticed I was ignoring errors from strdup(),
so I'll clean that up as well.

  -dann