On 12/28/2013 11:11 AM, Eric Blake wrote:
Having one API call into another is generally not good; among other issues, it gives confusing logs, and is not quite as efficient.
This fixes several instances, but not all: we still have instances in both libvirt.c and in backend hypervisors (lxc and qemu) calling the public virTypedParamsGetString and friends, which dispatch errors immediately. I'm not sure if it is worth trying to clean that up in a separate patch (such a cleanup may be easiest by separating the public function into a wrapper around the internal, then tweaking internal.h so that internal users directly use the internal function).
* src/libvirt.c (virDomainGetUUIDString, virNetworkGetUUIDString) (virStoragePoolGetUUIDString, virSecretGetUUIDString) (virNWFilterGetUUIDString): Avoid nested public API call. * src/util/virtypedparam.c (virTypedParamsReplaceString): Don't dispatch errors here. (virTypedParamsGet): No need to reset errors.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/libvirt.c | 31 +++++-------------------------- src/util/virtypedparam.c | 14 +++++++++----- 2 files changed, 14 insertions(+), 31 deletions(-)
ACK Although I do note that virTypedParamsGetBoolean() has a slightly different call flow than other similar functions... That is usually it's virResetLastError() virTypedParamsGet() VIR_TYPED_PARAM_CHECK_TYPE() but, GetBoolean swaps ResetLast and TypedParamsGet() Since this is a cleanup of sorts - just thought it may be useful to be consistent. John