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@xxxxxxxxxxx>
> ---
> 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.
