On Sat, May 05, 2018 at 01:04:20PM +0100, ramyelkest wrote:
> Many places in the code call virGetLastError() just to check the
> raised error code, or domain. However virGetLastError() can return
> NULL, so the code has to check for that first. This patch therefore
> introduces virGetLasErrorCode/Domain function which always returns a
> valid error code/domain respectively, thus dropping the need to
> perform any checks on the error object.
>
> Signed-off-by: Ramy Elkest <ramyelkest@xxxxxxxxx>
> ---
> include/libvirt/virterror.h | 2 ++
> src/libvirt_public.syms | 6 ++++++
> src/util/virerror.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 50 insertions(+)
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 3e7c7a0..5e58b6a 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -344,6 +344,8 @@ void virResetLastError (void);
> void virResetError (virErrorPtr err);
> void virFreeError (virErrorPtr err);
>
> +int virGetLastErrorCode (void);
> +int virGetLastErrorDomain (void);
> const char * virGetLastErrorMessage (void);
>
> virErrorPtr virConnGetLastError (virConnectPtr conn);
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 95df3a0..85b6b6d 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -785,4 +785,10 @@ LIBVIRT_4.1.0 {
> virStoragePoolLookupByTargetPath;
> } LIBVIRT_3.9.0;
>
> +LIBVIRT_4.4.0 {
> + global:
> + virGetLastErrorCode;
> + virGetLastErrorDomain;
> +} LIBVIRT_4.1.0;
> +
> # .... define new API here using predicted next version number ....
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index c000b00..842fc49 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -272,6 +272,48 @@ virGetLastError(void)
>
>
> /**
> + * virGetLastErrorCode:
> + *
> + * Get the most recent error code
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.
A tiny detail I missed during v1, we don't really need to mention ^this bit,
we'd only explicitly document if an API wasn't thread safe, the internal facts
should be transparent to users, it should *just* work, same applies to the hunk
below, I'd suggest squashing in the following so that you don't have to send a
v3:
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 842fc493f1..93632dbdf7 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -274,13 +274,9 @@ virGetLastError(void)
/**
* virGetLastErrorCode:
*
- * Get the most recent error code
+ * Get the most recent error code (enum virErrorNumber).
*
- * The error object is kept in thread local storage, so separate
- * threads can safely access this concurrently.
- *
- * Returns the most recent error code in this thread,
- * or VIR_ERR_OK if none is set
+ * Returns the most recent error code, or VIR_ERR_OK if none is set.
*/
int
virGetLastErrorCode(void)
@@ -295,13 +291,10 @@ virGetLastErrorCode(void)
/**
* virGetLastErrorDomain:
*
- * Get the most recent error domain
+ * Get the most recent error domain (enum virErrorDomain).
*
- * The error object is kept in thread local storage, so separate
- * threads can safely access this concurrently.
- *
- * Returns the most recent error code in this thread,
- * or VIR_FROM_NONE if none is set
+ * Returns a numerical value of the most recent error's origin, or VIR_FROM_NONE
+ * if none is set.
*/
int
virGetLastErrorDomain(void)
Other than that, you have my
Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
