Re: [PATCH v4 1/1] add display of map information in JSON format

[Gris Ge <fge@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2548 bytes --]

On Fri, May 20, 2016 at 03:59:07PM -0400, Todd Gill wrote:
> 
> Hi Gris,
> 
> On 05/19/2016 09:50 AM, Gris Ge wrote:
> > On Wed, May 18, 2016 at 01:26:34PM -0400, Todd Gill wrote:
> >>
> >> v4:
> >>
> >> removed space in major_version and minor_version keys.
> > Hi Todd,
> > 
> > Few things:
> >     * Please provide path group id from 'pgindex' of struct path.
> 
> The paths are contained inside the groups they belong.  Why do we need
> the 'pgindex'?
> 
The pgindex could be used for switching active path group, so I
would like to see that property show in 'path_group' object.
Yes. The pgindex is the index of 'path_groups' array.
But I prefer it been show explicitly as 'pg_id' in 'path_group'
section.
> > 
> >     * Replace key name 'host adapter' with 'host_adapter'.
> 
> I followed what is displayed via 'multipathd show wildcards'.  You
> are thinking there should not be spaces in keys?  Valid JSON does
> allow spaces in keys.  I thought it was best to be consistent with
> the description of the wild card.
> 
We are defining the API, why should API be consistent with
CLI output which is inconsistent on naming scheme? Now we got:
    total_q_time
    dm-st
    host WWNN

> >     * Remove output properties as many as you can, only expose
> >       those with clear user case and good definition.
> >       For API, it's easy to add but hard to remove or change.
> >       For example: IMHO, we don't need to expose
> >       hcil, next_check, size, serial right now,
> 
> These values are already available via the format specifiers for
> output.  I think we are already committed to them.
> 
Again. Why API sync with CLI output?
For example, each path are having the identical 'size' and 'serial',
why duplicate the information?

IMHO, exposing all properties used by 'multipath -ll' is sufficient
for initial patch. We could always add more properties when needed or
user requested.
> > 
> >     * Performance concern. I am getting bad performance(25 seconds
> >       while previous 'raw format' way only take 1.5 seconds) on 10k
> >       disks. I am still investigating which part slow things down.
> > 
> 
> There are performance problems with systems that have a large number
> of maps (3k+).  But they are not related to the JSON changes.  The
> JSON displays in less time than "show maps topology".
> 
> I think if we want to address the performance/scale issues - we
> should do it with a separate patch set.
OK.
> 
> Thanks,
> Todd

-- 
Gris Ge

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]