[libvirt] [PATCH 1/3] esx_vi.c: Simplify error handling in MachineByName

[Michal Prívozník]

On 07/03/2018 04:20 AM, Marcos Paulo de Souza wrote:
> The same pattern is used in lots of other places.
> 
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org at gmail.com>
> ---
>  src/esx/esx_vi.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index 43ff7ea048..25fbdc7e44 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -3014,16 +3014,10 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name,
>          break;
>      }
>  
> -    if (!(*virtualMachine)) {
> -        if (occurrence == esxVI_Occurrence_OptionalItem) {
> -            result = 0;
> -
> -            goto cleanup;
> -        } else {
> -            virReportError(VIR_ERR_NO_DOMAIN,
> -                           _("Could not find domain with name '%s'"), name);
> -            goto cleanup;
> -        }
> +    if (!(*virtualMachine) && occurrence != esxVI_Occurrence_OptionalItem) {
> +        virReportError(VIR_ERR_NO_DOMAIN,
> +                       _("Could not find domain with name '%s'"), name);
> +        goto cleanup;
>      }

I think this error message should be dropped. Firstly, this function is
called from esxDomainDefineXMLFlags() where it is used to check if a
domain with the same name as in provided XML does not exist. If it
doesn't this function reports an error which is undesired.

Secondly, every caller checks for virtualMachine == NULL and reports
their own error effectively overwriting this one.

Thirdly, this function is supposed to act like
virDomainObjListFindByName() which doesn't report error either.


ACK with this squashed in:

diff --git i/src/esx/esx_vi.c w/src/esx/esx_vi.c
index 25fbdc7e44..0bdfc5a8be 100644
--- i/src/esx/esx_vi.c
+++ w/src/esx/esx_vi.c
@@ -3014,11 +3014,8 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name,
         break;
     }
 
-    if (!(*virtualMachine) && occurrence != esxVI_Occurrence_OptionalItem) {
-        virReportError(VIR_ERR_NO_DOMAIN,
-                       _("Could not find domain with name '%s'"), name);
+    if (!(*virtualMachine) && occurrence != esxVI_Occurrence_OptionalItem)
         goto cleanup;
-    }
 
     result = 0;


And pushed.

Michal