Change Details

The new function can be used with output objects, allowing errors to be added to XML output. If one of the old function is used, errors will go to stderr or logs only. We should only use the new function internally (setting the output functions where appropriate), and deprecate the old ones.

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.

The new function can be used with output objects,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. allowing errors to be added to XML output.It is also strange that `get_schema_version()` maps a NULL `name` to "none", If one of the old function is usedthen tries comparing that against known schemas, errorswhich will go to stderr or logs onlynever match it -- it should return immediately. We should only usIt might be worthwhile to make the new functionis a (deprecated) wrapper for a new internally (setting the output functions where appropriate)l function `int pcmk__schema_index(const char *name, and deprecateint *schema_index)` that returns a standard code so we can distinguish the old onesunknown and "none" cases.