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:
signature.asc
Description: PGP signature
-- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel