Re: [libvirt] [PATCH v2 1/3] hyperv: Fix hypervInitConnection error reporting

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(a)redhat.com&gt; > --- > 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.

I've removed them locally before pushing...

Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; 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); >