diff mbox

[ndctl,v5,5/5] ndctl: show bad dimm's name by ndctl list command.

Message ID 20170907140854.7F80.E1E9C6FF@jp.fujitsu.com (mailing list archive)
State Accepted
Commit f51b980664b4
Headers show

Commit Message

Gotou, Yasunori/五島 康文 Sept. 7, 2017, 5:08 a.m. UTC
Show dimm's name which has badblock by ndctl list command.
This patch uses translate SPA interface to get bad dimm info.

Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 util/json.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Dan Williams Sept. 7, 2017, 5:55 a.m. UTC | #1
On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> Show dimm's name which has badblock by ndctl list command.
> This patch uses translate SPA interface to get bad dimm info.
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> ---
>  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/util/json.c b/util/json.c
> index 98165b7..639f0ff 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
>         return NULL;
>  }
>
> +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
> +               unsigned long long spa)
> +{
> +       struct ndctl_bus *bus;
> +
> +       bus = ndctl_region_get_bus(region);
> +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
> +}
> +
>  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>                 unsigned int *bb_count, unsigned long flags)
>  {
> @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>
>         ndctl_region_badblock_foreach(region, bb) {
>                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> +                       struct ndctl_dimm *dimm = NULL;
> +                       unsigned long long spa;
> +
> +                       /* get start address of region */
> +                       spa = ndctl_region_get_resource(region);
> +                       if (spa == ULONG_MAX)
> +                               goto err_array;
> +
> +                       /* get address of bad block */
> +                       spa += bb->offset << 9;
> +                       dimm = badblock_to_dimm(region, spa);

Ok the libndctl pieces are looking good, but I have question about
this. We only return the first DIMM that has the error... what if the
length of the record causes us to span more than one dimm?

> +
>                         jbb = json_object_new_object();
>                         if (!jbb)
>                                 goto err_array;
> @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>                                 goto err;
>                         json_object_object_add(jbb, "length", jobj);
>
> +                       if (dimm) {
> +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));

We probably need to make this an array of DIMMs and call badblock() to
DIMM for every 512 byte block in the range.

> +                               if (!jobj)
> +                                       goto err;
> +                               json_object_object_add(jbb, "dimm", jobj);
> +                       }
>                         json_object_array_add(jbbs, jbb);
>                 }
>
> @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>
>         ndctl_region_badblock_foreach(region, bb) {
>                 unsigned long long bb_begin, bb_end, begin, end;
> +               struct ndctl_dimm *dimm = NULL;
>
>                 bb_begin = region_begin + (bb->offset << 9);
>                 bb_end = bb_begin + (bb->len << 9) - 1;
> @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>                 offset = (begin - dev_begin) >> 9;
>                 len = (end - begin + 1) >> 9;
>
> +               dimm = badblock_to_dimm(region, begin);
> +
>                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>                         /* add to json */
>                         jbb = json_object_new_object();
> @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>                                 goto err;
>                         json_object_object_add(jbb, "length", jobj);
>
> +                       if (dimm) {
> +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));

