6:24 a.m.
Current stats cache uses same lock as domain cache, namely lock
of virDomainObjPtr. First this is not necessary, domain cache is
used to translate id of object of interest from libvirt to vzsdk
one. Besides this domain object is not used. Second this is risky
game as lock is dropped on waiting for stat events from sdk deep
down on stack so callers throughout the stack should be written
very carefully.
This patch series makes stats cache self locking and thus making
code much more solid. This is done thru introducing extra lock for
stats cache. Statistics API functions first use domain cache then
drop domain lock, query stats cache and unreference domain object.
Patches 1-4 conform API functions to this structure. Patches 5-7
makes subsidiary improvments. Patch 8 finally introduce new lock.
Last patches are cleanup[9] and bugfix[10].
Nikolay Shirokovskiy (10):
vz: pass string instead of disk definition to block stat function
vz: prepare disks names before getting all disks stats
vz: move getting stats in vzDomainGetVcpus to the end
vz: move getting stats in vzDomainGetInfo to the end
vz: use consistent naming for different domain object in vz_driver.c
vz: simplify refcount on sdkdom in prlsdkLoadDomain
vz: hold stats cache in a pointer in vzDomObj
vz: introduce stats cache lock
vz: extract on stats cache update into a function
vz: fix many stat request issue
src/vz/vz_driver.c | 397 +++++++++++++++++++++++++++++++++++------------------
src/vz/vz_sdk.c | 230 +++++++++++++++++++++----------
src/vz/vz_sdk.h | 15 +-
src/vz/vz_utils.c | 9 --
src/vz/vz_utils.h | 6 +-
5 files changed, 438 insertions(+), 219 deletions(-)
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 01/10] vz: pass string instead of disk definition to block stat function
As a preparation to get statistics without domain lock. Without
this lock we can't use disk definition.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 15 +++++++++++++--
src/vz/vz_sdk.c | 21 ++++++++++++++++-----
src/vz/vz_sdk.h | 3 ++-
3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 177a57a..725076f 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1274,6 +1274,7 @@ vzDomainBlockStats(virDomainPtr domain, const char *path,
int ret = -1;
size_t i;
int idx;
+ char *disk = NULL;
if (!(dom = vzDomObjFromDomainRef(domain)))
return -1;
@@ -1283,7 +1284,11 @@ vzDomainBlockStats(virDomainPtr domain, const char *path,
virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path);
goto cleanup;
}
- if (prlsdkGetBlockStats(dom, dom->def->disks[idx], stats) < 0)
+
+ if (!(disk = prlsdkGetDiskStatName(dom->def->disks[idx])))
+ goto cleanup;
+
+ if (prlsdkGetBlockStats(dom, disk, stats) < 0)
goto cleanup;
} else {
virDomainBlockStatsStruct s;
@@ -1296,7 +1301,10 @@ 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 (!(disk = prlsdkGetDiskStatName(dom->def->disks[i])))
+ goto cleanup;
+
+ if (prlsdkGetBlockStats(dom, disk, &s) < 0)
goto cleanup;
#define PARALLELS_SUM_STATS(VAR, TYPE, NAME) \
@@ -1306,6 +1314,8 @@ vzDomainBlockStats(virDomainPtr domain, const char *path,
PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_SUM_STATS)
#undef PARALLELS_SUM_STATS
+
+ VIR_FREE(disk);
}
}
stats->errs = -1;
@@ -1314,6 +1324,7 @@ vzDomainBlockStats(virDomainPtr domain, const char *path,
cleanup:
if (dom)
virDomainObjEndAPI(&dom);
+ VIR_FREE(disk);
return ret;
}
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 6c05698..b7627e7 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4075,13 +4075,11 @@ prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long
long *val)
return -1;
}
-int
-prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBlockStatsPtr
stats)
+char* prlsdkGetDiskStatName(virDomainDiskDefPtr disk)
{
virDomainDeviceDriveAddressPtr address;
int idx;
const char *prefix;
- int ret = -1;
char *name = NULL;
address = &disk->info.addr.drive;
@@ -4101,12 +4099,25 @@ prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk,
virDomainBloc
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown disk bus: %X"), disk->bus);
- goto cleanup;
+ return NULL;
}
+ if (virAsprintf(&name, "%s%d", prefix, idx) < 0)
+ return NULL;
+
+ return name;
+}
+
+int
+prlsdkGetBlockStats(virDomainObjPtr dom,
+ const char *disk,
+ virDomainBlockStatsPtr stats)
+{
+ int ret = -1;
+ char *name = NULL;
#define PRLSDK_GET_STAT_PARAM(VAL, TYPE, NAME) \
- if (virAsprintf(&name, "devices.%s%d.%s", prefix, idx, NAME) < 0)
\
+ if (virAsprintf(&name, "devices.%s.%s", disk, NAME) < 0)
\
goto cleanup; \
if (prlsdkGetStatsParam(dom, name, &stats->VAL) < 0) \
goto cleanup; \
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index f570560..033a3fe 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -66,8 +66,9 @@ int
prlsdkAttachVolume(vzDriverPtr driver, virDomainObjPtr dom, virDomainDiskDefPtr disk);
int
prlsdkDetachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk);
+char* prlsdkGetDiskStatName(virDomainDiskDefPtr disk);
int
-prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBlockStatsPtr
stats);
+prlsdkGetBlockStats(virDomainObjPtr dom, const char *disk, virDomainBlockStatsPtr
stats);
int
prlsdkAttachNet(vzDriverPtr driver, virDomainObjPtr dom, virDomainNetDefPtr net);
int
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 02/10] vz: prepare disks names before getting all disks stats
As a preparation to get statistics without domain lock. Resolving
libvirt names to vzsdk ones is not possible without domain lock
thus we need to do it beforehand.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 725076f..0609594 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1275,6 +1275,8 @@ vzDomainBlockStats(virDomainPtr domain, const char *path,
size_t i;
int idx;
char *disk = NULL;
+ char **disks = NULL;
+ int ndisks = 0;
if (!(dom = vzDomObjFromDomainRef(domain)))
return -1;
@@ -1300,10 +1302,16 @@ vzDomainBlockStats(virDomainPtr domain, const char *path,
#undef PARALLELS_ZERO_STATS
- for (i = 0; i < dom->def->ndisks; i++) {
- if (!(disk = prlsdkGetDiskStatName(dom->def->disks[i])))
+ ndisks = dom->def->ndisks;
+
+ if (VIR_ALLOC_N(disks, ndisks))
+ goto cleanup;
+
+ for (i = 0; i < ndisks; i++)
+ if (!(disks[i] = prlsdkGetDiskStatName(dom->def->disks[i])))
goto cleanup;
+ for (i = 0; i < ndisks; i++) {
if (prlsdkGetBlockStats(dom, disk, &s) < 0)
goto cleanup;
@@ -1314,8 +1322,6 @@ vzDomainBlockStats(virDomainPtr domain, const char *path,
PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_SUM_STATS)
#undef PARALLELS_SUM_STATS
-
- VIR_FREE(disk);
}
}
stats->errs = -1;
@@ -1325,6 +1331,7 @@ vzDomainBlockStats(virDomainPtr domain, const char *path,
if (dom)
virDomainObjEndAPI(&dom);
VIR_FREE(disk);
+ virStringFreeListCount(disks, ndisks);
return ret;
}
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 03/10] vz: move getting stats in vzDomainGetVcpus to the end
As a preparation to get statistics without domain lock. Let's
take all we want from domain cache first. After this point
domain lock can be dropped.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 0609594..62d5047 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -885,24 +885,29 @@ vzDomainGetVcpus(virDomainPtr domain,
goto cleanup;
}
- if (maxinfo >= 1) {
- if (info != NULL) {
- memset(info, 0, sizeof(*info) * maxinfo);
- for (i = 0; i < maxinfo; i++) {
- info[i].number = i;
- info[i].state = VIR_VCPU_RUNNING;
- if (prlsdkGetVcpuStats(privdom, i, &info[i].cpuTime) < 0)
- goto cleanup;
- }
- }
- if (cpumaps != NULL) {
- memset(cpumaps, 0, maplen * maxinfo);
- for (i = 0; i < maxinfo; i++)
- virBitmapToDataBuf(privdom->def->cpumask,
- VIR_GET_CPUMAP(cpumaps, maplen, i),
- maplen);
+ if (maxinfo < 1) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (cpumaps != NULL) {
+ memset(cpumaps, 0, maplen * maxinfo);
+ for (i = 0; i < maxinfo; i++)
+ virBitmapToDataBuf(privdom->def->cpumask,
+ VIR_GET_CPUMAP(cpumaps, maplen, i),
+ maplen);
+ }
+
+ if (info != NULL) {
+ memset(info, 0, sizeof(*info) * maxinfo);
+ for (i = 0; i < maxinfo; i++) {
+ info[i].number = i;
+ info[i].state = VIR_VCPU_RUNNING;
+ if (prlsdkGetVcpuStats(privdom, i, &info[i].cpuTime) < 0)
+ goto cleanup;
}
}
+
ret = maxinfo;
cleanup:
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 04/10] vz: move getting stats in vzDomainGetInfo to the end
As a preparation to get statistics without domain lock. Let's
take all we want from domain cache first. After this point
domain lock can be dropped.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 62d5047..810017c 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -601,6 +601,8 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
{
virDomainObjPtr privdom;
int ret = -1;
+ size_t i;
+ unsigned int nvcpus;
if (!(privdom = vzDomObjFromDomainRef(domain)))
goto cleanup;
@@ -611,18 +613,22 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
info->nrVirtCpu = virDomainDefGetVcpus(privdom->def);
info->cpuTime = 0;
- if (virDomainObjIsActive(privdom)) {
+ if (!virDomainObjIsActive(privdom)) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ nvcpus = virDomainDefGetVcpus(privdom->def);
+
+ for (i = 0; i < nvcpus; ++i) {
unsigned long long vtime;
- size_t i;
- for (i = 0; i < virDomainDefGetVcpus(privdom->def); ++i) {
- if (prlsdkGetVcpuStats(privdom, i, &vtime) < 0) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("cannot read cputime for domain"));
- goto cleanup;
- }
- info->cpuTime += vtime;
+ if (prlsdkGetVcpuStats(privdom, i, &vtime) < 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("cannot read cputime for domain"));
+ goto cleanup;
}
+ info->cpuTime += vtime;
}
ret = 0;
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 05/10] vz: use consistent naming for different domain object in vz_driver.c
Naming scheme is next:
virDomainPtr domain;
virDomainObjPtr dom;
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 158 ++++++++++++++++++++++++++---------------------------
1 file changed, 79 insertions(+), 79 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 810017c..95739bd 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -599,31 +599,31 @@ vzDomainLookupByName(virConnectPtr conn, const char *name)
static int
vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
{
- virDomainObjPtr privdom;
+ virDomainObjPtr dom;
int ret = -1;
size_t i;
unsigned int nvcpus;
- if (!(privdom = vzDomObjFromDomainRef(domain)))
+ if (!(dom = vzDomObjFromDomainRef(domain)))
goto cleanup;
- info->state = virDomainObjGetState(privdom, NULL);
- info->memory = privdom->def->mem.cur_balloon;
- info->maxMem = virDomainDefGetMemoryActual(privdom->def);
- info->nrVirtCpu = virDomainDefGetVcpus(privdom->def);
+ info->state = virDomainObjGetState(dom, NULL);
+ info->memory = dom->def->mem.cur_balloon;
+ info->maxMem = virDomainDefGetMemoryActual(dom->def);
+ info->nrVirtCpu = virDomainDefGetVcpus(dom->def);
info->cpuTime = 0;
- if (!virDomainObjIsActive(privdom)) {
+ if (!virDomainObjIsActive(dom)) {
ret = 0;
goto cleanup;
}
- nvcpus = virDomainDefGetVcpus(privdom->def);
+ nvcpus = virDomainDefGetVcpus(dom->def);
for (i = 0; i < nvcpus; ++i) {
unsigned long long vtime;
- if (prlsdkGetVcpuStats(privdom, i, &vtime) < 0) {
+ if (prlsdkGetVcpuStats(dom, i, &vtime) < 0) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("cannot read cputime for domain"));
goto cleanup;
@@ -633,42 +633,42 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
ret = 0;
cleanup:
- virDomainObjEndAPI(&privdom);
+ virDomainObjEndAPI(&dom);
return ret;
}
static char *
vzDomainGetOSType(virDomainPtr domain)
{
- virDomainObjPtr privdom;
+ virDomainObjPtr dom;
char *ret = NULL;
- if (!(privdom = vzDomObjFromDomain(domain)))
+ if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
- ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(privdom->def->os.type)));
+ ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(dom->def->os.type)));
cleanup:
- if (privdom)
- virObjectUnlock(privdom);
+ if (dom)
+ virObjectUnlock(dom);
return ret;
}
static int
vzDomainIsPersistent(virDomainPtr domain)
{
- virDomainObjPtr privdom;
+ virDomainObjPtr dom;
int ret = -1;
- if (!(privdom = vzDomObjFromDomain(domain)))
+ if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
ret = 1;
cleanup:
- if (privdom)
- virObjectUnlock(privdom);
+ if (dom)
+ virObjectUnlock(dom);
return ret;
}
@@ -676,19 +676,19 @@ static int
vzDomainGetState(virDomainPtr domain,
int *state, int *reason, unsigned int flags)
{
- virDomainObjPtr privdom;
+ virDomainObjPtr dom;
int ret = -1;
virCheckFlags(0, -1);
- if (!(privdom = vzDomObjFromDomain(domain)))
+ if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
- *state = virDomainObjGetState(privdom, reason);
+ *state = virDomainObjGetState(dom, reason);
ret = 0;
cleanup:
- if (privdom)
- virObjectUnlock(privdom);
+ if (dom)
+ virObjectUnlock(dom);
return ret;
}
@@ -697,40 +697,40 @@ vzDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
{
vzConnPtr privconn = domain->conn->privateData;
virDomainDefPtr def;
- virDomainObjPtr privdom;
+ virDomainObjPtr dom;
char *ret = NULL;
/* Flags checked by virDomainDefFormat */
- if (!(privdom = vzDomObjFromDomain(domain)))
+ if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
def = (flags & VIR_DOMAIN_XML_INACTIVE) &&
- privdom->newDef ? privdom->newDef : privdom->def;
+ dom->newDef ? dom->newDef : dom->def;
ret = virDomainDefFormat(def, privconn->driver->caps, flags);
cleanup:
- if (privdom)
- virObjectUnlock(privdom);
+ if (dom)
+ virObjectUnlock(dom);
return ret;
}
static int
vzDomainGetAutostart(virDomainPtr domain, int *autostart)
{
- virDomainObjPtr privdom;
+ virDomainObjPtr dom;
int ret = -1;
- if (!(privdom = vzDomObjFromDomain(domain)))
+ if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
- *autostart = privdom->autostart;
+ *autostart = dom->autostart;
ret = 0;
cleanup:
- if (privdom)
- virObjectUnlock(privdom);
+ if (dom)
+ virObjectUnlock(dom);
return ret;
}
@@ -877,14 +877,14 @@ vzDomainGetVcpus(virDomainPtr domain,
unsigned char *cpumaps,
int maplen)
{
- virDomainObjPtr privdom = NULL;
+ virDomainObjPtr dom = NULL;
size_t i;
int ret = -1;
- if (!(privdom = vzDomObjFromDomainRef(domain)))
+ if (!(dom = vzDomObjFromDomainRef(domain)))
goto cleanup;
- if (!virDomainObjIsActive(privdom)) {
+ if (!virDomainObjIsActive(dom)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s",
_("cannot list vcpu pinning for an inactive domain"));
@@ -899,7 +899,7 @@ vzDomainGetVcpus(virDomainPtr domain,
if (cpumaps != NULL) {
memset(cpumaps, 0, maplen * maxinfo);
for (i = 0; i < maxinfo; i++)
- virBitmapToDataBuf(privdom->def->cpumask,
+ virBitmapToDataBuf(dom->def->cpumask,
VIR_GET_CPUMAP(cpumaps, maplen, i),
maplen);
}
@@ -909,7 +909,7 @@ vzDomainGetVcpus(virDomainPtr domain,
for (i = 0; i < maxinfo; i++) {
info[i].number = i;
info[i].state = VIR_VCPU_RUNNING;
- if (prlsdkGetVcpuStats(privdom, i, &info[i].cpuTime) < 0)
+ if (prlsdkGetVcpuStats(dom, i, &info[i].cpuTime) < 0)
goto cleanup;
}
}
@@ -917,8 +917,8 @@ vzDomainGetVcpus(virDomainPtr domain,
ret = maxinfo;
cleanup:
- if (privdom)
- virDomainObjEndAPI(&privdom);
+ if (dom)
+ virDomainObjEndAPI(&dom);
return ret;
}
@@ -1145,31 +1145,31 @@ static int vzCheckConfigUpdateFlags(virDomainObjPtr dom, unsigned
int *flags)
return 0;
}
-static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
+static int vzDomainAttachDeviceFlags(virDomainPtr domain, const char *xml,
unsigned int flags)
{
int ret = -1;
- vzConnPtr privconn = dom->conn->privateData;
+ vzConnPtr privconn = domain->conn->privateData;
virDomainDeviceDefPtr dev = NULL;
- virDomainObjPtr privdom = NULL;
+ virDomainObjPtr dom = NULL;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- if (!(privdom = vzDomObjFromDomain(dom)))
+ if (!(dom = vzDomObjFromDomain(domain)))
return -1;
- if (vzCheckConfigUpdateFlags(privdom, &flags) < 0)
+ if (vzCheckConfigUpdateFlags(dom, &flags) < 0)
goto cleanup;
- dev = virDomainDeviceDefParse(xml, privdom->def, privconn->driver->caps,
+ dev = virDomainDeviceDefParse(xml, dom->def, privconn->driver->caps,
privconn->driver->xmlopt,
VIR_DOMAIN_XML_INACTIVE);
if (dev == NULL)
goto cleanup;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = prlsdkAttachVolume(privconn->driver, privdom, dev->data.disk);
+ ret = prlsdkAttachVolume(privconn->driver, dom, dev->data.disk);
if (ret) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("disk attach failed"));
@@ -1177,7 +1177,7 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char
*xml,
}
break;
case VIR_DOMAIN_DEVICE_NET:
- ret = prlsdkAttachNet(privconn->driver, privdom, dev->data.net);
+ ret = prlsdkAttachNet(privconn->driver, dom, dev->data.net);
if (ret) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("network attach failed"));
@@ -1193,42 +1193,42 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char
*xml,
ret = 0;
cleanup:
- virObjectUnlock(privdom);
+ virObjectUnlock(dom);
return ret;
}
-static int vzDomainAttachDevice(virDomainPtr dom, const char *xml)
+static int vzDomainAttachDevice(virDomainPtr domain, const char *xml)
{
- return vzDomainAttachDeviceFlags(dom, xml,
+ return vzDomainAttachDeviceFlags(domain, xml,
VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_AFFECT_LIVE);
}
-static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
+static int vzDomainDetachDeviceFlags(virDomainPtr domain, const char *xml,
unsigned int flags)
{
int ret = -1;
- vzConnPtr privconn = dom->conn->privateData;
+ vzConnPtr privconn = domain->conn->privateData;
virDomainDeviceDefPtr dev = NULL;
- virDomainObjPtr privdom = NULL;
+ virDomainObjPtr dom = NULL;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- privdom = vzDomObjFromDomain(dom);
- if (privdom == NULL)
+ dom = vzDomObjFromDomain(domain);
+ if (dom == NULL)
return -1;
- if (vzCheckConfigUpdateFlags(privdom, &flags) < 0)
+ if (vzCheckConfigUpdateFlags(dom, &flags) < 0)
goto cleanup;
- dev = virDomainDeviceDefParse(xml, privdom->def, privconn->driver->caps,
+ dev = virDomainDeviceDefParse(xml, dom->def, privconn->driver->caps,
privconn->driver->xmlopt,
VIR_DOMAIN_XML_INACTIVE);
if (dev == NULL)
goto cleanup;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = prlsdkDetachVolume(privdom, dev->data.disk);
+ ret = prlsdkDetachVolume(dom, dev->data.disk);
if (ret) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("disk detach failed"));
@@ -1236,7 +1236,7 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char
*xml,
}
break;
case VIR_DOMAIN_DEVICE_NET:
- ret = prlsdkDetachNet(privconn->driver, privdom, dev->data.net);
+ ret = prlsdkDetachNet(privconn->driver, dom, dev->data.net);
if (ret) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("network detach failed"));
@@ -1252,14 +1252,14 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char
*xml,
ret = 0;
cleanup:
- virObjectUnlock(privdom);
+ virObjectUnlock(dom);
return ret;
}
-static int vzDomainDetachDevice(virDomainPtr dom, const char *xml)
+static int vzDomainDetachDevice(virDomainPtr domain, const char *xml)
{
- return vzDomainDetachDeviceFlags(dom, xml,
+ return vzDomainDetachDeviceFlags(domain, xml,
VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_AFFECT_LIVE);
}
@@ -1434,52 +1434,52 @@ vzDomainMemoryStats(virDomainPtr domain,
}
static int
-vzDomainGetVcpusFlags(virDomainPtr dom,
+vzDomainGetVcpusFlags(virDomainPtr domain,
unsigned int flags)
{
- virDomainObjPtr privdom = NULL;
+ virDomainObjPtr dom = NULL;
int ret = -1;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
- if (!(privdom = vzDomObjFromDomain(dom)))
+ if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
if (flags & VIR_DOMAIN_VCPU_MAXIMUM)
- ret = virDomainDefGetVcpusMax(privdom->def);
+ ret = virDomainDefGetVcpusMax(dom->def);
else
- ret = virDomainDefGetVcpus(privdom->def);
+ ret = virDomainDefGetVcpus(dom->def);
cleanup:
- if (privdom)
- virObjectUnlock(privdom);
+ if (dom)
+ virObjectUnlock(dom);
return ret;
}
-static int vzDomainGetMaxVcpus(virDomainPtr dom)
+static int vzDomainGetMaxVcpus(virDomainPtr domain)
{
- return vzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
- VIR_DOMAIN_VCPU_MAXIMUM));
+ return vzDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_VCPU_MAXIMUM));
}
-static int vzDomainIsUpdated(virDomainPtr dom)
+static int vzDomainIsUpdated(virDomainPtr domain)
{
- virDomainObjPtr privdom;
+ virDomainObjPtr dom;
int ret = -1;
/* As far as VZ domains are always updated (e.g. current==persistent),
* we just check for domain existence */
- if (!(privdom = vzDomObjFromDomain(dom)))
+ if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
ret = 0;
cleanup:
- if (privdom)
- virObjectUnlock(privdom);
+ if (dom)
+ virObjectUnlock(dom);
return ret;
}
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 06/10] vz: simplify refcount on sdkdom in prlsdkLoadDomain
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index b7627e7..67c68df 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1642,17 +1642,16 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
prlsdkConvertDomainState(domainState, envId, dom);
- if (!pdom->sdkdom) {
- PrlHandle_AddRef(sdkdom);
+ if (pdom->sdkdom == PRL_INVALID_HANDLE)
pdom->sdkdom = sdkdom;
- }
+ else
+ PrlHandle_Free(sdkdom);
if (autostart == PAO_VM_START_ON_LOAD)
dom->autostart = 1;
else
dom->autostart = 0;
- PrlHandle_Free(sdkdom);
return 0;
error:
PrlHandle_Free(sdkdom);
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 07/10] vz: hold stats cache in a pointer in vzDomObj
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(a)virtuozzo.com>
---
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
6:24 a.m.
New subject: [libvirt] [PATCH 08/10] vz: introduce stats cache lock
Use this lock to coordinate requests to cache and stats events
from vzsdk. Request is a series of calls of vzCountersCacheGetParam
which must be enslosed in a vzCountersCacheBegin, vzCountersCacheEnd.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 236 ++++++++++++++++++++++++++++++++++++++---------------
src/vz/vz_sdk.c | 120 ++++++++++++++++-----------
src/vz/vz_sdk.h | 14 +++-
src/vz/vz_utils.h | 2 +
4 files changed, 256 insertions(+), 116 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 95739bd..1aabfd7 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -597,15 +597,45 @@ vzDomainLookupByName(virConnectPtr conn, const char *name)
}
static int
+vzStatsGetCpuTotal(vzCountersCachePtr cache,
+ unsigned int num,
+ unsigned long long *total)
+{
+ int ret = -1;
+ size_t i;
+
+ if (vzCountersCacheBegin(cache) < 0)
+ return -1;
+
+ for (i = 0; i < num; ++i) {
+ unsigned long long cpu;
+
+ if (prlsdkGetVcpuStats(cache, i, &cpu) < 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("cannot read cputime for domain"));
+ goto cleanup;
+ }
+ total += cpu;
+ }
+
+ ret = 0;
+
+ cleanup:
+ vzCountersCacheEnd(cache);
+
+ return ret;
+}
+
+static int
vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
{
virDomainObjPtr dom;
+ vzDomObjPtr privdom;
int ret = -1;
- size_t i;
unsigned int nvcpus;
if (!(dom = vzDomObjFromDomainRef(domain)))
- goto cleanup;
+ return -1;
info->state = virDomainObjGetState(dom, NULL);
info->memory = dom->def->mem.cur_balloon;
@@ -614,26 +644,17 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
info->cpuTime = 0;
if (!virDomainObjIsActive(dom)) {
- ret = 0;
- goto cleanup;
+ virDomainObjEndAPI(&dom);
+ return 0;
}
nvcpus = virDomainDefGetVcpus(dom->def);
- for (i = 0; i < nvcpus; ++i) {
- unsigned long long vtime;
+ virObjectUnlock(dom);
+ privdom = dom->privateData;
+ ret = vzStatsGetCpuTotal(privdom->cache, nvcpus, &info->cpuTime);
+ virObjectUnref(dom);
- if (prlsdkGetVcpuStats(dom, i, &vtime) < 0) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("cannot read cputime for domain"));
- goto cleanup;
- }
- info->cpuTime += vtime;
- }
- ret = 0;
-
- cleanup:
- virDomainObjEndAPI(&dom);
return ret;
}
@@ -869,6 +890,33 @@ vzConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED,
return cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags);
}
+static int
+vzStatsGetCpuInfo(vzCountersCachePtr cache,
+ virVcpuInfoPtr info,
+ int num)
+
+{
+ int ret = -1;
+ size_t i;
+
+ if (vzCountersCacheBegin(cache) < 0)
+ return -1;
+
+ memset(info, 0, sizeof(*info) * num);
+ for (i = 0; i < num; i++) {
+ info[i].number = i;
+ info[i].state = VIR_VCPU_RUNNING;
+ if (prlsdkGetVcpuStats(cache, i, &info[i].cpuTime) < 0)
+ goto cleanup;
+ }
+
+ ret = num;
+
+ cleanup:
+ vzCountersCacheEnd(cache);
+
+ return ret;
+}
static int
vzDomainGetVcpus(virDomainPtr domain,
@@ -878,11 +926,12 @@ vzDomainGetVcpus(virDomainPtr domain,
int maplen)
{
virDomainObjPtr dom = NULL;
+ vzDomObjPtr privdom;
size_t i;
int ret = -1;
if (!(dom = vzDomObjFromDomainRef(domain)))
- goto cleanup;
+ return -1;
if (!virDomainObjIsActive(dom)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -905,20 +954,19 @@ vzDomainGetVcpus(virDomainPtr domain,
}
if (info != NULL) {
- memset(info, 0, sizeof(*info) * maxinfo);
- for (i = 0; i < maxinfo; i++) {
- info[i].number = i;
- info[i].state = VIR_VCPU_RUNNING;
- if (prlsdkGetVcpuStats(dom, i, &info[i].cpuTime) < 0)
- goto cleanup;
- }
+ virObjectUnlock(dom);
+ privdom = dom->privateData;
+ ret = vzStatsGetCpuInfo(privdom->cache, info, maxinfo);
+ virObjectUnref(dom);
+
+ return ret;
}
ret = maxinfo;
cleanup:
- if (dom)
- virDomainObjEndAPI(&dom);
+ virDomainObjEndAPI(&dom);
+
return ret;
}
@@ -1278,10 +1326,64 @@ vzDomainGetMaxMemory(virDomainPtr domain)
}
static int
+vzStatsGetBlockStats(vzCountersCachePtr cache,
+ const char *disk,
+ char **disks,
+ int ndisks,
+ virDomainBlockStatsPtr stats)
+{
+ int ret = -1;
+ size_t i;
+
+ if (vzCountersCacheBegin(cache) < 0)
+ return -1;
+
+ if (disk) {
+ if (prlsdkGetBlockStats(cache, disk, stats) < 0)
+ goto cleanup;
+ } else if (disks) {
+ virDomainBlockStatsStruct s;
+
+#define PARALLELS_ZERO_STATS(VAR, TYPE, NAME) \
+ stats->VAR = 0;
+
+ PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_ZERO_STATS)
+
+#undef PARALLELS_ZERO_STATS
+
+ for (i = 0; i < ndisks; i++) {
+ if (prlsdkGetBlockStats(cache, disks[i], &s) < 0)
+ goto cleanup;
+
+#define PARALLELS_SUM_STATS(VAR, TYPE, NAME) \
+ if (s.VAR != -1) \
+ stats->VAR += s.VAR;
+
+ PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_SUM_STATS)
+
+#undef PARALLELS_SUM_STATS
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("one of disks or disk parameters should be set"));
+ goto cleanup;
+ }
+
+ stats->errs = -1;
+ ret = 0;
+
+ cleanup:
+ vzCountersCacheEnd(cache);
+
+ return ret;
+}
+
+static int
vzDomainBlockStats(virDomainPtr domain, const char *path,
virDomainBlockStatsPtr stats)
{
virDomainObjPtr dom = NULL;
+ vzDomObjPtr privdom;
int ret = -1;
size_t i;
int idx;
@@ -1292,59 +1394,41 @@ 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;
+ goto error;
}
if (!(disk = prlsdkGetDiskStatName(dom->def->disks[idx])))
- goto cleanup;
-
- if (prlsdkGetBlockStats(dom, disk, stats) < 0)
- goto cleanup;
+ goto error;
} else {
- virDomainBlockStatsStruct s;
-
-#define PARALLELS_ZERO_STATS(VAR, TYPE, NAME) \
- stats->VAR = 0;
-
- PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_ZERO_STATS)
-
-#undef PARALLELS_ZERO_STATS
-
ndisks = dom->def->ndisks;
if (VIR_ALLOC_N(disks, ndisks))
- goto cleanup;
+ goto error;
for (i = 0; i < ndisks; i++)
if (!(disks[i] = prlsdkGetDiskStatName(dom->def->disks[i])))
- goto cleanup;
-
- for (i = 0; i < ndisks; i++) {
- if (prlsdkGetBlockStats(dom, disk, &s) < 0)
- goto cleanup;
-
-#define PARALLELS_SUM_STATS(VAR, TYPE, NAME) \
- if (s.VAR != -1) \
- stats->VAR += s.VAR;
-
- PARALLELS_BLOCK_STATS_FOREACH(PARALLELS_SUM_STATS)
-
-#undef PARALLELS_SUM_STATS
- }
+ goto error;
}
- stats->errs = -1;
- ret = 0;
+
+ virObjectUnlock(dom);
+ privdom = dom->privateData;
+ ret = vzStatsGetBlockStats(privdom->cache, disk, disks, ndisks, stats);
+ virObjectUnref(dom);
cleanup:
- if (dom)
- virDomainObjEndAPI(&dom);
VIR_FREE(disk);
virStringFreeListCount(disks, ndisks);
return ret;
+
+ error:
+ virDomainObjEndAPI(&dom);
+ goto cleanup;
}
static int
@@ -1403,13 +1487,25 @@ vzDomainInterfaceStats(virDomainPtr domain,
virDomainInterfaceStatsPtr stats)
{
virDomainObjPtr dom = NULL;
+ vzDomObjPtr privdom;
+ vzCountersCachePtr cache;
int ret;
if (!(dom = vzDomObjFromDomainRef(domain)))
return -1;
- ret = prlsdkGetNetStats(dom, path, stats);
- virDomainObjEndAPI(&dom);
+ virObjectUnlock(dom);
+ privdom = dom->privateData;
+ cache = privdom->cache;
+
+ if (vzCountersCacheBegin(cache) < 0) {
+ virObjectUnref(dom);
+ return -1;
+ }
+
+ ret = prlsdkGetNetStats(cache, privdom->sdkdom, path, stats);
+ vzCountersCacheEnd(cache);
+ virObjectUnref(dom);
return ret;
}
@@ -1421,14 +1517,26 @@ vzDomainMemoryStats(virDomainPtr domain,
unsigned int flags)
{
virDomainObjPtr dom = NULL;
+ vzDomObjPtr privdom;
+ vzCountersCachePtr cache;
int ret = -1;
virCheckFlags(0, -1);
if (!(dom = vzDomObjFromDomainRef(domain)))
return -1;
- ret = prlsdkGetMemoryStats(dom, stats, nr_stats);
- virDomainObjEndAPI(&dom);
+ virObjectUnlock(dom);
+ privdom = dom->privateData;
+ cache = privdom->cache;
+
+ if (vzCountersCacheBegin(cache) < 0) {
+ virObjectUnref(dom);
+ return -1;
+ }
+
+ ret = prlsdkGetMemoryStats(cache, stats, nr_stats);
+ vzCountersCacheEnd(cache);
+ virObjectUnref(dom);
return ret;
}
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 5cef432..a5474dc 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -41,7 +41,7 @@ static int
prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid);
static vzCountersCachePtr
-vzCountersCacheNew(void);
+vzCountersCacheNew(PRL_HANDLE sdkdom);
static void
vzCountersCacheFree(vzCountersCachePtr cache);
@@ -1638,7 +1638,7 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom)
}
if (pdom->sdkdom == PRL_INVALID_HANDLE) {
- if (!(pdom->cache = vzCountersCacheNew()))
+ if (!(pdom->cache = vzCountersCacheNew(sdkdom)))
goto error;
pdom->sdkdom = sdkdom;
} else {
@@ -1876,47 +1876,52 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
#define PARALLELS_STATISTICS_DROP_COUNT 3
-static PRL_RESULT
+static void
prlsdkHandlePerfEvent(vzDriverPtr driver,
PRL_HANDLE event,
unsigned char *uuid)
{
- virDomainObjPtr dom = NULL;
+ virDomainObjPtr dom;
vzDomObjPtr privdom = NULL;
PRL_HANDLE job = PRL_INVALID_HANDLE;
+ vzCountersCachePtr cache;
- dom = virDomainObjListFindByUUID(driver->domains, uuid);
- if (dom == NULL)
- goto cleanup;
+ dom = virDomainObjListFindByUUIDRef(driver->domains, uuid);
+ if (dom == NULL) {
+ PrlHandle_Free(event);
+ return;
+ }
+
+ virObjectUnlock(dom);
privdom = dom->privateData;
+ cache = privdom->cache;
+ virMutexLock(&cache->lock);
/* delayed event after unsubscribe */
- if (privdom->cache->count == -1)
+ if (cache->count == -1)
goto cleanup;
- PrlHandle_Free(privdom->cache->stats);
- privdom->cache->stats = PRL_INVALID_HANDLE;
+ PrlHandle_Free(cache->stats);
+ cache->stats = PRL_INVALID_HANDLE;
- if (privdom->cache->count > PARALLELS_STATISTICS_DROP_COUNT) {
- job = PrlVm_UnsubscribeFromPerfStats(privdom->sdkdom);
+ if (cache->count > PARALLELS_STATISTICS_DROP_COUNT) {
+ job = PrlVm_UnsubscribeFromPerfStats(cache->sdkdom);
if (PRL_FAILED(waitJob(job)))
goto cleanup;
/* change state to unsubscribed */
- privdom->cache->count = -1;
+ cache->count = -1;
} else {
- ++privdom->cache->count;
- privdom->cache->stats = event;
+ ++cache->count;
+ cache->stats = event;
/* thus we get own of event handle */
event = PRL_INVALID_HANDLE;
- virCondSignal(&privdom->cache->cond);
+ virCondSignal(&cache->cond);
}
cleanup:
PrlHandle_Free(event);
- if (dom)
- virObjectUnlock(dom);
-
- return PRL_ERR_SUCCESS;
+ virMutexUnlock(&cache->lock);
+ virObjectUnref(dom);
}
static PRL_RESULT
@@ -4004,14 +4009,14 @@ prlsdkDomainManagedSaveRemove(virDomainObjPtr dom)
}
static int
-prlsdkExtractStatsParam(PRL_HANDLE sdkstats, const char *name, long long *val)
+vzCountersCacheGetParam(vzCountersCachePtr cache, const char *name, long long *val)
{
PRL_HANDLE param = PRL_INVALID_HANDLE;
PRL_RESULT pret;
PRL_INT64 pval = 0;
int ret = -1;
- pret = PrlEvent_GetParamByName(sdkstats, name, ¶m);
+ pret = PrlEvent_GetParamByName(cache->stats, name, ¶m);
if (pret == PRL_ERR_NO_DATA) {
*val = -1;
ret = 0;
@@ -4033,36 +4038,37 @@ 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)
+int
+vzCountersCacheBegin(vzCountersCachePtr cache)
{
- vzDomObjPtr privdom = dom->privateData;
PRL_HANDLE job = PRL_INVALID_HANDLE;
unsigned long long now;
- if (privdom->cache->stats != PRL_INVALID_HANDLE) {
+ virMutexLock(&cache->lock);
+
+ if (cache->stats != PRL_INVALID_HANDLE) {
/* reset count to keep subscribtion */
- privdom->cache->count = 0;
- return prlsdkExtractStatsParam(privdom->cache->stats, name, val);
+ cache->count = 0;
+ return 0;
}
- if (privdom->cache->count == -1) {
- job = PrlVm_SubscribeToPerfStats(privdom->sdkdom, NULL);
+ if (cache->count == -1) {
+ job = PrlVm_SubscribeToPerfStats(cache->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;
+ 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 (cache->stats == PRL_INVALID_HANDLE) {
+ if (virCondWaitUntil(&cache->cond, &cache->lock,
now + PARALLELS_STATISTICS_TIMEOUT) < 0) {
if (errno == ETIMEDOUT) {
virReportError(VIR_ERR_OPERATION_TIMEOUT, "%s",
@@ -4076,11 +4082,20 @@ prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long
long *val)
}
}
- return prlsdkExtractStatsParam(privdom->cache->stats, name, val);
+ return 0;
+
error:
+ virMutexUnlock(&cache->lock);
+
return -1;
}
+void
+vzCountersCacheEnd(vzCountersCachePtr cache)
+{
+ virMutexUnlock(&cache->lock);
+}
+
static void
vzCountersCacheFree(vzCountersCachePtr cache)
{
@@ -4089,11 +4104,12 @@ vzCountersCacheFree(vzCountersCachePtr cache)
PrlHandle_Free(cache->stats);
virCondDestroy(&cache->cond);
+ virMutexDestroy(&cache->lock);
VIR_FREE(cache);
}
static vzCountersCachePtr
-vzCountersCacheNew(void)
+vzCountersCacheNew(PRL_HANDLE sdkdom)
{
vzCountersCachePtr cache = NULL;
@@ -4104,8 +4120,14 @@ vzCountersCacheNew(void)
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize
condition"));
goto error;
}
+ if (virMutexInit(&cache->lock) < 0) {
+ virCondDestroy(&cache->cond);
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize
mutex"));
+ goto error;
+ }
cache->stats = PRL_INVALID_HANDLE;
cache->count = -1;
+ cache->sdkdom = sdkdom;
return cache;
@@ -4114,6 +4136,7 @@ vzCountersCacheNew(void)
return NULL;
}
+
char* prlsdkGetDiskStatName(virDomainDiskDefPtr disk)
{
virDomainDeviceDriveAddressPtr address;
@@ -4148,7 +4171,7 @@ char* prlsdkGetDiskStatName(virDomainDiskDefPtr disk)
}
int
-prlsdkGetBlockStats(virDomainObjPtr dom,
+prlsdkGetBlockStats(vzCountersCachePtr cache,
const char *disk,
virDomainBlockStatsPtr stats)
{
@@ -4158,7 +4181,7 @@ prlsdkGetBlockStats(virDomainObjPtr dom,
#define PRLSDK_GET_STAT_PARAM(VAL, TYPE, NAME) \
if (virAsprintf(&name, "devices.%s.%s", disk, NAME) < 0)
\
goto cleanup; \
- if (prlsdkGetStatsParam(dom, name, &stats->VAL) < 0) \
+ if (vzCountersCacheGetParam(cache, name, &stats->VAL) < 0) \
goto cleanup; \
VIR_FREE(name);
@@ -4176,20 +4199,19 @@ prlsdkGetBlockStats(virDomainObjPtr dom,
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,
@@ -4216,7 +4238,9 @@ prlsdkFindNetByPath(virDomainObjPtr dom, const char *path)
}
int
-prlsdkGetNetStats(virDomainObjPtr dom, const char *path,
+prlsdkGetNetStats(vzCountersCachePtr cache,
+ PRL_HANDLE sdkdom,
+ const char *path,
virDomainInterfaceStatsPtr stats)
{
int ret = -1;
@@ -4225,7 +4249,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;
@@ -4235,7 +4259,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 (vzCountersCacheGetParam(cache, name, &stats->VAL) < 0) \
goto cleanup; \
VIR_FREE(name);
@@ -4259,7 +4283,7 @@ prlsdkGetNetStats(virDomainObjPtr dom, const char *path,
}
int
-prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *vtime)
+prlsdkGetVcpuStats(vzCountersCachePtr cache, int idx, unsigned long long *vtime)
{
char *name = NULL;
long long ptime = 0;
@@ -4267,7 +4291,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 (vzCountersCacheGetParam(cache, name, &ptime) < 0)
goto cleanup;
*vtime = ptime == -1 ? 0 : ptime;
ret = 0;
@@ -4278,7 +4302,7 @@ prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long
*vtime)
}
int
-prlsdkGetMemoryStats(virDomainObjPtr dom,
+prlsdkGetMemoryStats(vzCountersCachePtr cache,
virDomainMemoryStatPtr stats,
unsigned int nr_stats)
{
@@ -4287,7 +4311,7 @@ prlsdkGetMemoryStats(virDomainObjPtr dom,
size_t i = 0;
#define PRLSDK_GET_COUNTER(NAME, VALUE) \
- if (prlsdkGetStatsParam(dom, NAME, &VALUE) < 0) \
+ if (vzCountersCacheGetParam(cache, 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 033a3fe..87466e7 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -68,17 +68,20 @@ int
prlsdkDetachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk);
char* prlsdkGetDiskStatName(virDomainDiskDefPtr disk);
int
-prlsdkGetBlockStats(virDomainObjPtr dom, const char *disk, virDomainBlockStatsPtr
stats);
+prlsdkGetBlockStats(vzCountersCachePtr cache, const char *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(vzCountersCachePtr cache,
+ PRL_HANDLE sdkdom,
+ const char *path,
+ virDomainInterfaceStatsPtr stats);
int
-prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *time);
+prlsdkGetVcpuStats(vzCountersCachePtr cache, int idx, unsigned long long *time);
int
-prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int
nr_stats);
+prlsdkGetMemoryStats(vzCountersCachePtr cache, virDomainMemoryStatPtr stats, unsigned int
nr_stats);
void
prlsdkDomObjFreePrivate(void *p);
/* memsize is in MiB */
@@ -94,3 +97,6 @@ prlsdkMigrate(virDomainObjPtr dom,
const char unsigned *session_uuid,
const char *dname,
unsigned int flags);
+
+int vzCountersCacheBegin(vzCountersCachePtr cache);
+void vzCountersCacheEnd(vzCountersCachePtr cache);
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index 43d46ca..a3ff152 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -95,7 +95,9 @@ typedef struct _vzConn *vzConnPtr;
struct _vzCountersCache {
+ PRL_HANDLE sdkdom; /* unreferenced */
PRL_HANDLE stats;
+ virMutex lock;
virCond cond;
/* = -1 - unsubscribed
-1 - subscribed */
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 09/10] vz: extract on stats cache update into a function
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 68 ++++++++++++++++++++++++++++++-------------------------
src/vz/vz_utils.h | 2 +-
2 files changed, 38 insertions(+), 32 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index a5474dc..51730be 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -44,6 +44,8 @@ static vzCountersCachePtr
vzCountersCacheNew(PRL_HANDLE sdkdom);
static void
vzCountersCacheFree(vzCountersCachePtr cache);
+static void
+vzCountersCacheEvent(vzCountersCachePtr cache, PRL_HANDLE event);
VIR_LOG_INIT("parallels.sdk");
@@ -1874,8 +1876,6 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
return;
}
-#define PARALLELS_STATISTICS_DROP_COUNT 3
-
static void
prlsdkHandlePerfEvent(vzDriverPtr driver,
PRL_HANDLE event,
@@ -1883,8 +1883,6 @@ prlsdkHandlePerfEvent(vzDriverPtr driver,
{
virDomainObjPtr dom;
vzDomObjPtr privdom = NULL;
- PRL_HANDLE job = PRL_INVALID_HANDLE;
- vzCountersCachePtr cache;
dom = virDomainObjListFindByUUIDRef(driver->domains, uuid);
if (dom == NULL) {
@@ -1894,33 +1892,7 @@ prlsdkHandlePerfEvent(vzDriverPtr driver,
virObjectUnlock(dom);
privdom = dom->privateData;
- cache = privdom->cache;
- virMutexLock(&cache->lock);
-
- /* delayed event after unsubscribe */
- if (cache->count == -1)
- goto cleanup;
-
- PrlHandle_Free(cache->stats);
- cache->stats = PRL_INVALID_HANDLE;
-
- if (cache->count > PARALLELS_STATISTICS_DROP_COUNT) {
- job = PrlVm_UnsubscribeFromPerfStats(cache->sdkdom);
- if (PRL_FAILED(waitJob(job)))
- goto cleanup;
- /* change state to unsubscribed */
- cache->count = -1;
- } else {
- ++cache->count;
- cache->stats = event;
- /* thus we get own of event handle */
- event = PRL_INVALID_HANDLE;
- virCondSignal(&cache->cond);
- }
-
- cleanup:
- PrlHandle_Free(event);
- virMutexUnlock(&cache->lock);
+ vzCountersCacheEvent(privdom->cache, event);
virObjectUnref(dom);
}
@@ -4137,6 +4109,40 @@ vzCountersCacheNew(PRL_HANDLE sdkdom)
return NULL;
}
+#define PARALLELS_STATISTICS_DROP_COUNT 3
+
+static void
+vzCountersCacheEvent(vzCountersCachePtr cache, PRL_HANDLE event)
+{
+ PRL_HANDLE job = PRL_INVALID_HANDLE;
+ virMutexLock(&cache->lock);
+
+ /* delayed event after unsubscribe */
+ if (cache->count == -1)
+ goto cleanup;
+
+ PrlHandle_Free(cache->stats);
+ cache->stats = PRL_INVALID_HANDLE;
+
+ if (cache->count > PARALLELS_STATISTICS_DROP_COUNT) {
+ job = PrlVm_UnsubscribeFromPerfStats(cache->sdkdom);
+ if (PRL_FAILED(waitJob(job)))
+ goto cleanup;
+ /* change state to unsubscribed */
+ cache->count = -1;
+ } else {
+ ++cache->count;
+ cache->stats = event;
+ /* thus we get own of event handle */
+ event = PRL_INVALID_HANDLE;
+ virCondSignal(&cache->cond);
+ }
+
+ cleanup:
+ PrlHandle_Free(event);
+ virMutexUnlock(&cache->lock);
+}
+
char* prlsdkGetDiskStatName(virDomainDiskDefPtr disk)
{
virDomainDeviceDriveAddressPtr address;
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index a3ff152..d4ee88d 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -110,7 +110,7 @@ typedef vzCountersCache *vzCountersCachePtr;
struct vzDomObj {
int id;
PRL_HANDLE sdkdom;
- /* immutable */
+ /* immutable self-locking*/
vzCountersCachePtr cache;
};
--
1.8.3.1
6:24 a.m.
New subject: [libvirt] [PATCH 10/10] vz: fix many stat request issue
Signalling one thread on arriving stats data is not correct. In case
2 stat requests and empty stats cache one thread subscribes to the
stats events and both wait arriving stats event.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 51730be..50b21a8 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4135,7 +4135,7 @@ vzCountersCacheEvent(vzCountersCachePtr cache, PRL_HANDLE event)
cache->stats = event;
/* thus we get own of event handle */
event = PRL_INVALID_HANDLE;
- virCondSignal(&cache->cond);
+ virCondBroadcast(&cache->cond);
}
cleanup:
--
1.8.3.1
2:19 a.m.
This patch series is superseded by
https://www.redhat.com/archives/libvir-list/2016-June/msg00065.html
which is more simple.
On 02.06.2016 14:24, Nikolay Shirokovskiy wrote:
Current stats cache uses same lock as domain cache, namely lock
of virDomainObjPtr. First this is not necessary, domain cache is
used to translate id of object of interest from libvirt to vzsdk
one. Besides this domain object is not used. Second this is risky
game as lock is dropped on waiting for stat events from sdk deep
down on stack so callers throughout the stack should be written
very carefully.
This patch series makes stats cache self locking and thus making
code much more solid. This is done thru introducing extra lock for
stats cache. Statistics API functions first use domain cache then
drop domain lock, query stats cache and unreference domain object.
Patches 1-4 conform API functions to this structure. Patches 5-7
makes subsidiary improvments. Patch 8 finally introduce new lock.
Last patches are cleanup[9] and bugfix[10].
Nikolay Shirokovskiy (10):
vz: pass string instead of disk definition to block stat function
vz: prepare disks names before getting all disks stats
vz: move getting stats in vzDomainGetVcpus to the end
vz: move getting stats in vzDomainGetInfo to the end
vz: use consistent naming for different domain object in vz_driver.c
vz: simplify refcount on sdkdom in prlsdkLoadDomain
vz: hold stats cache in a pointer in vzDomObj
vz: introduce stats cache lock
vz: extract on stats cache update into a function
vz: fix many stat request issue
src/vz/vz_driver.c | 397 +++++++++++++++++++++++++++++++++++------------------
src/vz/vz_sdk.c | 230 +++++++++++++++++++++----------
src/vz/vz_sdk.h | 15 +-
src/vz/vz_utils.c | 9 --
src/vz/vz_utils.h | 6 +-
5 files changed, 438 insertions(+), 219 deletions(-)
3094
days inactive
3095
days old
11 comments
1 participants
participants (1)
-
Nikolay Shirokovskiy