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

[Gris Ge <fge@redhat.com>]

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

On Wed, May 04, 2016 at 04:23:48PM -0400, Todd Gill wrote:
> The patch adds these commands:
>
> multipathd show maps json
> multipathd show map $map json
>
> Each command will output the requested map(s) in JSON.
>
> For the "show maps json" command, the patch pre-allocates
> INITIAL_REPLY_LEN * 5.  The JSON text is about 5x the size of
> the "show maps topology" text.
Hi Todd,

How about define a constant with this line as comment, instead of
using magic number directly?
>
> Signed-off-by: Todd Gill <tgill@redhat.com>
> ---
> +int
> +snprint_multipath_json (char * buff, int len, struct multipath * mpp, int last)
> +{
> +	int i, fwd = 0;
> +	struct path *pp;
> +
> +	fwd +=  snprint_json(buff + fwd,
> +			len - fwd, 1, PRINT_JSON_START_ELEM);
> +	if (fwd > len)
> +		return len;
> +
> +	fwd += snprint_multipath(buff + fwd, len - fwd,
> +			PRINT_JSON_MAP, mpp, 0);
If we are using snprint_multipath() there, basally this is just a
daemon version of 'show maps raw format <fancy_json_format>'.

So if I may sum up, we have three options to provide a user-friendly
library.

 A) Expand 'show maps raw format' to include all formation where
    user/client could selectively print needed properties out in their
    favored format. And create a wrapper library(check my previous
    post on libdmmp).

        Pros:
            * Minimum changes to daemon.
            * Client only have to parse properties they are
              interested which saves the CPU time and IPC
              communication.

        Cons:
            * We have to document every formatters,
              example: the meanings and possible values of "%N".

 B) Provide 'show maps json' and do the string
    formatting(JSON/XML/etc) at the daemon side.

        Pros:
            * Easy for client to parse IPC output.

        Cons:
            * More IPC communication, like Todd said, about 5 times.
              And client will waste their CPU and memory on parsing
              un-needed properties if only interested on few.
              For example, you have 1000 maps, and just want to know
              which maps has failed path, then you have to parse
              about 108,000 lines of json string.

            * People (we already have) might suggest XML or whatever
              format is better than JSON, why not use that.

 C) Provide both expanded 'show maps raw format' and 'show maps json'
        Pros:
            * All pros above.

        Cons:
            * Do we have to maintain these two ways?

My negligible vote to option B), ideally the wrapper library should be
user's first choice instead messing with 'raw format'.

Thanks for the patch.
Best regards.

-- 
Gris Ge

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

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