[libvirt] [PATCH 1/2] util: adding virHasLastError and virGetLastErrorCode/Domain
Erik Skultety
eskultet at redhat.com
Fri May 4 11:46:37 UTC 2018
On Fri, May 04, 2018 at 04:46:47AM +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 as well.
s/as well/first.
>
> So Instead we create functions virGetLastErrorCode and virGetLastErrorDomain
> (in addition to the existing virGetLastErrorMessage) that always return a
> valid error code/domain/message, to simplify callers.
No paragraph, how about "This patch therefore introduces virGetLasErrorCode
function which always returns a valid error code, thus dropping the need to
perform any checks on the error object".
>
> Also created virHasLastErrorCode for completion
>
> My first commit, for:
> https://wiki.libvirt.org/page/BiteSizedTasks#Add_and_use_virGetLastErrorCode.28.29
>
You mentioned this in the cover letter already, no need to have it in the commit
messages too ;).
> Signed-off-by: Ramy Elkest <ramyelkest at gmail.com>
> ---
> include/libvirt/virterror.h | 3 +++
> src/libvirt_public.syms | 7 +++++
> src/util/virerror.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 73 insertions(+)
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 3e7c7a0..8336258 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -344,6 +344,9 @@ void virResetLastError (void);
> void virResetError (virErrorPtr err);
> void virFreeError (virErrorPtr err);
>
> +int virHasLastError (void);
> +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..3a641a3 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -785,4 +785,11 @@ LIBVIRT_4.1.0 {
> virStoragePoolLookupByTargetPath;
> } LIBVIRT_3.9.0;
>
> +LIBVIRT_4.4.0 {
> + global:
> + virHasLastError;
> + 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..818af2e 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -272,6 +272,69 @@ virGetLastError(void)
>
>
> /**
> + * virHasLastError:
> + *
> + * Check for a reported last error
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.
> + *
> + * Returns 1 if there is an error and the error code is not VIR_ERR_OK,
> + otherwise returns 0
> + */
> +int
> +virHasLastError(void)
> +{
> + virErrorPtr err = virLastErrorObject();
> + if (!err || err->code == VIR_ERR_OK)
> + return 0;
> + return 1;
> +}
As you noted in the cover letter, ^this one is essentially identical to
virGetLastErrorCode, minus the "1 vs err->code" and since VIR_ERR_OK == 0, you
can use virGetLastErrorCode the same way as you used virHasLastError as a
replacement in many occurrences of virGetLastError(), so I don't see the need
for this function.
> +
> +
> +/**
> + * virGetLastErrorCode:
> + *
> + * Get the most recent error code
> + *
> + * 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
> + */
> +int
> +virGetLastErrorCode(void)
> +{
> + virErrorPtr err = virLastErrorObject();
> + if (!err)
> + return VIR_ERR_OK;
> + return err->code;
> +}
> +
> +
> +/**
> + * virGetLastErrorDomain:
> + *
> + * Get the most recent error domain
> + *
> + * 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
> + */
> +int
> +virGetLastErrorDomain(void)
> +{
> + virErrorPtr err = virLastErrorObject();
> + if (!err)
> + return VIR_FROM_NONE;
> + return err->domain;
> +}
I can see a need for ^this function, even though we don't have an immediate use
case for it internally, it's still a public API which someone else might
consume, otherwise they'd have to query the object itself.
Erik
More information about the libvir-list
mailing list