10:11 a.m.
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(a)redhat.com>
---
src/libvirt.c | 31 +++++--------------------------
src/util/virtypedparam.c | 14 +++++++++-----
2 files changed, 14 insertions(+), 31 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index cd1112a..587690f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3616,8 +3616,6 @@ error:
int
virDomainGetUUIDString(virDomainPtr domain, char *buf)
{
- unsigned char uuid[VIR_UUID_BUFLEN];
-
VIR_DOMAIN_DEBUG(domain, "buf=%p", buf);
virResetLastError();
@@ -3629,10 +3627,7 @@ virDomainGetUUIDString(virDomainPtr domain, char *buf)
}
virCheckNonNullArgGoto(buf, error);
- if (virDomainGetUUID(domain, &uuid[0]))
- goto error;
-
- virUUIDFormat(uuid, buf);
+ virUUIDFormat(domain->uuid, buf);
return 0;
error:
@@ -12181,7 +12176,6 @@ error:
int
virNetworkGetUUIDString(virNetworkPtr network, char *buf)
{
- unsigned char uuid[VIR_UUID_BUFLEN];
VIR_DEBUG("network=%p, buf=%p", network, buf);
virResetLastError();
@@ -12193,10 +12187,7 @@ virNetworkGetUUIDString(virNetworkPtr network, char *buf)
}
virCheckNonNullArgGoto(buf, error);
- if (virNetworkGetUUID(network, &uuid[0]))
- goto error;
-
- virUUIDFormat(uuid, buf);
+ virUUIDFormat(network->uuid, buf);
return 0;
error:
@@ -14273,7 +14264,6 @@ int
virStoragePoolGetUUIDString(virStoragePoolPtr pool,
char *buf)
{
- unsigned char uuid[VIR_UUID_BUFLEN];
VIR_DEBUG("pool=%p, buf=%p", pool, buf);
virResetLastError();
@@ -14285,10 +14275,7 @@ virStoragePoolGetUUIDString(virStoragePoolPtr pool,
}
virCheckNonNullArgGoto(buf, error);
- if (virStoragePoolGetUUID(pool, &uuid[0]))
- goto error;
-
- virUUIDFormat(uuid, buf);
+ virUUIDFormat(pool->uuid, buf);
return 0;
error:
@@ -16816,7 +16803,6 @@ error:
int
virSecretGetUUIDString(virSecretPtr secret, char *buf)
{
- unsigned char uuid[VIR_UUID_BUFLEN];
VIR_DEBUG("secret=%p, buf=%p", secret, buf);
virResetLastError();
@@ -16828,10 +16814,7 @@ virSecretGetUUIDString(virSecretPtr secret, char *buf)
}
virCheckNonNullArgGoto(buf, error);
- if (virSecretGetUUID(secret, &uuid[0]))
- goto error;
-
- virUUIDFormat(uuid, buf);
+ virUUIDFormat(secret->uuid, buf);
return 0;
error:
@@ -18472,7 +18455,6 @@ error:
int
virNWFilterGetUUIDString(virNWFilterPtr nwfilter, char *buf)
{
- unsigned char uuid[VIR_UUID_BUFLEN];
VIR_DEBUG("nwfilter=%p, buf=%p", nwfilter, buf);
virResetLastError();
@@ -18484,10 +18466,7 @@ virNWFilterGetUUIDString(virNWFilterPtr nwfilter, char *buf)
}
virCheckNonNullArgGoto(buf, error);
- if (virNWFilterGetUUID(nwfilter, &uuid[0]))
- goto error;
-
- virUUIDFormat(uuid, buf);
+ virUUIDFormat(nwfilter->uuid, buf);
return 0;
error:
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index 5334a9f..26fb78c 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -1,7 +1,7 @@
/*
* virtypedparam.c: utility functions for dealing with virTypedParameters
*
- * Copyright (C) 2011-2012 Red Hat, Inc.
+ * Copyright (C) 2011-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -41,6 +41,12 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST,
"boolean",
"string")
+/* When editing this file, ensure that public exported functions
+ * (those in libvirt_public.syms) either trigger no errors, or else
+ * reset error on entrance and call virDispatchError() on exit; while
+ * internal utility functions (those in libvirt_private.syms) may
+ * report errors that the caller will dispatch. */
+
/* Validate that PARAMS contains only recognized parameter names with
* correct types, and with no duplicates. Pass in as many name/type
* pairs as appropriate, and pass NULL to end the list of accepted
@@ -346,8 +352,6 @@ virTypedParamsReplaceString(virTypedParameterPtr *params,
size_t n = *nparams;
virTypedParameterPtr param;
- virResetLastError();
-
param = virTypedParamsGet(*params, n, name);
if (param) {
if (param->type != VIR_TYPED_PARAM_STRING) {
@@ -378,7 +382,6 @@ virTypedParamsReplaceString(virTypedParameterPtr *params,
return 0;
error:
- virDispatchError(NULL);
return -1;
}
@@ -426,6 +429,7 @@ virTypedParamsCopy(virTypedParameterPtr *dst,
* Finds typed parameter called @name.
*
* Returns pointer to the parameter or NULL if it does not exist in @params.
+ * This function does not raise an error, even when returning NULL.
*/
virTypedParameterPtr
virTypedParamsGet(virTypedParameterPtr params,
@@ -434,7 +438,7 @@ virTypedParamsGet(virTypedParameterPtr params,
{
size_t i;
- virResetLastError();
+ /* No need to reset errors, since this function doesn't report any. */
if (!params || !name)
return NULL;
--
1.8.4.2