src/esx/esx_driver.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
From: Peter Krempa <pkrempa@redhat.com>
In esxConnectListAllDomains if the lookup of the VM name and UUID fails
for a single VM (possible e.g. with broken storage) the whole API would
return failure even when there are working VMs.
Rework the lookup so that if a subset fails we ignore the failure on
those. We report an error only if lookup of all of the objects failed.
Failure is reported from the last one.
Resolves: https://issues.redhat.com/browse/RHEL-80606
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
src/esx/esx_driver.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 554fb3e18f..d869481698 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4792,18 +4792,21 @@ esxConnectListAllDomains(virConnectPtr conn,
virtualMachine = virtualMachine->_next) {
g_autofree char *name = NULL;
- if (needIdentity) {
- if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id,
- &name, uuid) < 0) {
+ /* If the lookup of the required properties fails for some of the machines
+ * in the list it's preferrable to return the valid objects instead of
+ * failing outright */
+ if ((needIdentity && esxVI_GetVirtualMachineIdentity(virtualMachine, &id, &name, uuid) < 0) ||
+ (needPowerState && esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0)) {
+
+ /* Raise error from last lookup if we didn't successfuly fetch any
+ * domain objecst yet */
+ if (count == 0 && !virtualMachine->_next)
goto cleanup;
- }
- }
- if (needPowerState) {
- if (esxVI_GetVirtualMachinePowerState(virtualMachine,
- &powerState) < 0) {
- goto cleanup;
- }
+ /* failure to fetch information of a single VM must not interrupt
+ * the lookup of the rest */
+ virResetLastError();
+ continue;
}
/* filter by active state */
--
2.49.0On a Wednesday in 2025, Peter Krempa via Devel wrote:
>From: Peter Krempa <pkrempa@redhat.com>
>
>In esxConnectListAllDomains if the lookup of the VM name and UUID fails
>for a single VM (possible e.g. with broken storage) the whole API would
>return failure even when there are working VMs.
>
>Rework the lookup so that if a subset fails we ignore the failure on
>those. We report an error only if lookup of all of the objects failed.
>Failure is reported from the last one.
>
>Resolves: https://issues.redhat.com/browse/RHEL-80606
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/esx/esx_driver.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
>diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
>index 554fb3e18f..d869481698 100644
>--- a/src/esx/esx_driver.c
>+++ b/src/esx/esx_driver.c
>@@ -4792,18 +4792,21 @@ esxConnectListAllDomains(virConnectPtr conn,
> virtualMachine = virtualMachine->_next) {
> g_autofree char *name = NULL;
>
>- if (needIdentity) {
>- if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id,
>- &name, uuid) < 0) {
>+ /* If the lookup of the required properties fails for some of the machines
>+ * in the list it's preferrable to return the valid objects instead of
>+ * failing outright */
>+ if ((needIdentity && esxVI_GetVirtualMachineIdentity(virtualMachine, &id, &name, uuid) < 0) ||
>+ (needPowerState && esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0)) {
>+
>+ /* Raise error from last lookup if we didn't successfuly fetch any
>+ * domain objecst yet */
typo.
Also, the comment does not address the fact that we only raise error if
there are no more domains to fetch.
>+ if (count == 0 && !virtualMachine->_next)
> goto cleanup;
>- }
>- }
>
>- if (needPowerState) {
>- if (esxVI_GetVirtualMachinePowerState(virtualMachine,
>- &powerState) < 0) {
>- goto cleanup;
>- }
>+ /* failure to fetch information of a single VM must not interrupt
>+ * the lookup of the rest */
>+ virResetLastError();
>+ continue;
> }
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
On Wed, Mar 26, 2025 at 15:07:37 +0100, Ján Tomko wrote:
> On a Wednesday in 2025, Peter Krempa via Devel wrote:
> > From: Peter Krempa <pkrempa@redhat.com>
> >
> > In esxConnectListAllDomains if the lookup of the VM name and UUID fails
> > for a single VM (possible e.g. with broken storage) the whole API would
> > return failure even when there are working VMs.
> >
> > Rework the lookup so that if a subset fails we ignore the failure on
> > those. We report an error only if lookup of all of the objects failed.
> > Failure is reported from the last one.
> >
> > Resolves: https://issues.redhat.com/browse/RHEL-80606
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > src/esx/esx_driver.c | 23 +++++++++++++----------
> > 1 file changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> > index 554fb3e18f..d869481698 100644
> > --- a/src/esx/esx_driver.c
> > +++ b/src/esx/esx_driver.c
> > @@ -4792,18 +4792,21 @@ esxConnectListAllDomains(virConnectPtr conn,
> > virtualMachine = virtualMachine->_next) {
> > g_autofree char *name = NULL;
> >
> > - if (needIdentity) {
> > - if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id,
> > - &name, uuid) < 0) {
> > + /* If the lookup of the required properties fails for some of the machines
> > + * in the list it's preferrable to return the valid objects instead of
> > + * failing outright */
> > + if ((needIdentity && esxVI_GetVirtualMachineIdentity(virtualMachine, &id, &name, uuid) < 0) ||
> > + (needPowerState && esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0)) {
> > +
> > + /* Raise error from last lookup if we didn't successfuly fetch any
> > + * domain objecst yet */
>
> typo.
>
> Also, the comment does not address the fact that we only raise error if
> there are no more domains to fetch.
Yeah I utterly failed at trying to express that concisely and gave up I
guess. :D
Perhaps:
"Raise error only if we didn't successfuly fill any domain"
?
On Wed, Mar 26, 2025 at 13:39:55 +0100, Peter Krempa via Devel wrote: > From: Peter Krempa <pkrempa@redhat.com> > > In esxConnectListAllDomains if the lookup of the VM name and UUID fails > for a single VM (possible e.g. with broken storage) the whole API would > return failure even when there are working VMs. > > Rework the lookup so that if a subset fails we ignore the failure on > those. We report an error only if lookup of all of the objects failed. > Failure is reported from the last one. > > Resolves: https://issues.redhat.com/browse/RHEL-80606 > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- Forgot to add disclaimer. I didn't test this because I don't have a (broken) ESX setup.
© 2016 - 2025 Red Hat, Inc.