[v3,6/6] ndctl: Add supported_alignments to the JSON output
diff mbox series

Message ID 20190116094909.23112-6-oohall@gmail.com
State New
Headers show
Series
  • [v3,1/6] libndctl: Use the supported_alignment attribute
Related show

Commit Message

Oliver Jan. 16, 2019, 9:49 a.m. UTC
Add the list of supported alignemnts to PFN and DAX namespaces. Also add the
list of supported sector sizes to BTT namespaces.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
Not sure the namespace JSON blob are the best place to put these. The
region might be better, but slightly less accessable to users.
---
 util/json.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

Comments

Oliver Jan. 29, 2019, 2:40 p.m. UTC | #1
On Wed, Jan 16, 2019 at 8:49 PM Oliver O'Halloran <oohall@gmail.com> wrote:
>
> Add the list of supported alignemnts to PFN and DAX namespaces. Also add the
> list of supported sector sizes to BTT namespaces.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> Not sure the namespace JSON blob are the best place to put these. The
> region might be better, but slightly less accessable to users.
> ---
>  util/json.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/util/json.c b/util/json.c
> index 77c96fb53c27..6c66033bd312 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -303,6 +303,38 @@ struct json_object *util_daxctl_devs_to_list(struct daxctl_region *region,
>         return jdevs;
>  }
>
> +#define _SZ(get_max, get_elem, type) \
> +static struct json_object *type##_build_size_array(struct type *arg)   \
> +{                                                              \
> +       struct json_object *arr = json_object_new_array();      \
> +       int i;                                                  \
> +                                                               \
> +       if (!arr)                                               \
> +               return NULL;                                    \
> +                                                               \
> +       for (i = 0; i < get_max(arg); i++) {                    \
> +               struct json_object *jobj;                       \
> +               int64_t align;                                  \
> +                                                               \
> +               align = get_elem(arg, i);                       \
> +               jobj = json_object_new_int64(align);            \
> +               if (!jobj)                                      \
> +                       goto err;                               \
> +               json_object_array_add(arr, jobj);               \
> +       }                                                       \
> +                                                               \
> +       return arr;                                             \
> +err:                                                           \
> +       json_object_put(arr);                                   \
> +       return NULL;                                            \
> +}
> +#define SZ(type, kind) _SZ(ndctl_##type##_get_num_##kind##s, \
> +                          ndctl_##type##_get_supported_##kind, ndctl_##type)
> +SZ(pfn, alignment)
> +SZ(dax, alignment)
> +SZ(btt, sector_size)
> +//SZ(namespace, sector_size)
> +
>  struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
>                 const char *ident, unsigned long flags)
>  {
> @@ -739,7 +771,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>  {
>         struct json_object *jndns = json_object_new_object();
>         enum ndctl_pfn_loc loc = NDCTL_PFN_LOC_NONE;
> -       struct json_object *jobj, *jbbs = NULL;
> +       struct json_object *jobj, *jbbs = NULL, *size_array = NULL;
>         const char *locations[] = {
>                 [NDCTL_PFN_LOC_NONE] = "none",
>                 [NDCTL_PFN_LOC_RAM] = "mem",
> @@ -749,6 +781,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>         unsigned int sector_size = UINT_MAX;
>         enum ndctl_namespace_mode mode;
>         const char *bdev = NULL, *name;
> +       const char *size_array_name;
>         unsigned int bb_count = 0;
>         struct ndctl_btt *btt;
>         struct ndctl_pfn *pfn;
> @@ -936,6 +969,19 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
>                         json_object_object_add(jndns, "numa_node", jobj);
>         }
>
> +       if (pfn) {
> +               size_array_name = "supported_alignments";
> +               size_array = ndctl_pfn_build_size_array(pfn);
> +       } else if (dax) {
> +               size_array_name = "supported_alignments";
> +               size_array = ndctl_dax_build_size_array(dax);
> +       } else if (btt) {
> +               size_array_name = "supported sector sizes";
> +               size_array = ndctl_btt_build_size_array(btt);
> +       }
> +       if (size_array)
> +               json_object_object_add(jndns, size_array_name, size_array);

So apparently I forgot to run the `make check` on v3 because this
completely breaks the unit tests.

The problem is that the tests rely on a sed script to parse the JSON
blob into shell variables. That works when the JSON blob only contains
scalars (and strings with no spaces), but it chokes when confronted
with a JSON array. I can't think of any simple ways to fix it other
than using a real JSON parser (jq?). Should I drop this patch for now?
I don't mind doing a real fix if you don't mind waiting a bit longer
:)

Oliver

> +
>         if (pfn)
>                 jbbs = util_pfn_badblocks_to_json(pfn, &bb_count, flags);
>         else if (dax)
> --
> 2.20.1
>
Verma, Vishal L Jan. 29, 2019, 7:28 p.m. UTC | #2
On Wed, 2019-01-30 at 01:40 +1100, Oliver wrote:
> On Wed, Jan 16, 2019 at 8:49 PM Oliver O'Halloran <oohall@gmail.com> wrote:
> > Add the list of supported alignemnts to PFN and DAX namespaces. Also add the
> > list of supported sector sizes to BTT namespaces.
> > 
> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> > ---
> > Not sure the namespace JSON blob are the best place to put these. The
> > region might be better, but slightly less accessable to users.
> > ---
> >  util/json.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 47 insertions(+), 1 deletion(-)
> > 
> > diff --git a/util/json.c b/util/json.c
> > index 77c96fb53c27..6c66033bd312 100644
> > --- a/util/json.c
> > +++ b/util/json.c
> > @@ -303,6 +303,38 @@ struct json_object *util_daxctl_devs_to_list(struct daxctl_region *region,
> >         return jdevs;
> >  }
> > 
> > +#define _SZ(get_max, get_elem, type) \
> > +static struct json_object *type##_build_size_array(struct type *arg)   \
> > +{                                                              \
> > +       struct json_object *arr = json_object_new_array();      \
> > +       int i;                                                  \
> > +                                                               \
> > +       if (!arr)                                               \
> > +               return NULL;                                    \
> > +                                                               \
> > +       for (i = 0; i < get_max(arg); i++) {                    \
> > +               struct json_object *jobj;                       \
> > +               int64_t align;                                  \
> > +                                                               \
> > +               align = get_elem(arg, i);                       \
> > +               jobj = json_object_new_int64(align);            \
> > +               if (!jobj)                                      \
> > +                       goto err;                               \
> > +               json_object_array_add(arr, jobj);               \
> > +       }                                                       \
> > +                                                               \
> > +       return arr;                                             \
> > +err:                                                           \
> > +       json_object_put(arr);                                   \
> > +       return NULL;                                            \
> > +}
> > +#define SZ(type, kind) _SZ(ndctl_##type##_get_num_##kind##s, \
> > +                          ndctl_##type##_get_supported_##kind, ndctl_##type)
> > +SZ(pfn, alignment)
> > +SZ(dax, alignment)
> > +SZ(btt, sector_size)
> > +//SZ(namespace, sector_size)
> > +
> >  struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
> >                 const char *ident, unsigned long flags)
> >  {
> > @@ -739,7 +771,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
> >  {
> >         struct json_object *jndns = json_object_new_object();
> >         enum ndctl_pfn_loc loc = NDCTL_PFN_LOC_NONE;
> > -       struct json_object *jobj, *jbbs = NULL;
> > +       struct json_object *jobj, *jbbs = NULL, *size_array = NULL;
> >         const char *locations[] = {
> >                 [NDCTL_PFN_LOC_NONE] = "none",
> >                 [NDCTL_PFN_LOC_RAM] = "mem",
> > @@ -749,6 +781,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
> >         unsigned int sector_size = UINT_MAX;
> >         enum ndctl_namespace_mode mode;
> >         const char *bdev = NULL, *name;
> > +       const char *size_array_name;
> >         unsigned int bb_count = 0;
> >         struct ndctl_btt *btt;
> >         struct ndctl_pfn *pfn;
> > @@ -936,6 +969,19 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
> >                         json_object_object_add(jndns, "numa_node", jobj);
> >         }
> > 
> > +       if (pfn) {
> > +               size_array_name = "supported_alignments";
> > +               size_array = ndctl_pfn_build_size_array(pfn);
> > +       } else if (dax) {
> > +               size_array_name = "supported_alignments";
> > +               size_array = ndctl_dax_build_size_array(dax);
> > +       } else if (btt) {
> > +               size_array_name = "supported sector sizes";
> > +               size_array = ndctl_btt_build_size_array(btt);
> > +       }
> > +       if (size_array)
> > +               json_object_object_add(jndns, size_array_name, size_array);
> 
> So apparently I forgot to run the `make check` on v3 because this
> completely breaks the unit tests.
> 
> The problem is that the tests rely on a sed script to parse the JSON
> blob into shell variables. That works when the JSON blob only contains
> scalars (and strings with no spaces), but it chokes when confronted
> with a JSON array. I can't think of any simple ways to fix it other
> than using a real JSON parser (jq?). Should I drop this patch for now?
> I don't mind doing a real fix if you don't mind waiting a bit longer
> :)

I think we should just fix the tests to properly parse json - we use jq
in several other places, and if something that didn't use jq breaks the
hackier sed/eval stuff, we should just take the chance to fix that in
the test. If this is a larger piece of work, I'm happy to defer it to
the next release. The other semi-related question might be, should we
always show the supported alignments array, or should hide it under one
or more -v verbosity levels. How often would a user need to know these?
If the array is now shown at the default non-verbose level, then maybe
the parsing problem goes away until a future time :)
Dan Williams Jan. 29, 2019, 7:43 p.m. UTC | #3
On Tue, Jan 29, 2019 at 11:28 AM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
>
> On Wed, 2019-01-30 at 01:40 +1100, Oliver wrote:
> > On Wed, Jan 16, 2019 at 8:49 PM Oliver O'Halloran <oohall@gmail.com> wrote:
> > > Add the list of supported alignemnts to PFN and DAX namespaces. Also add the
> > > list of supported sector sizes to BTT namespaces.
> > >
> > > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> > > ---
> > > Not sure the namespace JSON blob are the best place to put these. The
> > > region might be better, but slightly less accessable to users.
> > > ---
> > >  util/json.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 47 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/util/json.c b/util/json.c
> > > index 77c96fb53c27..6c66033bd312 100644
> > > --- a/util/json.c
> > > +++ b/util/json.c
> > > @@ -303,6 +303,38 @@ struct json_object *util_daxctl_devs_to_list(struct daxctl_region *region,
> > >         return jdevs;
> > >  }
> > >
> > > +#define _SZ(get_max, get_elem, type) \
> > > +static struct json_object *type##_build_size_array(struct type *arg)   \
> > > +{                                                              \
> > > +       struct json_object *arr = json_object_new_array();      \
> > > +       int i;                                                  \
> > > +                                                               \
> > > +       if (!arr)                                               \
> > > +               return NULL;                                    \
> > > +                                                               \
> > > +       for (i = 0; i < get_max(arg); i++) {                    \
> > > +               struct json_object *jobj;                       \
> > > +               int64_t align;                                  \
> > > +                                                               \
> > > +               align = get_elem(arg, i);                       \
> > > +               jobj = json_object_new_int64(align);            \
> > > +               if (!jobj)                                      \
> > > +                       goto err;                               \
> > > +               json_object_array_add(arr, jobj);               \
> > > +       }                                                       \
> > > +                                                               \
> > > +       return arr;                                             \
> > > +err:                                                           \
> > > +       json_object_put(arr);                                   \
> > > +       return NULL;                                            \
> > > +}
> > > +#define SZ(type, kind) _SZ(ndctl_##type##_get_num_##kind##s, \
> > > +                          ndctl_##type##_get_supported_##kind, ndctl_##type)
> > > +SZ(pfn, alignment)
> > > +SZ(dax, alignment)
> > > +SZ(btt, sector_size)
> > > +//SZ(namespace, sector_size)
> > > +
> > >  struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
> > >                 const char *ident, unsigned long flags)
> > >  {
> > > @@ -739,7 +771,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
> > >  {
> > >         struct json_object *jndns = json_object_new_object();
> > >         enum ndctl_pfn_loc loc = NDCTL_PFN_LOC_NONE;
> > > -       struct json_object *jobj, *jbbs = NULL;
> > > +       struct json_object *jobj, *jbbs = NULL, *size_array = NULL;
> > >         const char *locations[] = {
> > >                 [NDCTL_PFN_LOC_NONE] = "none",
> > >                 [NDCTL_PFN_LOC_RAM] = "mem",
> > > @@ -749,6 +781,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
> > >         unsigned int sector_size = UINT_MAX;
> > >         enum ndctl_namespace_mode mode;
> > >         const char *bdev = NULL, *name;
> > > +       const char *size_array_name;
> > >         unsigned int bb_count = 0;
> > >         struct ndctl_btt *btt;
> > >         struct ndctl_pfn *pfn;
> > > @@ -936,6 +969,19 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
> > >                         json_object_object_add(jndns, "numa_node", jobj);
> > >         }
> > >
> > > +       if (pfn) {
> > > +               size_array_name = "supported_alignments";
> > > +               size_array = ndctl_pfn_build_size_array(pfn);
> > > +       } else if (dax) {
> > > +               size_array_name = "supported_alignments";
> > > +               size_array = ndctl_dax_build_size_array(dax);
> > > +       } else if (btt) {
> > > +               size_array_name = "supported sector sizes";
> > > +               size_array = ndctl_btt_build_size_array(btt);
> > > +       }
> > > +       if (size_array)
> > > +               json_object_object_add(jndns, size_array_name, size_array);
> >
> > So apparently I forgot to run the `make check` on v3 because this
> > completely breaks the unit tests.
> >
> > The problem is that the tests rely on a sed script to parse the JSON
> > blob into shell variables. That works when the JSON blob only contains
> > scalars (and strings with no spaces), but it chokes when confronted
> > with a JSON array. I can't think of any simple ways to fix it other
> > than using a real JSON parser (jq?). Should I drop this patch for now?
> > I don't mind doing a real fix if you don't mind waiting a bit longer
> > :)
>
> I think we should just fix the tests to properly parse json - we use jq
> in several other places, and if something that didn't use jq breaks the
> hackier sed/eval stuff, we should just take the chance to fix that in
> the test. If this is a larger piece of work, I'm happy to defer it to
> the next release. The other semi-related question might be, should we
> always show the supported alignments array, or should hide it under one
> or more -v verbosity levels. How often would a user need to know these?
> If the array is now shown at the default non-verbose level, then maybe
> the parsing problem goes away until a future time :)
>

