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@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