Re: [libvirt] [PATCH 1/3] esx_vi.c: Simplify error handling in MachineByName
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(a)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