There are 2 motivations to do it:
1. Cleaner error paths on stats cache initialization.
Current checks on error in vzNewDomain in quite cryptic and
rely upon quite subtle order of variable initialization. Adding
lock with existing structure will be a real pain.
2. Postpone stats cache initialization to prlsdkLoadDomain
when we get sdkdom object. Moving stats cache to self locking
requires this object to init the cache.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
---
src/vz/vz_sdk.c | 86 ++++++++++++++++++++++++++++++++++++++++---------------
src/vz/vz_utils.c | 9 ------
src/vz/vz_utils.h | 4 ++-
3 files changed, 66 insertions(+), 33 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 67c68df..5cef432 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -40,6 +40,11 @@
static int
prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid);
+static vzCountersCachePtr
+vzCountersCacheNew(void);
+static void
+vzCountersCacheFree(vzCountersCachePtr cache);
+
VIR_LOG_INIT("parallels.sdk");
/*
@@ -467,8 +472,7 @@ prlsdkDomObjFreePrivate(void *p)
return;
PrlHandle_Free(pdom->sdkdom);
- PrlHandle_Free(pdom->cache.stats);
- virCondDestroy(&pdom->cache.cond);
+ vzCountersCacheFree(pdom->cache);
VIR_FREE(p);
};
@@ -1633,6 +1637,14 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
goto error;
}
+ if (pdom->sdkdom == PRL_INVALID_HANDLE) {
+ if (!(pdom->cache = vzCountersCacheNew()))
+ goto error;
+ pdom->sdkdom = sdkdom;
+ } else {
+ PrlHandle_Free(sdkdom);
+ }
+
/* assign new virDomainDef without any checks
* we can't use virDomainObjAssignDef, because it checks
* for state and domain name */
@@ -1642,11 +1654,6 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
prlsdkConvertDomainState(domainState, envId, dom);
- if (pdom->sdkdom == PRL_INVALID_HANDLE)
- pdom->sdkdom = sdkdom;
- else
- PrlHandle_Free(sdkdom);
-
if (autostart == PAO_VM_START_ON_LOAD)
dom->autostart = 1;
else
@@ -1884,24 +1891,24 @@ prlsdkHandlePerfEvent(vzDriverPtr driver,
privdom = dom->privateData;
/* delayed event after unsubscribe */
- if (privdom->cache.count == -1)
+ if (privdom->cache->count == -1)
goto cleanup;
- PrlHandle_Free(privdom->cache.stats);
- privdom->cache.stats = PRL_INVALID_HANDLE;
+ PrlHandle_Free(privdom->cache->stats);
+ privdom->cache->stats = PRL_INVALID_HANDLE;
- if (privdom->cache.count > PARALLELS_STATISTICS_DROP_COUNT) {
+ if (privdom->cache->count > PARALLELS_STATISTICS_DROP_COUNT) {
job = PrlVm_UnsubscribeFromPerfStats(privdom->sdkdom);
if (PRL_FAILED(waitJob(job)))
goto cleanup;
/* change state to unsubscribed */
- privdom->cache.count = -1;
+ privdom->cache->count = -1;
} else {
- ++privdom->cache.count;
- privdom->cache.stats = event;
+ ++privdom->cache->count;
+ privdom->cache->stats = event;
/* thus we get own of event handle */
event = PRL_INVALID_HANDLE;
- virCondSignal(&privdom->cache.cond);
+ virCondSignal(&privdom->cache->cond);
}
cleanup:
@@ -4033,13 +4040,13 @@ prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val)
PRL_HANDLE job = PRL_INVALID_HANDLE;
unsigned long long now;
- if (privdom->cache.stats != PRL_INVALID_HANDLE) {
+ if (privdom->cache->stats != PRL_INVALID_HANDLE) {
/* reset count to keep subscribtion */
- privdom->cache.count = 0;
- return prlsdkExtractStatsParam(privdom->cache.stats, name, val);
+ privdom->cache->count = 0;
+ return prlsdkExtractStatsParam(privdom->cache->stats, name, val);
}
- if (privdom->cache.count == -1) {
+ if (privdom->cache->count == -1) {
job = PrlVm_SubscribeToPerfStats(privdom->sdkdom, NULL);
if (PRL_FAILED(waitJob(job)))
goto error;
@@ -4047,15 +4054,15 @@ prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val)
/* change state to subscribed in case of unsubscribed
or reset count so we stop unsubscribe attempts */
- privdom->cache.count = 0;
+ privdom->cache->count = 0;
if (virTimeMillisNow(&now) < 0) {
virReportSystemError(errno, "%s", _("Unable to get current time"));
goto error;
}
- while (privdom->cache.stats == PRL_INVALID_HANDLE) {
- if (virCondWaitUntil(&privdom->cache.cond, &dom->parent.lock,
+ while (privdom->cache->stats == PRL_INVALID_HANDLE) {
+ if (virCondWaitUntil(&privdom->cache->cond, &dom->parent.lock,
now + PARALLELS_STATISTICS_TIMEOUT) < 0) {
if (errno == ETIMEDOUT) {
virReportError(VIR_ERR_OPERATION_TIMEOUT, "%s",
@@ -4069,11 +4076,44 @@ prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val)
}
}
- return prlsdkExtractStatsParam(privdom->cache.stats, name, val);
+ return prlsdkExtractStatsParam(privdom->cache->stats, name, val);
error:
return -1;
}
+static void
+vzCountersCacheFree(vzCountersCachePtr cache)
+{
+ if (!cache)
+ return;
+
+ PrlHandle_Free(cache->stats);
+ virCondDestroy(&cache->cond);
+ VIR_FREE(cache);
+}
+
+static vzCountersCachePtr
+vzCountersCacheNew(void)
+{
+ vzCountersCachePtr cache = NULL;
+
+ if (VIR_ALLOC(cache) < 0)
+ return NULL;
+
+ if (virCondInit(&cache->cond) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize condition"));
+ goto error;
+ }
+ cache->stats = PRL_INVALID_HANDLE;
+ cache->count = -1;
+
+ return cache;
+
+ error:
+ VIR_FREE(cache);
+
+ return NULL;
+}
char* prlsdkGetDiskStatName(virDomainDiskDefPtr disk)
{
virDomainDeviceDriveAddressPtr address;
diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
index 5427314..0fcedc6 100644
--- a/src/vz/vz_utils.c
+++ b/src/vz/vz_utils.c
@@ -172,13 +172,6 @@ vzNewDomain(vzDriverPtr driver, const char *name, const unsigned char *uuid)
if (VIR_ALLOC(pdom) < 0)
goto error;
- if (virCondInit(&pdom->cache.cond) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize condition"));
- goto error;
- }
- pdom->cache.stats = PRL_INVALID_HANDLE;
- pdom->cache.count = -1;
-
def->virtType = VIR_DOMAIN_VIRT_VZ;
if (!(dom = virDomainObjListAdd(driver->domains, def,
@@ -192,8 +185,6 @@ vzNewDomain(vzDriverPtr driver, const char *name, const unsigned char *uuid)
return dom;
error:
- if (pdom && pdom->cache.count == -1)
- virCondDestroy(&pdom->cache.cond);
virDomainDefFree(def);
VIR_FREE(pdom);
return NULL;
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index dae34ab..43d46ca 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -103,11 +103,13 @@ struct _vzCountersCache {
};
typedef struct _vzCountersCache vzCountersCache;
+typedef vzCountersCache *vzCountersCachePtr;
struct vzDomObj {
int id;
PRL_HANDLE sdkdom;
- vzCountersCache cache;
+ /* immutable */
+ vzCountersCachePtr cache;
};
typedef struct vzDomObj *vzDomObjPtr;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list