extend illumos compatibility log_sysevent to support more nvlist types

[avg]

Summary

This change adds support for string arrays, nested nvlist-s and nvlist arrays.
To achieve that the nvlist processing code is extracted into a new function,
print_nvlist_sbuf, that can be called recursively.

While there, some whitespace issues are fixed.

[asomers]

Can you give some examples of the output? What happens if a string array contains strings with embedded spaces? Or what happens if an array contains a string with embedded spaces?

[avg]

In D12797#265770, @asomers wrote:

Can you give some examples of the output?

I have some examples, but at the moment I do not have any that demonstrate the new types.

What happens if a string array contains strings with embedded spaces?

Each string in the array will be printed as is, including the space.
So, when the buffer is passed to devctl_notify the result won't be nice.

Or what happens if an array contains a string with embedded spaces?

What kind of an array? Is this question really different from the previous one?
The answer should be approximately the same in any case.

But I should add that exactly the same problem already exists for printing a single string (DATA_TYPE_STRING).

[mav]

sys/cddl/compat/opensolaris/kern/opensolaris_sysevent.c

298

Wouldn't it be better to avoid extra comma at the end? Or it is intentional?

Why elements of string array are separated with comma, while of nvlist array by brackets?

[avg]

Based on the very good points in the feedback I now see that I haven't thought through any of the tough points of supporting those three additional types.
Given that currently they are not passed to the fm compatibility code, it probably makes sense to avoid them in the future rather than struggling to support them.

So, closing this request.

sys/cddl/compat/opensolaris/kern/opensolaris_sysevent.c

298

Both are good points. The trailing coma is not intentional.
I used different separators for nvlists arrays as they can be complex structures, with embedded comas of their own, etc. But I see that I didn't handle spaces embedded into the printed nested nvlists, so the whole thing would not be easily parsable by the devctl handling code anyway.

[avg]

I've just had an accident and got some log messages that demonstrates how the patch formats an event with a nested nvlist:

Nov 11 07:10:21 trant ZFS: checksum mismatch, zpool=pond path=/dev/ada2p3 offset=1465774328832 size=12288

Nov 11 07:10:21 trant zfsd: ZFS: Notify  cksum_actual=00000bfffffff400004805ffffb7fa00204803fedfb7fc00b041ff9e4fbdfd00 cksum_algorithm=fletcher4 cksum_expected=000002e5439e5de70012d763a1055f19531ecf2c77b7466e9826e33b7d364d37 class=ereport.fs.zfs.checksum detector=[ ena=8235824839065079809 parent_guid=13200452317190639285 parent_type=mirror pool=pond pool_context=0 pool_failmode=wait pool_guid=9570370676287618392 scheme=zfs subsystem=ZFS timestamp=1510377021 type=ereport.fs.zfs.checksum vdev=17243312957467959989] vdev_guid=17243312957467959989 vdev_path=/dev/ada3p3 vdev_type=disk version=0 zio_blkid=31 zio_err=122 zio_level=0 zio_object=120 zio_objset=1562 zio_offset=1465774328832 zio_size=12288

The first line is how /etc/devd/zfs.conf reported the event and the second is how zfsd logged the complete devctl message.

So, it's obvious that the patch could be useful, but it still requires some work.
E.g., this is not nice "detector=[ " and neither is this "vdev=17243312957467959989]".
Also, it is impossible to tell the nested elements from the outer elements.

I am thinking that maybe it would be better to get rid of square brackets altogether and to add a prefix to each nested element.
E.g., "detector.ena", "detector.parent_guid", etc.

By the way, the current devd action for the event looks like this:

action "logger -p local7.warn -t ZFS 'checksum mismatch, zpool=$pool path=$vdev_path offset=$zio_offset size=$zio_size'";

It seems that without the patch (in its current format) $pool won't be expanded at all as pool is nested into detector.