Re: [libvirt] [PATCH 3/3] vz: keep subscription to performance events thru domain lifetime

The approach of subscribing on first stat API call and then waiting for receiving of performance event from sdk to process the call originates in times when every vz libvirt connections spawns its own sdk connection. Thus without this waiting virsh stat call would return empty stats. Now with single sdk connection this scheme is unnecessary complicated. This patch subscribes to performance events on first domain appearence and unsubscribe on its removing. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; --- src/vz/vz_driver.c | 28 ++++++++--- src/vz/vz_sdk.c | 139 +++++++++++++++-------------------------------------- src/vz/vz_sdk.h | 8 +-- src/vz/vz_utils.c | 10 +--- src/vz/vz_utils.h | 12 +---- 5 files changed, 67 insertions(+), 130 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index aa10aa0..96221a0 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -613,10 +613,13 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) if (virDomainObjIsActive(dom)) { unsigned long long vtime; + vzDomObjPtr privdom; size_t i; + privdom = dom->privateData; + for (i = 0; i < virDomainDefGetVcpus(dom->def); ++i) { - if (prlsdkGetVcpuStats(dom, i, &vtime) < 0) { + if (prlsdkGetVcpuStats(privdom->stats, i, &vtime) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read cputime for domain")); goto cleanup; @@ -887,11 +890,15 @@ vzDomainGetVcpus(virDomainPtr domain, if (maxinfo >= 1) { if (info != NULL) { + vzDomObjPtr privdom; + memset(info, 0, sizeof(*info) * maxinfo); + privdom = dom->privateData; + for (i = 0; i < maxinfo; i++) { info[i].number = i; info[i].state = VIR_VCPU_RUNNING; - if (prlsdkGetVcpuStats(dom, i, &info[i].cpuTime) < 0) + if (prlsdkGetVcpuStats(privdom->stats, i, &info[i].cpuTime) < 0) goto cleanup; } } @@ -1271,6 +1278,7 @@ vzDomainBlockStats(virDomainPtr domain, const char *path, virDomainBlockStatsPtr stats) { virDomainObjPtr dom = NULL; + vzDomObjPtr privdom; int ret = -1; size_t i; int idx; @@ -1278,12 +1286,14 @@ vzDomainBlockStats(virDomainPtr domain, const char *path, if (!(dom = vzDomObjFromDomainRef(domain))) return -1; + privdom = dom->privateData; + if (*path) { if ((idx = virDomainDiskIndexByName(dom->def, path, false)) < 0) { virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); goto cleanup; } - if (prlsdkGetBlockStats(dom, dom->def->disks[idx], stats) < 0) + if (prlsdkGetBlockStats(privdom->stats, dom->def->disks[idx], stats) < 0) goto cleanup; } else { virDomainBlockStatsStruct s; @@ -1296,7 +1306,7 @@ vzDomainBlockStats(virDomainPtr domain, const char *path, #undef PARALLELS_ZERO_STATS for (i = 0; i < dom->def->ndisks; i++) { - if (prlsdkGetBlockStats(dom, dom->def->disks[i], &s) < 0) + if (prlsdkGetBlockStats(privdom->stats, dom->def->disks[i], &s) < 0) goto cleanup; #define PARALLELS_SUM_STATS(VAR, TYPE, NAME) \ @@ -1374,12 +1384,15 @@ vzDomainInterfaceStats(virDomainPtr domain, virDomainInterfaceStatsPtr stats) { virDomainObjPtr dom = NULL; + vzDomObjPtr privdom; int ret; if (!(dom = vzDomObjFromDomainRef(domain))) return -1; - ret = prlsdkGetNetStats(dom, path, stats); + privdom = dom->privateData; + + ret = prlsdkGetNetStats(privdom->stats, privdom->sdkdom, path, stats); virDomainObjEndAPI(&dom); return ret; @@ -1392,13 +1405,16 @@ vzDomainMemoryStats(virDomainPtr domain, unsigned int flags) { virDomainObjPtr dom = NULL; + vzDomObjPtr privdom; int ret = -1; virCheckFlags(0, -1); if (!(dom = vzDomObjFromDomainRef(domain))) return -1; - ret = prlsdkGetMemoryStats(dom, stats, nr_stats); + privdom = dom->privateData; + + ret = prlsdkGetMemoryStats(privdom->stats, stats, nr_stats); virDomainObjEndAPI(&dom); return ret; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 73bf748..e067574 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -467,8 +467,7 @@ prlsdkDomObjFreePrivate(void *p) return; PrlHandle_Free(pdom->sdkdom); - PrlHandle_Free(pdom->cache.stats); - virCondDestroy(&pdom->cache.cond); + PrlHandle_Free(pdom->stats); VIR_FREE(pdom->home); VIR_FREE(p); }; @@ -1513,6 +1512,7 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom) PRL_UINT32 envId; PRL_VM_AUTOSTART_OPTION autostart; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; + PRL_HANDLE job; virCheckNonNullArgGoto(dom, error); @@ -1604,6 +1604,16 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom) goto error; } + if (pdom->sdkdom == PRL_INVALID_HANDLE) { + job = PrlVm_SubscribeToPerfStats(sdkdom, NULL); + if (PRL_FAILED(waitJob(job))) + 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 */ @@ -1615,11 +1625,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 @@ -1827,6 +1832,8 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver, unsigned char *uuid) { virDomainObjPtr dom = NULL; + vzDomObjPtr privdom; + PRL_HANDLE job; dom = virDomainObjListFindByUUID(driver->domains, uuid); /* domain was removed from the list from the libvirt @@ -1837,53 +1844,32 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver, prlsdkSendEvent(driver, dom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); + privdom = dom->privateData; + job = PrlVm_UnsubscribeFromPerfStats(privdom->sdkdom); + ignore_value(waitJob(job)); +

virDomainObjListRemove(driver->domains, dom); return; } #define PARALLELS_STATISTICS_DROP_COUNT 3 -static PRL_RESULT +static void prlsdkHandlePerfEvent(vzDriverPtr driver, PRL_HANDLE event, unsigned char *uuid) { virDomainObjPtr dom = NULL; vzDomObjPtr privdom = NULL; - PRL_HANDLE job = PRL_INVALID_HANDLE; - dom = virDomainObjListFindByUUID(driver->domains, uuid); - if (dom == NULL) - goto cleanup; + if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid))) + return; + privdom = dom->privateData; + PrlHandle_Free(privdom->stats); + privdom->stats = event; - /* delayed event after unsubscribe */ - if (privdom->cache.count == -1) - goto cleanup; - - PrlHandle_Free(privdom->cache.stats); - privdom->cache.stats = PRL_INVALID_HANDLE; - - 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; - } else { - ++privdom->cache.count; - privdom->cache.stats = event; - /* thus we get own of event handle */ - event = PRL_INVALID_HANDLE; - virCondSignal(&privdom->cache.cond); - } - - cleanup: - PrlHandle_Free(event); - if (dom) - virObjectUnlock(dom); - - return PRL_ERR_SUCCESS; + virObjectUnlock(dom); } static PRL_RESULT @@ -3961,56 +3947,10 @@ prlsdkExtractStatsParam(PRL_HANDLE sdkstats, const char *name, long long *val) #define PARALLELS_STATISTICS_TIMEOUT (60 * 1000) -static int -prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val) -{ - vzDomObjPtr privdom = dom->privateData; - PRL_HANDLE job = PRL_INVALID_HANDLE; - unsigned long long now; - - if (privdom->cache.stats != PRL_INVALID_HANDLE) { - /* reset count to keep subscribtion */ - privdom->cache.count = 0; - return prlsdkExtractStatsParam(privdom->cache.stats, name, val); - } - - if (privdom->cache.count == -1) { - job = PrlVm_SubscribeToPerfStats(privdom->sdkdom, NULL); - if (PRL_FAILED(waitJob(job))) - goto error; - } - - /* change state to subscribed in case of unsubscribed - or reset count so we stop unsubscribe attempts */ - 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, - now + PARALLELS_STATISTICS_TIMEOUT) < 0) { - if (errno == ETIMEDOUT) { - virReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", - _("Timeout on waiting statistics event.")); - goto error; - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to wait on monitor condition")); - goto error; - } - } - } - - return prlsdkExtractStatsParam(privdom->cache.stats, name, val); - error: - return -1; -} - int -prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBlockStatsPtr stats) +prlsdkGetBlockStats(PRL_HANDLE sdkstats, + virDomainDiskDefPtr disk, + virDomainBlockStatsPtr stats) { virDomainDeviceDriveAddressPtr address; int idx; @@ -4042,7 +3982,7 @@ prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBloc #define PRLSDK_GET_STAT_PARAM(VAL, TYPE, NAME) \ if (virAsprintf(&name, "devices.%s%d.%s", prefix, idx, NAME) < 0) \ goto cleanup; \ - if (prlsdkGetStatsParam(dom, name, &stats->VAL) < 0) \ + if (prlsdkExtractStatsParam(sdkstats, name, &stats->VAL) < 0) \ goto cleanup; \ VIR_FREE(name); @@ -4060,20 +4000,19 @@ prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBloc static PRL_HANDLE -prlsdkFindNetByPath(virDomainObjPtr dom, const char *path) +prlsdkFindNetByPath(PRL_HANDLE sdkdom, const char *path) { PRL_UINT32 count = 0; - vzDomObjPtr privdom = dom->privateData; PRL_RESULT pret; size_t i; char *name = NULL; PRL_HANDLE net = PRL_INVALID_HANDLE; - pret = PrlVmCfg_GetNetAdaptersCount(privdom->sdkdom, &count); + pret = PrlVmCfg_GetNetAdaptersCount(sdkdom, &count); prlsdkCheckRetGoto(pret, error); for (i = 0; i < count; ++i) { - pret = PrlVmCfg_GetNetAdapter(privdom->sdkdom, i, &net); + pret = PrlVmCfg_GetNetAdapter(sdkdom, i, &net); prlsdkCheckRetGoto(pret, error); if (!(name = prlsdkGetStringParamVar(PrlVmDevNet_GetHostInterfaceName, @@ -4100,7 +4039,7 @@ prlsdkFindNetByPath(virDomainObjPtr dom, const char *path) } int -prlsdkGetNetStats(virDomainObjPtr dom, const char *path, +prlsdkGetNetStats(PRL_HANDLE sdkstats, PRL_HANDLE sdkdom, const char *path, virDomainInterfaceStatsPtr stats) { int ret = -1; @@ -4109,7 +4048,7 @@ prlsdkGetNetStats(virDomainObjPtr dom, const char *path, PRL_RESULT pret; PRL_HANDLE net = PRL_INVALID_HANDLE; - net = prlsdkFindNetByPath(dom, path); + net = prlsdkFindNetByPath(sdkdom, path); if (net == PRL_INVALID_HANDLE) goto cleanup; @@ -4119,7 +4058,7 @@ prlsdkGetNetStats(virDomainObjPtr dom, const char *path, #define PRLSDK_GET_NET_COUNTER(VAL, NAME) \ if (virAsprintf(&name, "net.nic%d.%s", net_index, NAME) < 0) \ goto cleanup; \ - if (prlsdkGetStatsParam(dom, name, &stats->VAL) < 0) \ + if (prlsdkExtractStatsParam(sdkstats, name, &stats->VAL) < 0) \ goto cleanup; \ VIR_FREE(name); @@ -4143,7 +4082,7 @@ prlsdkGetNetStats(virDomainObjPtr dom, const char *path, } int -prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *vtime) +prlsdkGetVcpuStats(PRL_HANDLE sdkstats, int idx, unsigned long long *vtime) { char *name = NULL; long long ptime = 0; @@ -4151,7 +4090,7 @@ prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *vtime) if (virAsprintf(&name, "guest.vcpu%u.time", (unsigned int)idx) < 0) goto cleanup; - if (prlsdkGetStatsParam(dom, name, &ptime) < 0) + if (prlsdkExtractStatsParam(sdkstats, name, &ptime) < 0) goto cleanup; *vtime = ptime == -1 ? 0 : ptime; ret = 0; @@ -4162,7 +4101,7 @@ prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *vtime) } int -prlsdkGetMemoryStats(virDomainObjPtr dom, +prlsdkGetMemoryStats(PRL_HANDLE sdkstats, virDomainMemoryStatPtr stats, unsigned int nr_stats) { @@ -4171,7 +4110,7 @@ prlsdkGetMemoryStats(virDomainObjPtr dom, size_t i = 0; #define PRLSDK_GET_COUNTER(NAME, VALUE) \ - if (prlsdkGetStatsParam(dom, NAME, &VALUE) < 0) \ + if (prlsdkExtractStatsParam(sdkstats, NAME, &VALUE) < 0) \ goto cleanup; \ #define PRLSDK_MEMORY_STAT_SET(TAG, VALUE) \ diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index f570560..1860c99 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -67,17 +67,17 @@ prlsdkAttachVolume(vzDriverPtr driver, virDomainObjPtr dom, virDomainDiskDefPtr int prlsdkDetachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk); int -prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBlockStatsPtr stats); +prlsdkGetBlockStats(PRL_HANDLE sdkstats, virDomainDiskDefPtr disk, virDomainBlockStatsPtr stats); int prlsdkAttachNet(vzDriverPtr driver, virDomainObjPtr dom, virDomainNetDefPtr net); int prlsdkDetachNet(vzDriverPtr driver, virDomainObjPtr dom, virDomainNetDefPtr net); int -prlsdkGetNetStats(virDomainObjPtr dom, const char *path, virDomainInterfaceStatsPtr stats); +prlsdkGetNetStats(PRL_HANDLE sdkstas, PRL_HANDLE sdkdom, const char *path, virDomainInterfaceStatsPtr stats); int -prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *time); +prlsdkGetVcpuStats(PRL_HANDLE sdkstas, int idx, unsigned long long *time); int -prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats); +prlsdkGetMemoryStats(PRL_HANDLE sdkstas, virDomainMemoryStatPtr stats, unsigned int nr_stats); void prlsdkDomObjFreePrivate(void *p); /* memsize is in MiB */ diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index 5427314..db23b72 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -172,13 +172,7 @@ 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; - + pdom->stats = PRL_INVALID_HANDLE; def->virtType = VIR_DOMAIN_VIRT_VZ; if (!(dom = virDomainObjListAdd(driver->domains, def, @@ -192,8 +186,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 2a99b9f..f5bb40e 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -94,21 +94,11 @@ typedef struct _vzConn vzConn; typedef struct _vzConn *vzConnPtr; -struct _vzCountersCache { - PRL_HANDLE stats; - virCond cond; - /* = -1 - unsubscribed - > -1 - subscribed */ - int count; -}; - -typedef struct _vzCountersCache vzCountersCache; - struct vzDomObj { int id; char *home; PRL_HANDLE sdkdom; - vzCountersCache cache; + PRL_HANDLE stats; }; typedef struct vzDomObj *vzDomObjPtr;