[libvirt] [PATCH v2] util: set OOM in virCopyLastError if error is not set

[John Ferlan]

On 07/02/2018 07:33 AM, Nikolay Shirokovskiy wrote:
> virCopyLastError is intended to be used after last error is set.
> However due to virLastErrorObject failures (very unlikely through
> as thread local error is allocated on first use) we can have zero
> fields in a copy as a result. In particular code field can be set
> to VIR_ERR_OK.
> 
> In some places (qemu monitor, qemu agent and qemu migaration code
> for example) we use copy result as a flag and this leads to bugs.
> 
> Let's set OOM-like error in copy in case of virLastErrorObject failures.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
> 
> Changes from v1:
> - check @to
> - set OMM error instead of using virErrorGenericFailure
> 
>  src/util/virerror.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index f198f27..737ea92 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -366,19 +366,25 @@ virSetError(virErrorPtr newerr)
>   *
>   * One will need to free the result with virResetError()
>   *
> - * Returns 0 if no error was found and the error code otherwise and -1 in case
> - *         of parameter error.
> + * Returns error code or -1 in case of parameter error.
>   */
>  int
>  virCopyLastError(virErrorPtr to)
>  {
>      virErrorPtr err = virLastErrorObject();
> +
> +    if (!to)
> +        return -1;
> +
>      /* We can't guarantee caller has initialized it to zero */
>      memset(to, 0, sizeof(*to));
> -    if (err)
> +    if (err) {
>          virCopyError(err, to);
> -    else
> -        virResetError(to);
> +    } else {
> +        to->code = VIR_ERR_NO_MEMORY;
> +        to->domain = VIR_FROM_NONE;
> +        to->level = VIR_ERR_ERROR;

Should we do a VIR_FREE(to->message); so that nothing that was there
before somehow remains... I don't think either agent or monitor
"lastError" is reset until Dispose time.

Beyond that, I think this works for this edge/odd/bad case...

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

> +    }
>      return to->code;
>  }
>  
>