[libvirt] [PATCH] esx: Fix and improve esxListAllDomains function
Matthias Bolte
matthias.bolte at googlemail.com
Sun Sep 9 10:41:46 UTC 2012
Avoid requesting information such as identity or power state when it
is not necessary.
Lookup virtual machine list with the required fields (configStatus,
name, and config.uuid) to make esxVI_GetVirtualMachineIdentity work.
No need to call esxVI_GetNumberOfSnapshotTrees. rootSnapshotTreeList
can be tested for emptiness by checking it for NULL.
esxVI_LookupRootSnapshotTreeList already does the error reporting,
don't overwrite it.
Check if autostart is enabled at all before looking up the individual
autostart setting of a virtual machine.
Reorder VIR_EXPAND_N(doms, ndoms, 1) to avoid leaking the result of
the call to virGetDomain if VIR_EXPAND_N fails.
If virGetDomain fails it already reports an error, don't overwrite it
with an OOM error.
All items in doms up to the count-th one are valid, no need to double
check before freeing them.
Finally, don't leak autoStartDefaults and powerInfoList.
---
src/esx/esx_driver.c | 116 ++++++++++++++++++++++++++++++++-----------------
1 files changed, 76 insertions(+), 40 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 28e2c65..28f3386 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5010,13 +5010,15 @@ esxListAllDomains(virConnectPtr conn,
{
int ret = -1;
esxPrivate *priv = conn->privateData;
+ bool needIdentity;
+ bool needPowerState;
virDomainPtr dom;
virDomainPtr *doms = NULL;
size_t ndoms = 0;
+ esxVI_String *propertyNameList = NULL;
esxVI_ObjectContent *virtualMachineList = NULL;
esxVI_ObjectContent *virtualMachine = NULL;
- esxVI_String *propertyNameList = NULL;
- esxVI_AutoStartDefaults *autostart_defaults = NULL;
+ esxVI_AutoStartDefaults *autoStartDefaults = NULL;
esxVI_VirtualMachinePowerState powerState;
esxVI_AutoStartPowerInfo *powerInfoList = NULL;
esxVI_AutoStartPowerInfo *powerInfo = NULL;
@@ -5025,7 +5027,6 @@ esxListAllDomains(virConnectPtr conn,
int id;
unsigned char uuid[VIR_UUID_BUFLEN];
int count = 0;
- int snapshotCount;
bool autostart;
int state;
@@ -5035,7 +5036,7 @@ esxListAllDomains(virConnectPtr conn,
* - persistence: all esx machines are persistent
* - managed save: esx doesn't support managed save
*/
- if ((MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) &&
+ if ((MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) &&
!MATCH(VIR_CONNECT_LIST_DOMAINS_PERSISTENT)) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE) &&
!MATCH(VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE))) {
@@ -5047,23 +5048,49 @@ esxListAllDomains(virConnectPtr conn,
goto cleanup;
}
- if (esxVI_EnsureSession(priv->primary) < 0)
+ if (esxVI_EnsureSession(priv->primary) < 0)
return -1;
/* check system default autostart value */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_AUTOSTART)) {
if (esxVI_LookupAutoStartDefaults(priv->primary,
- &autostart_defaults) < 0)
+ &autoStartDefaults) < 0) {
goto cleanup;
+ }
+
+ if (autoStartDefaults->enabled == esxVI_Boolean_True) {
+ if (esxVI_LookupAutoStartPowerInfoList(priv->primary,
+ &powerInfoList) < 0) {
+ goto cleanup;
+ }
+ }
+ }
- if (esxVI_LookupAutoStartPowerInfoList(priv->primary,
- &powerInfoList) < 0)
+ needIdentity = MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT) ||
+ domains != NULL;
+
+ if (needIdentity) {
+ /* Request required data for esxVI_GetVirtualMachineIdentity */
+ if (esxVI_String_AppendValueListToList(&propertyNameList,
+ "configStatus\0"
+ "name\0"
+ "config.uuid\0") < 0) {
goto cleanup;
+ }
}
- if (esxVI_String_AppendValueToList(&propertyNameList,
- "runtime.powerState") < 0 ||
- esxVI_LookupVirtualMachineList(priv->primary, propertyNameList,
+ needPowerState = MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) ||
+ MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE) ||
+ domains != NULL;
+
+ if (needPowerState) {
+ if (esxVI_String_AppendValueToList(&propertyNameList,
+ "runtime.powerState") < 0) {
+ goto cleanup;
+ }
+ }
+
+ if (esxVI_LookupVirtualMachineList(priv->primary, propertyNameList,
&virtualMachineList) < 0)
goto cleanup;
@@ -5075,12 +5102,21 @@ esxListAllDomains(virConnectPtr conn,
for (virtualMachine = virtualMachineList; virtualMachine != NULL;
virtualMachine = virtualMachine->_next) {
+ if (needIdentity) {
+ VIR_FREE(name);
- VIR_FREE(name);
+ if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id,
+ &name, uuid) < 0) {
+ goto cleanup;
+ }
+ }
- if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id, &name, uuid) < 0 ||
- esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0)
- goto cleanup;
+ if (needPowerState) {
+ if (esxVI_GetVirtualMachinePowerState(virtualMachine,
+ &powerState) < 0) {
+ goto cleanup;
+ }
+ }
/* filter by active state */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) &&
@@ -5092,23 +5128,17 @@ esxListAllDomains(virConnectPtr conn,
/* filter by snapshot existence */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT)) {
+ esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotTreeList);
+
if (esxVI_LookupRootSnapshotTreeList(priv->primary, uuid,
&rootSnapshotTreeList) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Couldn't retrieve snapshot list for "
- "domain '%s'"), name);
goto cleanup;
}
- snapshotCount = esxVI_GetNumberOfSnapshotTrees(rootSnapshotTreeList,
- true, false);
-
- esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotTreeList);
-
if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT) &&
- snapshotCount > 0) ||
+ rootSnapshotTreeList != NULL) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT) &&
- snapshotCount == 0)))
+ rootSnapshotTreeList == NULL)))
continue;
}
@@ -5116,19 +5146,18 @@ esxListAllDomains(virConnectPtr conn,
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_AUTOSTART)) {
autostart = false;
- for (powerInfo = powerInfoList; powerInfo != NULL;
- powerInfo = powerInfo->_next) {
- if (STREQ(powerInfo->key->value, virtualMachine->obj->value)) {
- if (STRCASEEQ(powerInfo->startAction, "powerOn"))
- autostart = true;
+ if (autoStartDefaults->enabled == esxVI_Boolean_True) {
+ for (powerInfo = powerInfoList; powerInfo != NULL;
+ powerInfo = powerInfo->_next) {
+ if (STREQ(powerInfo->key->value, virtualMachine->obj->value)) {
+ if (STRCASEEQ(powerInfo->startAction, "powerOn"))
+ autostart = true;
- break;
+ break;
+ }
}
}
- autostart = autostart &&
- autostart_defaults->enabled == esxVI_Boolean_True;
-
if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART) &&
autostart) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART) &&
@@ -5139,6 +5168,7 @@ esxListAllDomains(virConnectPtr conn,
/* filter by domain state */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) {
state = esxVI_VirtualMachinePowerState_ConvertToLibvirt(powerState);
+
if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_RUNNING) &&
state == VIR_DOMAIN_RUNNING) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_PAUSED) &&
@@ -5158,17 +5188,18 @@ esxListAllDomains(virConnectPtr conn,
continue;
}
- if (!(dom = virGetDomain(conn, name, uuid)))
+ if (VIR_EXPAND_N(doms, ndoms, 1) < 0)
goto no_memory;
+ if (!(dom = virGetDomain(conn, name, uuid)))
+ goto cleanup;
+
/* Only running/suspended virtual machines have an ID != -1 */
if (powerState != esxVI_VirtualMachinePowerState_PoweredOff)
dom->id = id;
else
dom->id = -1;
- if (VIR_EXPAND_N(doms, ndoms, 1) < 0)
- goto no_memory;
doms[count++] = dom;
}
@@ -5180,14 +5211,19 @@ esxListAllDomains(virConnectPtr conn,
cleanup:
if (doms) {
for (id = 0; id < count; id++) {
- if (doms[id])
- virDomainFree(doms[id]);
+ virDomainFree(doms[id]);
}
+
+ VIR_FREE(doms);
}
- VIR_FREE(doms);
+
VIR_FREE(name);
+ esxVI_AutoStartDefaults_Free(&autoStartDefaults);
+ esxVI_AutoStartPowerInfo_Free(&powerInfoList);
esxVI_String_Free(&propertyNameList);
esxVI_ObjectContent_Free(&virtualMachineList);
+ esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotTreeList);
+
return ret;
no_memory:
--
1.7.4.1
More information about the libvir-list
mailing list