[PATCH] Make virConnectOpenInternal() report error in all cases

[Peter Krempa]

On Thu, Jan 06, 2022 at 20:34:26 +0530, Ani Sinha wrote:
> virConnectOpenInternal() does not report error in all failure scenarios, except
> in some specific cases. This inconsistent behavior forces the caller of this
> function report a generic error for all failure modes which then hides specific
> error scenarios. This change makes virConnectOpenInternal() report failure in
> all cases so that it can generate specific errors based on the type of failure
> encountered. The reporiting of the errors can be made more fine grained in
> subsequent changes.
> 
> Signed-off-by: Ani Sinha <ani at anisinha.ca>
> ---
>  src/libvirt.c               | 24 ++++++++++++++++--------
>  src/libxl/libxl_migration.c |  3 ---
>  src/qemu/qemu_migration.c   |  3 ---
>  3 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 45315f484c..53ceee1359 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -893,8 +893,12 @@ virConnectOpenInternal(const char *name,
>      bool embed = false;
>  
>      ret = virGetConnect();
> -    if (ret == NULL)
> +    if (ret == NULL) {
> +        virReportError(VIR_ERR_INVALID_CONN,
> +                       _("Failed to create connection object for URI %s"),
> +                       NULLSTR(name));

'virGetConnect' is reporting errors in all cases when NULL is returned.

>          return NULL;
> +    }
>  
>      if (virConfLoadConfig(&conf, "libvirt.conf") < 0)

This seems to be reporting them too.

>          goto failed;
> @@ -974,7 +978,7 @@ virConnectOpenInternal(const char *name,
>              virReportError(VIR_ERR_NO_CONNECT,
>                             _("URI '%s' does not include a driver name"),
>                             name);
> -            goto failed;
> +            goto failed_no_report;
>          }
>  
>          if (virConnectCheckURIMissingSlash(uristr,

e.g this function is also reporting errors and better than the one you
are replacing it with.

Based on at least the two counts above I think you'll need to be more
specific in your claims in the commit message and re-asses all the code
paths.