On Mon, Oct 16, 2017 at 3:58 PM, John Ferlan <jferlan@redhat.com> wrote:
On 10/06/2017 02:47 AM, Ladi Prosek wrote:
"%s is not a Hyper-V server" is not a correct generalization of all possible error conditions of hypervEnumAndPull. For example:
$ virsh --connect hyperv://localhost/?transport=http Enter username for localhost [administrator]: Enter administrator's password for localhost: <enters incorrect password> error: failed to connect to the hypervisor error: internal error: localhost is not a Hyper-V server
This commit removes the general virReportError from hypervInitConnection and also the "Invalid query" virReportError from hypervSerializeEprParam, which does not correctly describe the error either (virBufferCheckError has already set a meaningful error message at that point).
The same scenario with the fix:
$ virsh --connect hyperv://localhost/?transport=http Enter username for localhost [administrator]: Enter administrator's password for localhost: <enters incorrect password> error: failed to connect to the hypervisor error: internal error: Transport error during enumeration: User, password or similar was not accepted (26)
Signed-off-by: Ladi Prosek <lprosek@redhat.com> --- src/hyperv/hyperv_driver.c | 2 -- src/hyperv/hyperv_wmi.c | 1 - 2 files changed, 3 deletions(-)
Seems reasonable -
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 09912610c..52274239c 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -105,8 +105,6 @@ hypervInitConnection(virConnectPtr conn, hypervPrivate *priv, priv->wmiVersion = HYPERV_WMI_VERSION_V1;
if (hypervEnumAndPull(priv, &wqlQuery, &computerSystem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("%s is not a Hyper-V server"), conn->uri->server); goto cleanup; }
Before posting be sure to "make check syntax-check" which would have told you that since there's only one line (cleanup;) in the condition that the { } are not necessary.
Will do, sorry about that.
I've removed them locally before pushing...
Thank you!
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
} diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 980a00e28..0b9431bfa 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -514,7 +514,6 @@ hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, /* Get query and create filter based on it */ if (virBufferCheckError(p->epr.query) < 0) { virBufferFreeAndReset(p->epr.query); - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid query")); goto cleanup; } query_string = virBufferContentAndReset(p->epr.query);