Since this will eventually be the same code as above lets turn it into
a common badblocks_to_jdimms() helper that returns a json array.
Gotou, Yasunori/五島 康文 Sept. 7, 2017, 6:21 a.m. UTC | #2
> On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >
> > Show dimm's name which has badblock by ndctl list command.
> > This patch uses translate SPA interface to get bad dimm info.
> >
> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> > ---
> >  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/util/json.c b/util/json.c
> > index 98165b7..639f0ff 100644
> > --- a/util/json.c
> > +++ b/util/json.c
> > @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
> >         return NULL;
> >  }
> >
> > +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
> > +               unsigned long long spa)
> > +{
> > +       struct ndctl_bus *bus;
> > +
> > +       bus = ndctl_region_get_bus(region);
> > +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
> > +}
> > +
> >  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >                 unsigned int *bb_count, unsigned long flags)
> >  {
> > @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >
> >         ndctl_region_badblock_foreach(region, bb) {
> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> > +                       struct ndctl_dimm *dimm = NULL;
> > +                       unsigned long long spa;
> > +
> > +                       /* get start address of region */
> > +                       spa = ndctl_region_get_resource(region);
> > +                       if (spa == ULONG_MAX)
> > +                               goto err_array;
> > +
> > +                       /* get address of bad block */
> > +                       spa += bb->offset << 9;
> > +                       dimm = badblock_to_dimm(region, spa);
> 
> Ok the libndctl pieces are looking good, but I have question about
> this. We only return the first DIMM that has the error... what if the
> length of the record causes us to span more than one dimm?
> 
> > +
> >                         jbb = json_object_new_object();
> >                         if (!jbb)
> >                                 goto err_array;
> > @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >                                 goto err;
> >                         json_object_object_add(jbb, "length", jobj);
> >
> > +                       if (dimm) {
> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
> 
> We probably need to make this an array of DIMMs and call badblock() to
> DIMM for every 512 byte block in the range.

Hmmmmm, Ok.


> 
> > +                               if (!jobj)
> > +                                       goto err;
> > +                               json_object_object_add(jbb, "dimm", jobj);
> > +                       }
> >                         json_object_array_add(jbbs, jbb);
> >                 }
> >
> > @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >
> >         ndctl_region_badblock_foreach(region, bb) {
> >                 unsigned long long bb_begin, bb_end, begin, end;
> > +               struct ndctl_dimm *dimm = NULL;
> >
> >                 bb_begin = region_begin + (bb->offset << 9);
> >                 bb_end = bb_begin + (bb->len << 9) - 1;
> > @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >                 offset = (begin - dev_begin) >> 9;
> >                 len = (end - begin + 1) >> 9;
> >
> > +               dimm = badblock_to_dimm(region, begin);
> > +
> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> >                         /* add to json */
> >                         jbb = json_object_new_object();
> > @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >                                 goto err;
> >                         json_object_object_add(jbb, "length", jobj);
> >
> > +                       if (dimm) {
> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
> 
> Since this will eventually be the same code as above lets turn it into
> a common badblocks_to_jdimms() helper that returns a json array.
> 

Ok. I'll make next version.

BTW, should I send this No.5 patch for v6?
Otherwise, should I re-send all of this patch set again?
Dan Williams Sept. 7, 2017, 6:25 a.m. UTC | #3
On Wed, Sep 6, 2017 at 11:21 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>> On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>> >
>> > Show dimm's name which has badblock by ndctl list command.
>> > This patch uses translate SPA interface to get bad dimm info.
>> >
>> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
>> > ---
>> >  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 37 insertions(+)
>> >
>> > diff --git a/util/json.c b/util/json.c
>> > index 98165b7..639f0ff 100644
>> > --- a/util/json.c
>> > +++ b/util/json.c
>> > @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
>> >         return NULL;
>> >  }
>> >
>> > +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
>> > +               unsigned long long spa)
>> > +{
>> > +       struct ndctl_bus *bus;
>> > +
>> > +       bus = ndctl_region_get_bus(region);
>> > +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
>> > +}
>> > +
>> >  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>> >                 unsigned int *bb_count, unsigned long flags)
>> >  {
>> > @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>> >
>> >         ndctl_region_badblock_foreach(region, bb) {
>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>> > +                       struct ndctl_dimm *dimm = NULL;
>> > +                       unsigned long long spa;
>> > +
>> > +                       /* get start address of region */
>> > +                       spa = ndctl_region_get_resource(region);
>> > +                       if (spa == ULONG_MAX)
>> > +                               goto err_array;
>> > +
>> > +                       /* get address of bad block */
>> > +                       spa += bb->offset << 9;
>> > +                       dimm = badblock_to_dimm(region, spa);
>>
>> Ok the libndctl pieces are looking good, but I have question about
>> this. We only return the first DIMM that has the error... what if the
>> length of the record causes us to span more than one dimm?
>>
>> > +
>> >                         jbb = json_object_new_object();
>> >                         if (!jbb)
>> >                                 goto err_array;
>> > @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>> >                                 goto err;
>> >                         json_object_object_add(jbb, "length", jobj);
>> >
>> > +                       if (dimm) {
>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
>>
>> We probably need to make this an array of DIMMs and call badblock() to
>> DIMM for every 512 byte block in the range.
>
> Hmmmmm, Ok.
>
>
>>
>> > +                               if (!jobj)
>> > +                                       goto err;
>> > +                               json_object_object_add(jbb, "dimm", jobj);
>> > +                       }
>> >                         json_object_array_add(jbbs, jbb);
>> >                 }
>> >
>> > @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>> >
>> >         ndctl_region_badblock_foreach(region, bb) {
>> >                 unsigned long long bb_begin, bb_end, begin, end;
>> > +               struct ndctl_dimm *dimm = NULL;
>> >
>> >                 bb_begin = region_begin + (bb->offset << 9);
>> >                 bb_end = bb_begin + (bb->len << 9) - 1;
>> > @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>> >                 offset = (begin - dev_begin) >> 9;
>> >                 len = (end - begin + 1) >> 9;
>> >
>> > +               dimm = badblock_to_dimm(region, begin);
>> > +
>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>> >                         /* add to json */
>> >                         jbb = json_object_new_object();
>> > @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>> >                                 goto err;
>> >                         json_object_object_add(jbb, "length", jobj);
>> >
>> > +                       if (dimm) {
>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
>>
>> Since this will eventually be the same code as above lets turn it into
>> a common badblocks_to_jdimms() helper that returns a json array.
>>
>
> Ok. I'll make next version.
>
> BTW, should I send this No.5 patch for v6?
> Otherwise, should I re-send all of this patch set again?

Just this one is fine, the others I'll get applied and pushed out tomorrow.
Dan Williams Sept. 8, 2017, 10:30 p.m. UTC | #4
On Wed, Sep 6, 2017 at 11:25 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Wed, Sep 6, 2017 at 11:21 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>>> On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>>> >
>>> > Show dimm's name which has badblock by ndctl list command.
>>> > This patch uses translate SPA interface to get bad dimm info.
>>> >
>>> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
>>> > ---
>>> >  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
>>> >  1 file changed, 37 insertions(+)
>>> >
>>> > diff --git a/util/json.c b/util/json.c
>>> > index 98165b7..639f0ff 100644
>>> > --- a/util/json.c
>>> > +++ b/util/json.c
>>> > @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
>>> >         return NULL;
>>> >  }
>>> >
>>> > +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
>>> > +               unsigned long long spa)
>>> > +{
>>> > +       struct ndctl_bus *bus;
>>> > +
>>> > +       bus = ndctl_region_get_bus(region);
>>> > +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
>>> > +}
>>> > +
>>> >  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>>> >                 unsigned int *bb_count, unsigned long flags)
>>> >  {
>>> > @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>>> >
>>> >         ndctl_region_badblock_foreach(region, bb) {
>>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>>> > +                       struct ndctl_dimm *dimm = NULL;
>>> > +                       unsigned long long spa;
>>> > +
>>> > +                       /* get start address of region */
>>> > +                       spa = ndctl_region_get_resource(region);
>>> > +                       if (spa == ULONG_MAX)
>>> > +                               goto err_array;
>>> > +
>>> > +                       /* get address of bad block */
>>> > +                       spa += bb->offset << 9;
>>> > +                       dimm = badblock_to_dimm(region, spa);
>>>
>>> Ok the libndctl pieces are looking good, but I have question about
>>> this. We only return the first DIMM that has the error... what if the
>>> length of the record causes us to span more than one dimm?
>>>
>>> > +
>>> >                         jbb = json_object_new_object();
>>> >                         if (!jbb)
>>> >                                 goto err_array;
>>> > @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
>>> >                                 goto err;
>>> >                         json_object_object_add(jbb, "length", jobj);
>>> >
>>> > +                       if (dimm) {
>>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
>>>
>>> We probably need to make this an array of DIMMs and call badblock() to
>>> DIMM for every 512 byte block in the range.
>>
>> Hmmmmm, Ok.
>>
>>
>>>
>>> > +                               if (!jobj)
>>> > +                                       goto err;
>>> > +                               json_object_object_add(jbb, "dimm", jobj);
>>> > +                       }
>>> >                         json_object_array_add(jbbs, jbb);
>>> >                 }
>>> >
>>> > @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>>> >
>>> >         ndctl_region_badblock_foreach(region, bb) {
>>> >                 unsigned long long bb_begin, bb_end, begin, end;
>>> > +               struct ndctl_dimm *dimm = NULL;
>>> >
>>> >                 bb_begin = region_begin + (bb->offset << 9);
>>> >                 bb_end = bb_begin + (bb->len << 9) - 1;
>>> > @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>>> >                 offset = (begin - dev_begin) >> 9;
>>> >                 len = (end - begin + 1) >> 9;
>>> >
>>> > +               dimm = badblock_to_dimm(region, begin);
>>> > +
>>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
>>> >                         /* add to json */
>>> >                         jbb = json_object_new_object();
>>> > @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
>>> >                                 goto err;
>>> >                         json_object_object_add(jbb, "length", jobj);
>>> >
>>> > +                       if (dimm) {
>>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
>>>
>>> Since this will eventually be the same code as above lets turn it into
>>> a common badblocks_to_jdimms() helper that returns a json array.
>>>
>>
>> Ok. I'll make next version.
>>
>> BTW, should I send this No.5 patch for v6?
>> Otherwise, should I re-send all of this patch set again?
>
> Just this one is fine, the others I'll get applied and pushed out tomorrow.

Since this is the last feature I'm waiting for to cut the v58 release,
I'll go ahead and send incremental patches to fix this up.
Gotou, Yasunori/五島 康文 Sept. 11, 2017, 1:05 a.m. UTC | #5
> On Wed, Sep 6, 2017 at 11:25 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> > On Wed, Sep 6, 2017 at 11:21 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >>> On Wed, Sep 6, 2017 at 10:08 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >>> >
> >>> > Show dimm's name which has badblock by ndctl list command.
> >>> > This patch uses translate SPA interface to get bad dimm info.
> >>> >
> >>> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> >>> > ---
> >>> >  util/json.c | 37 +++++++++++++++++++++++++++++++++++++
> >>> >  1 file changed, 37 insertions(+)
> >>> >
> >>> > diff --git a/util/json.c b/util/json.c
> >>> > index 98165b7..639f0ff 100644
> >>> > --- a/util/json.c
> >>> > +++ b/util/json.c
> >>> > @@ -366,6 +366,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
> >>> >         return NULL;
> >>> >  }
> >>> >
> >>> > +static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
> >>> > +               unsigned long long spa)
> >>> > +{
> >>> > +       struct ndctl_bus *bus;
> >>> > +
> >>> > +       bus = ndctl_region_get_bus(region);
> >>> > +       return ndctl_bus_get_dimm_by_physical_address(bus, spa);
> >>> > +}
> >>> > +
> >>> >  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >>> >                 unsigned int *bb_count, unsigned long flags)
> >>> >  {
> >>> > @@ -381,6 +390,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >>> >
> >>> >         ndctl_region_badblock_foreach(region, bb) {
> >>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> >>> > +                       struct ndctl_dimm *dimm = NULL;
> >>> > +                       unsigned long long spa;
> >>> > +
> >>> > +                       /* get start address of region */
> >>> > +                       spa = ndctl_region_get_resource(region);
> >>> > +                       if (spa == ULONG_MAX)
> >>> > +                               goto err_array;
> >>> > +
> >>> > +                       /* get address of bad block */
> >>> > +                       spa += bb->offset << 9;
> >>> > +                       dimm = badblock_to_dimm(region, spa);
> >>>
> >>> Ok the libndctl pieces are looking good, but I have question about
> >>> this. We only return the first DIMM that has the error... what if the
> >>> length of the record causes us to span more than one dimm?
> >>>
> >>> > +
> >>> >                         jbb = json_object_new_object();
> >>> >                         if (!jbb)
> >>> >                                 goto err_array;
> >>> > @@ -395,6 +416,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
> >>> >                                 goto err;
> >>> >                         json_object_object_add(jbb, "length", jobj);
> >>> >
> >>> > +                       if (dimm) {
> >>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
> >>>
> >>> We probably need to make this an array of DIMMs and call badblock() to
> >>> DIMM for every 512 byte block in the range.
> >>
> >> Hmmmmm, Ok.
> >>
> >>
> >>>
> >>> > +                               if (!jobj)
> >>> > +                                       goto err;
> >>> > +                               json_object_object_add(jbb, "dimm", jobj);
> >>> > +                       }
> >>> >                         json_object_array_add(jbbs, jbb);
> >>> >                 }
> >>> >
> >>> > @@ -436,6 +463,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >>> >
> >>> >         ndctl_region_badblock_foreach(region, bb) {
> >>> >                 unsigned long long bb_begin, bb_end, begin, end;
> >>> > +               struct ndctl_dimm *dimm = NULL;
> >>> >
> >>> >                 bb_begin = region_begin + (bb->offset << 9);
> >>> >                 bb_end = bb_begin + (bb->len << 9) - 1;
> >>> > @@ -456,6 +484,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >>> >                 offset = (begin - dev_begin) >> 9;
> >>> >                 len = (end - begin + 1) >> 9;
> >>> >
> >>> > +               dimm = badblock_to_dimm(region, begin);
> >>> > +
> >>> >                 if (flags & UTIL_JSON_MEDIA_ERRORS) {
> >>> >                         /* add to json */
> >>> >                         jbb = json_object_new_object();
> >>> > @@ -472,6 +502,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
> >>> >                                 goto err;
> >>> >                         json_object_object_add(jbb, "length", jobj);
> >>> >
> >>> > +                       if (dimm) {
> >>> > +                               jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
> >>>
> >>> Since this will eventually be the same code as above lets turn it into
> >>> a common badblocks_to_jdimms() helper that returns a json array.
> >>>
> >>
> >> Ok. I'll make next version.
> >>
> >> BTW, should I send this No.5 patch for v6?
> >> Otherwise, should I re-send all of this patch set again?
> >
> > Just this one is fine, the others I'll get applied and pushed out tomorrow.
> 
> Since this is the last feature I'm waiting for to cut the v58 release,
> I'll go ahead and send incremental patches to fix this up.
> 


Wow, thanks!

Since I was off from office at Friday,
This is great help for me.

Anyway, I'll review your patches.
diff mbox

Patch

diff --git a/util/json.c b/util/json.c
index 98165b7..639f0ff 100644
--- a/util/json.c
+++ b/util/json.c
@@ -366,6 +366,15 @@  struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
 	return NULL;
 }
 
+static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
+		unsigned long long spa)
+{
+	struct ndctl_bus *bus;
+
+	bus = ndctl_region_get_bus(region);
+	return ndctl_bus_get_dimm_by_physical_address(bus, spa);
+}
+
 struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 		unsigned int *bb_count, unsigned long flags)
 {
@@ -381,6 +390,18 @@  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 
 	ndctl_region_badblock_foreach(region, bb) {
 		if (flags & UTIL_JSON_MEDIA_ERRORS) {
+			struct ndctl_dimm *dimm = NULL;
+			unsigned long long spa;
+
+			/* get start address of region */
+			spa = ndctl_region_get_resource(region);
+			if (spa == ULONG_MAX)
+				goto err_array;
+
+			/* get address of bad block */
+			spa += bb->offset << 9;
+			dimm = badblock_to_dimm(region, spa);
+
 			jbb = json_object_new_object();
 			if (!jbb)
 				goto err_array;
@@ -395,6 +416,12 @@  struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 				goto err;
 			json_object_object_add(jbb, "length", jobj);
 
+			if (dimm) {
+				jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
+				if (!jobj)
+					goto err;
+				json_object_object_add(jbb, "dimm", jobj);
+			}
 			json_object_array_add(jbbs, jbb);
 		}
 
@@ -436,6 +463,7 @@  static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 
 	ndctl_region_badblock_foreach(region, bb) {
 		unsigned long long bb_begin, bb_end, begin, end;
+		struct ndctl_dimm *dimm = NULL;
 
 		bb_begin = region_begin + (bb->offset << 9);
 		bb_end = bb_begin + (bb->len << 9) - 1;
@@ -456,6 +484,8 @@  static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 		offset = (begin - dev_begin) >> 9;
 		len = (end - begin + 1) >> 9;
 
+		dimm = badblock_to_dimm(region, begin);
+
 		if (flags & UTIL_JSON_MEDIA_ERRORS) {
 			/* add to json */
 			jbb = json_object_new_object();
@@ -472,6 +502,13 @@  static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 				goto err;
 			json_object_object_add(jbb, "length", jobj);
 
+			if (dimm) {
+				jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
+				if (!jobj)
+					goto err;
+				json_object_object_add(jbb, "dimm", jobj);
+			}
+
 			json_object_array_add(jbbs, jbb);
 		}
 		bbs += len;