Yeah, if gating this property on UTIL_JSON_VERBOSE fixes the unit test
that seems like the fastest way forward for now.

Patch
diff mbox series

diff --git a/util/json.c b/util/json.c
index 77c96fb53c27..6c66033bd312 100644
--- a/util/json.c
+++ b/util/json.c
@@ -303,6 +303,38 @@  struct json_object *util_daxctl_devs_to_list(struct daxctl_region *region,
 	return jdevs;
 }
 
+#define _SZ(get_max, get_elem, type) \
+static struct json_object *type##_build_size_array(struct type *arg)	\
+{								\
+	struct json_object *arr = json_object_new_array();	\
+	int i;							\
+								\
+	if (!arr)						\
+		return NULL;					\
+								\
+	for (i = 0; i < get_max(arg); i++) {			\
+		struct json_object *jobj;			\
+		int64_t align;					\
+								\
+		align = get_elem(arg, i);			\
+		jobj = json_object_new_int64(align);		\
+		if (!jobj)					\
+			goto err;				\
+		json_object_array_add(arr, jobj);		\
+	}							\
+								\
+	return arr;						\
+err:								\
+	json_object_put(arr);					\
+	return NULL;						\
+}
+#define SZ(type, kind) _SZ(ndctl_##type##_get_num_##kind##s, \
+			   ndctl_##type##_get_supported_##kind, ndctl_##type)
+SZ(pfn, alignment)
+SZ(dax, alignment)
+SZ(btt, sector_size)
+//SZ(namespace, sector_size)
+
 struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
 		const char *ident, unsigned long flags)
 {
@@ -739,7 +771,7 @@  struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 {
 	struct json_object *jndns = json_object_new_object();
 	enum ndctl_pfn_loc loc = NDCTL_PFN_LOC_NONE;
-	struct json_object *jobj, *jbbs = NULL;
+	struct json_object *jobj, *jbbs = NULL, *size_array = NULL;
 	const char *locations[] = {
 		[NDCTL_PFN_LOC_NONE] = "none",
 		[NDCTL_PFN_LOC_RAM] = "mem",
@@ -749,6 +781,7 @@  struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 	unsigned int sector_size = UINT_MAX;
 	enum ndctl_namespace_mode mode;
 	const char *bdev = NULL, *name;
+	const char *size_array_name;
 	unsigned int bb_count = 0;
 	struct ndctl_btt *btt;
 	struct ndctl_pfn *pfn;
@@ -936,6 +969,19 @@  struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 			json_object_object_add(jndns, "numa_node", jobj);
 	}
 
+	if (pfn) {
+		size_array_name = "supported_alignments";
+		size_array = ndctl_pfn_build_size_array(pfn);
+	} else if (dax) {
+		size_array_name = "supported_alignments";
+		size_array = ndctl_dax_build_size_array(dax);
+	} else if (btt) {
+		size_array_name = "supported sector sizes";
+		size_array = ndctl_btt_build_size_array(btt);
+	}
+	if (size_array)
+		json_object_object_add(jndns, size_array_name, size_array);
+
 	if (pfn)
 		jbbs = util_pfn_badblocks_to_json(pfn, &bb_count, flags);
 	else if (dax)