The current mishmash of XML validation functions is confusing to work with:

• pcmk__validate_xml() is the newest and most preferred, because it can be used with output objects. That means error messages show up in XML output when called by a tool passed --output-as=xml; if one of the other functions is used (e.g. by cibadmin, crm_simulate, profile_file()), errors go to stderr or logs, and the XML output has only success/failure, limiting its usefulness. This currently returns a gboolean, but should be modified to return a standard Pacemaker return code (some new codes may be helpful, and the static functions it calls will likely need to be modified as well).

• validate_xml() is a simple wrapper that can (and should) be trivially replaced when its to_logs option is FALSE (passing a NULL error handler to pcmk__validate_xml(), which sends errors to stderr). If to_logs is TRUE, it uses the static function xml_log(), which sends libxml2/libxslt errors to libqb logs. We could potentially expose xml_log() internally to replace all uses of validate_xml(), but I'm not sure whether that's the best approach. Currently, a NULL handler is mapped to fprintf(stderr, ...) (ultimately by validate_with_relaxng()). Since it's easy enough to pass fprintf as the handler, maybe we should do that where needed and map NULL to xml_log() instead, which then doesn't need to be exposed.

• validate_xml_verbose() is a wrapper that writes the XML to a temporary file and re-reads it before validating. I'm not sure what that adds. We should investigate and either drop it if it's not terribly useful, or ensure that any additional messages can be sent to an output object.

Currently, some callers log errors, and some don't. Ideally, the validation functions should handle all errors, so callers shouldn't.

There is also some confusing handling in validate_with_silent(), which simply forces xml_log() to do nothing. There should be a simpler way to do that -- possibly by using a separate handler that does nothing (instead of using xml_log()).

Also, get_schema_version() currently returns -1 if the schema is unknown or "none", but it does not appear that all callers handle that appropriately. It is also strange that get_schema_version() maps a NULL name to "none", then tries comparing that against known schemas, which will never match it -- it should return immediately. It might be worthwhile to make this a (deprecated) wrapper for a new internal function int pcmk__schema_index(const char *name, int *schema_index) that returns a standard code so we can distinguish the unknown and "none" cases.