1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/3] Hyper-V driver fixes
  4. View patch

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

Ladi Prosek posted 3 patches 8 years, 2 months ago
[libvirt] [PATCH v2 1/3] hyperv: Fix hypervInitConnection error reporting
Posted by Ladi Prosek 8 years, 2 months ago 
"%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(-)

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;
         }
     }
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);
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] hyperv: Fix hypervInitConnection error reporting
Posted by John Ferlan 8 years, 2 months ago 

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.

I've removed them locally before pushing...

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);
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] hyperv: Fix hypervInitConnection error reporting
Posted by Ladi Prosek 8 years, 2 months ago 
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);
>>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.