On Tue, Apr 10, 2018 at 5:39 AM, Sage Weil <sweil@xxxxxxxxxx> wrote: > http://tracker.ceph.com/issues/23622 is a test failure because 'ceph > config-key dump' is not spitting out valid JSON. That's because it looks > like this: > > { > "config-history/1/": "`Zd\u0008\u0000\u0000\u0000\u0000", > "config-history/2/": "`Z4w*\u001b\u0000\u0000\u0000\u0000", > "config-history/2/+global/mon_pg_warn_min_per_osd": "3", > "config-history/2/+global/osd_pool_default_min_size": "1", > "config-history/2/+global/osd_pool_default_size": "1", > ... > > Those first two keys probably didn't paste into my message properly, but > they're binary data that aren't valid utf8 or unicode... just random > bytes. I didn't realize this before, but it seems that JSON isn't > actually capable of representing arbitrary binary strings... only > printable utf8/unicode stuff. > > In contrast, config-key will happily store binary data. I think that > currently nothing (in tree) does that except for the new encoded blob for > each config-history change entry (above). But even so, storing binary > data seems useful. But then 'ceph config-key dump' can't actually print > it in JSON. > > Should we > > 1- Print something else there even though it is not strictly correct? > That would mean that something can't parse the (JSON) dump output and > get the right answer. > > 2- Disallow binary data. This seems weak, but at least behavior would be > consistent. We'll probably break some random out-of-tree users out there. > > 3- Use XML as the output format for this command? FWIW XML is not without issues either. Our XmlFormatter doesn't do much about invalid XML element names, so one has to be careful even with valid strings as it can't start with a digit, etc: https://github.com/ceph/ceph/pull/19711#discussion_r160609275 https://github.com/ceph/ceph/pull/19937 Not really on point, but something that had to be fixed in rbd(8). Thanks, Ilya -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
