On 1/6/22 16:04, 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(a)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));
return NULL;
+ }
This overwrites an error reported by virGetConnect().
if (virConfLoadConfig(&conf, "libvirt.conf") < 0)
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;
I'm not a big fan of this label. But, there's an easy trick to avoid it.
So what's happening under 'failed' label is:
virReportError(VIR_ERR_OPERATION_FAILED,
_("Failed to connect to remote libvirt URI %s"),
NULLSTR(uristr));
VIR_FREE(uristr);
virObjectUnref(ret);
and we want to avoid virReportError() but do execute VIR_FREE() and
virObjectUnref(). Well, both these can be replaced with g_auto* and thus
'failed_no_report' would effectively be just plain 'return NULL'.
}
if (virConnectCheckURIMissingSlash(uristr,
@@ -992,7 +996,7 @@ virConnectOpenInternal(const char *name,
virReportError(VIR_ERR_NO_CONNECT,
_("URI scheme '%s' for embedded driver is
not valid"),
ret->uri->scheme);
- goto failed;
+ goto failed_no_report;
}
root = virURIGetParam(ret->uri, "root");
@@ -1002,7 +1006,7 @@ virConnectOpenInternal(const char *name,
if (!g_path_is_absolute(root)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("root path must be absolute"));
- goto failed;
+ goto failed_no_report;
}
if (virEventRequireImpl() < 0)
@@ -1062,7 +1066,7 @@ virConnectOpenInternal(const char *name,
__FILE__, __FUNCTION__, __LINE__,
_("libvirt was built without the '%s'
driver"),
ret->uri->scheme);
- goto failed;
+ goto failed_no_report;
}
VIR_DEBUG("trying driver %zu (%s) ...",
@@ -1112,13 +1116,13 @@ virConnectOpenInternal(const char *name,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Driver %s cannot be used in embedded mode"),
virConnectDriverTab[i]->hypervisorDriver->name);
- goto failed;
+ goto failed_no_report;
}
/* before starting the new connection, check if the driver only works
* with a server, and so return an error if the server is missing */
if (virConnectDriverTab[i]->remoteOnly && ret->uri &&
!ret->uri->server) {
virReportError(VIR_ERR_INVALID_ARG, "%s", _("URI is missing
the server part"));
- goto failed;
+ goto failed_no_report;
}
ret->driver = virConnectDriverTab[i]->hypervisorDriver;
@@ -1155,7 +1159,7 @@ virConnectOpenInternal(const char *name,
if (!ret->driver) {
/* If we reach here, then all drivers declined the connection. */
virReportError(VIR_ERR_NO_CONNECT, "%s", NULLSTR(name));
- goto failed;
+ goto failed_no_report;
}
VIR_FREE(uristr);
@@ -1163,6 +1167,10 @@ virConnectOpenInternal(const char *name,
return ret;
failed:
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Failed to connect to remote libvirt URI %s"),
+ NULLSTR(uristr));
+ failed_no_report:
VIR_FREE(uristr);
virObjectUnref(ret);
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 6d0ab4ee28..bc2b5401da 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -1134,9 +1134,6 @@ libxlDomainMigrationSrcPerformP2P(libxlDriverPrivate *driver,
virObjectLock(vm);
if (dconn == NULL) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Failed to connect to remote libvirt URI %s: %s"),
- dconnuri, virGetLastErrorMessage());
This change and the other hunk below should be a separate commit, IMO.
return ret;
}
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b9d7d582f5..2635ef1162 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5145,9 +5145,6 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver,
goto cleanup;
if (dconn == NULL) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Failed to connect to remote libvirt URI %s: %s"),
- dconnuri, virGetLastErrorMessage());
return -1;
}
Michal