[libvirt] [PATCH 3/3] vz: keep subscription to performance events thru domain lifetime
Maxim Nestratov
mnestratov at virtuozzo.com
Sat Jun 11 10:16:27 UTC 2016
03.06.2016 10:11, Nikolay Shirokovskiy пишет:
> 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 at virtuozzo.com>
> ---
> 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(-)
ACK with comment inline. I'm gonna fix it myself, no need to resend.
Maxim
> 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));
> +
This chunk is unnecessary because the domain is already removed.
> 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;
More information about the libvir-list
mailing list