[libvirt] [PATCH 2/3] util: added virGetLastErrorCode/Domain

[Erik Skultety]

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 at gmail.com>
> ---
>  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 at redhat.com>