[libvirt] [PATCH] openvz: read vmguarpages/privvmpages to set memory tunables [v2]
--- This is a reworked version of the patch already sent, now adding handling for "unlimited" (represented by LONG_MAX in openvz) as well as a schema testcase. I also switched all value to unsigned long long to avoid overflows. Note that privvmpages is the amount of memory available to the application. The total memory usage of the container might be higher due to used kernel memory. This does probably warrant an extra tunable later. O.k. to apply? Cheers, -- Guido src/openvz/openvz_conf.c | 113 ++++++++++++ src/openvz/openvz_driver.c | 208 +++++++++++++++++++++++ tests/domainschemadata/domain-openvz-simple.xml | 27 +++ 3 files changed, 348 insertions(+) create mode 100644 tests/domainschemadata/domain-openvz-simple.xml diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 5848ec4..a169ae6 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -129,6 +129,46 @@ int openvzExtractVersion(struct openvz_driver *driver) } +/* Parse config values of the form barrier:limit into barrier and limit */ +static int +openvzParseBarrierLimit(const char* value, + unsigned long long *barrier, + unsigned long long *limit) +{ + char *token; + char *saveptr = NULL; + char *str = strdup(value); + + if (str == NULL) { + virReportOOMError(); + goto error; + } + + token = strtok_r(str, ":", &saveptr); + if (token == NULL) { + goto error; + } else { + if (barrier != NULL) { + if (virStrToLong_ull(token, NULL, 10, barrier)) + goto error; + } + } + token = strtok_r(NULL, ":", &saveptr); + if (token == NULL) { + goto error; + } else { + if (limit != NULL) { + if (virStrToLong_ull(token, NULL, 10, limit)) + goto error; + } + } + return 0; +error: + VIR_FREE(str); + return -1; +} + + static int openvzDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ; @@ -423,6 +463,78 @@ error: } +static int +openvzReadMemConf(virDomainDefPtr def, int veid) +{ + int ret; + char *temp = NULL; + unsigned long long barrier, limit; + const char *param; + unsigned long kb_per_pages; + + kb_per_pages = sysconf(_SC_PAGESIZE) / 1024; + if (kb_per_pages == -1) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + goto error; + } + + /* Memory allocation guarantee */ + param = "VMGUARPAGES"; + ret = openvzReadVPSConfigParam(veid, param, &temp); + if (ret < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Could not read '%s' from config for container %d"), + param, veid); + goto error; + } else if (ret > 0) { + ret = openvzParseBarrierLimit(temp, &barrier, NULL); + if (ret < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse barrier of '%s' " + "from config for container %d"), param, veid); + goto error; + } + if (barrier == LONG_MAX) + def->mem.min_guarantee = 0ull; + else + def->mem.min_guarantee = barrier * kb_per_pages; + } + + /* Memory hard and soft limits */ + param = "PRIVVMPAGES"; + ret = openvzReadVPSConfigParam(veid, param, &temp); + if (ret < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Could not read '%s' from config for container %d"), + param, veid); + goto error; + } else if (ret > 0) { + ret = openvzParseBarrierLimit(temp, &barrier, &limit); + if (ret < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse barrier and limit of '%s' " + "from config for container %d"), param, veid); + goto error; + } + if (barrier == LONG_MAX) + def->mem.soft_limit = 0ull; + else + def->mem.soft_limit = barrier * kb_per_pages; + + if (limit == LONG_MAX) + def->mem.hard_limit = 0ull; + else + def->mem.hard_limit = limit * kb_per_pages; + } + + ret = 0; +error: + VIR_FREE(temp); + return ret; +} + + /* Free all memory associated with a openvz_driver structure */ void openvzFreeDriver(struct openvz_driver *driver) @@ -535,6 +647,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { openvzReadNetworkConf(dom->def, veid); openvzReadFSConf(dom->def, veid); + openvzReadMemConf(dom->def, veid); virUUIDFormat(dom->def->uuid, uuidstr); if (virHashAddEntry(driver->domains.objs, uuidstr, dom) < 0) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index e8b6915..555161e 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -54,6 +54,7 @@ #include "nodeinfo.h" #include "memory.h" #include "virfile.h" +#include "virtypedparam.h" #include "logging.h" #include "command.h" #include "viruri.h" @@ -65,6 +66,8 @@ #define CMDBUF_LEN 1488 #define CMDOP_LEN 288 +#define OPENVZ_NB_MEM_PARAM 3 + static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid); static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type); static int openvzDomainGetMaxVcpus(virDomainPtr dom); @@ -1631,6 +1634,209 @@ cleanup: return -1; } + +static int +openvzDomainGetBarrierLimit(virDomainPtr domain, + const char *param, + unsigned long long *barrier, + unsigned long long *limit) +{ + int status, ret = -1; + char *output = NULL; + virCommandPtr cmd = virCommandNewArgList(VZLIST, "--no-header", NULL); + + virCommandSetOutputBuffer(cmd, &output); + virCommandAddArgFormat(cmd, "-o%s.b,%s.l", param, param); + virCommandAddArg(cmd, domain->name); + if (virCommandRun(cmd, &status)) { + openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to get %s for %s: %d"), param, domain->name, + status); + goto cleanup; + } + + if (sscanf(output, "%llu %llu", barrier, limit) != 2) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't parse "VZLIST" output, got %s"), output); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(output); + virCommandFree(cmd); + return ret; +} + + +static int +openvzDomainSetBarrierLimit(virDomainPtr domain, + const char *param, + unsigned long long barrier, + unsigned long long limit) +{ + int status, ret = -1; + virCommandPtr cmd = virCommandNewArgList(VZCTL, "--quiet", "set", NULL); + + /* LONG_MAX indicates unlimited so reject larger values */ + if (barrier > LONG_MAX || limit > LONG_MAX) { + openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to set %s for %s: value too large"), param, + domain->name); + goto cleanup; + } + + virCommandAddArg(cmd, domain->name); + virCommandAddArgFormat(cmd, "--%s", param); + virCommandAddArgFormat(cmd, "%llu:%llu", barrier, limit); + virCommandAddArg(cmd, "--save"); + if (virCommandRun(cmd, &status)) { + openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to set %s for %s: %d"), param, domain->name, + status); + goto cleanup; + } + + ret = 0; +cleanup: + virCommandFree(cmd); + return ret; +} + + +static int +openvzDomainGetMemoryParameters(virDomainPtr domain, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + int i, result = -1; + const char *name; + long kb_per_pages; + unsigned long long barrier, limit, val; + + virCheckFlags(0, -1); + + kb_per_pages = sysconf(_SC_PAGESIZE) / 1024; + if (kb_per_pages == -1) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + goto cleanup; + } + + if (*nparams == 0) { + *nparams = OPENVZ_NB_MEM_PARAM; + return 0; + } + + for (i = 0; i <= *nparams; i++) { + virMemoryParameterPtr param = ¶ms[i]; + + switch (i) { + case 0: + name = "privvmpages"; + if (openvzDomainGetBarrierLimit(domain, name, &barrier, &limit) < 0) + goto cleanup; + + val = (limit == LONG_MAX) ? 0ull : limit * kb_per_pages; + if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) + goto cleanup; + break; + + case 1: + name = "privvmpages"; + if (openvzDomainGetBarrierLimit(domain, name, &barrier, &limit) < 0) + goto cleanup; + + val = (barrier == LONG_MAX) ? 0ull : barrier * kb_per_pages; + if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) + goto cleanup; + break; + + case 2: + name = "vmguarpages"; + if (openvzDomainGetBarrierLimit(domain, name, &barrier, &limit) < 0) + goto cleanup; + + val = (barrier == LONG_MAX) ? 0ull : barrier * kb_per_pages; + if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_MIN_GUARANTEE, + VIR_TYPED_PARAM_ULLONG, val) < 0) + goto cleanup; + break; + } + } + + if (*nparams > OPENVZ_NB_MEM_PARAM) + *nparams = OPENVZ_NB_MEM_PARAM; + result = 0; + +cleanup: + return result; +} + + +static int +openvzDomainSetMemoryParameters(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + int i, result = -1; + long kb_per_pages; + + kb_per_pages = sysconf(_SC_PAGESIZE) / 1024; + if (kb_per_pages == -1) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + goto cleanup; + } + + virCheckFlags(0, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_MEMORY_MIN_GUARANTEE, + VIR_TYPED_PARAM_ULLONG, + NULL) < 0) + return -1; + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + unsigned long long barrier, limit; + + if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) { + if (openvzDomainGetBarrierLimit(domain, "privvmpages", + &barrier, &limit) < 0) + goto cleanup; + limit = params[i].value.ul / kb_per_pages; + if (openvzDomainSetBarrierLimit(domain, "privvmpages", + barrier, limit) < 0) + goto cleanup; + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { + if (openvzDomainGetBarrierLimit(domain, "privvmpages", + &barrier, &limit) < 0) + goto cleanup; + barrier = params[i].value.ul / kb_per_pages; + if (openvzDomainSetBarrierLimit(domain, "privvmpages", + barrier, limit) < 0) + goto cleanup; + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { + barrier = params[i].value.ul / kb_per_pages; + if (openvzDomainSetBarrierLimit(domain, "vmguarpages", + barrier, LONG_MAX) < 0) + goto cleanup; + } + } + result = 0; +cleanup: + return result; +} + + static int openvzGetVEStatus(virDomainObjPtr vm, int *status, int *reason) { @@ -1752,6 +1958,8 @@ static virDriver openvzDriver = { .domainDestroy = openvzDomainShutdown, /* 0.3.1 */ .domainDestroyFlags = openvzDomainShutdownFlags, /* 0.9.4 */ .domainGetOSType = openvzGetOSType, /* 0.3.1 */ + .domainGetMemoryParameters = openvzDomainGetMemoryParameters, /* 0.9.12 */ + .domainSetMemoryParameters = openvzDomainSetMemoryParameters, /* 0.9.12 */ .domainGetInfo = openvzDomainGetInfo, /* 0.3.1 */ .domainGetState = openvzDomainGetState, /* 0.9.2 */ .domainSetVcpus = openvzDomainSetVcpus, /* 0.4.6 */ diff --git a/tests/domainschemadata/domain-openvz-simple.xml b/tests/domainschemadata/domain-openvz-simple.xml new file mode 100644 index 0000000..aba64a4 --- /dev/null +++ b/tests/domainschemadata/domain-openvz-simple.xml @@ -0,0 +1,27 @@ +<domain type='openvz'> + <name>100</name> + <uuid>7109d234-f5a8-30a6-5dd2-39ca85ce3958</uuid> + <memory unit='KiB'>0</memory> + <currentMemory unit='KiB'>0</currentMemory> + <memtune> + <hard_limit unit='KiB'>278528</hard_limit> + <soft_limit unit='KiB'>262144</soft_limit> + <min_guarantee unit='KiB'>135168</min_guarantee> + </memtune> + <vcpu>1</vcpu> + <os> + <type>exe</type> + <init>/sbin/init</init> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>destroy</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <filesystem type='template' accessmode='passthrough'> + <source name='debian'/> + <target dir='/'/> + </filesystem> + </devices> +</domain> + -- 1.7.10
On 04/26/2012 05:16 AM, Guido Günther wrote:
--- This is a reworked version of the patch already sent, now adding handling for "unlimited" (represented by LONG_MAX in openvz) as well as a schema testcase. I also switched all value to unsigned long long to avoid overflows. Note that privvmpages is the amount of memory available to the application. The total memory usage of the container might be higher due to used kernel memory. This does probably warrant an extra tunable later. O.k. to apply? Cheers, -- Guido
+static int +openvzReadMemConf(virDomainDefPtr def, int veid) +{ + int ret; + char *temp = NULL; + unsigned long long barrier, limit; + const char *param; + unsigned long kb_per_pages; + + kb_per_pages = sysconf(_SC_PAGESIZE) / 1024;
In general, you should be prepared for sysconf() to fail...
+ if (kb_per_pages == -1) {
but you already divided by 1024, so failure will no longer show as -1 at this point.
+ + if (sscanf(output, "%llu %llu", barrier, limit) != 2) {
sscanf() can't detect overflow. I'd rather use virStrToLong_ull() for safety.
+ +static int +openvzDomainGetMemoryParameters(virDomainPtr domain, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + int i, result = -1; + const char *name; + long kb_per_pages; + unsigned long long barrier, limit, val; + + virCheckFlags(0, -1); + + kb_per_pages = sysconf(_SC_PAGESIZE) / 1024; + if (kb_per_pages == -1) {
Oops, another inability to detect failure.
+ +static int +openvzDomainSetMemoryParameters(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + int i, result = -1; + long kb_per_pages; + + kb_per_pages = sysconf(_SC_PAGESIZE) / 1024; + if (kb_per_pages == -1) {
And again. But the bulk of this patch looks reasonable. Would you mind respinning a v2 to address the nits, and we can probably get this in 0.9.12 even if it misses rc1. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- Changes since last time: * check sysconf return value before division * use virStrToLong_ull instead of sscanf O.k. to apply? Cheers, -- Guido src/openvz/openvz_conf.c | 115 ++++++++++++ src/openvz/openvz_driver.c | 222 +++++++++++++++++++++++ tests/domainschemadata/domain-openvz-simple.xml | 27 +++ 3 files changed, 364 insertions(+) create mode 100644 tests/domainschemadata/domain-openvz-simple.xml diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 5848ec4..3806835 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -129,6 +129,46 @@ int openvzExtractVersion(struct openvz_driver *driver) } +/* Parse config values of the form barrier:limit into barrier and limit */ +static int +openvzParseBarrierLimit(const char* value, + unsigned long long *barrier, + unsigned long long *limit) +{ + char *token; + char *saveptr = NULL; + char *str = strdup(value); + + if (str == NULL) { + virReportOOMError(); + goto error; + } + + token = strtok_r(str, ":", &saveptr); + if (token == NULL) { + goto error; + } else { + if (barrier != NULL) { + if (virStrToLong_ull(token, NULL, 10, barrier)) + goto error; + } + } + token = strtok_r(NULL, ":", &saveptr); + if (token == NULL) { + goto error; + } else { + if (limit != NULL) { + if (virStrToLong_ull(token, NULL, 10, limit)) + goto error; + } + } + return 0; +error: + VIR_FREE(str); + return -1; +} + + static int openvzDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) { return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ; @@ -423,6 +463,80 @@ error: } +static int +openvzReadMemConf(virDomainDefPtr def, int veid) +{ + int ret; + char *temp = NULL; + unsigned long long barrier, limit; + const char *param; + unsigned long kb_per_pages; + + kb_per_pages = sysconf(_SC_PAGESIZE); + if (kb_per_pages > 0) { + kb_per_pages /= 1024; + } else { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + goto error; + } + + /* Memory allocation guarantee */ + param = "VMGUARPAGES"; + ret = openvzReadVPSConfigParam(veid, param, &temp); + if (ret < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Could not read '%s' from config for container %d"), + param, veid); + goto error; + } else if (ret > 0) { + ret = openvzParseBarrierLimit(temp, &barrier, NULL); + if (ret < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse barrier of '%s' " + "from config for container %d"), param, veid); + goto error; + } + if (barrier == LONG_MAX) + def->mem.min_guarantee = 0ull; + else + def->mem.min_guarantee = barrier * kb_per_pages; + } + + /* Memory hard and soft limits */ + param = "PRIVVMPAGES"; + ret = openvzReadVPSConfigParam(veid, param, &temp); + if (ret < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Could not read '%s' from config for container %d"), + param, veid); + goto error; + } else if (ret > 0) { + ret = openvzParseBarrierLimit(temp, &barrier, &limit); + if (ret < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse barrier and limit of '%s' " + "from config for container %d"), param, veid); + goto error; + } + if (barrier == LONG_MAX) + def->mem.soft_limit = 0ull; + else + def->mem.soft_limit = barrier * kb_per_pages; + + if (limit == LONG_MAX) + def->mem.hard_limit = 0ull; + else + def->mem.hard_limit = limit * kb_per_pages; + } + + ret = 0; +error: + VIR_FREE(temp); + return ret; +} + + /* Free all memory associated with a openvz_driver structure */ void openvzFreeDriver(struct openvz_driver *driver) @@ -535,6 +649,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { openvzReadNetworkConf(dom->def, veid); openvzReadFSConf(dom->def, veid); + openvzReadMemConf(dom->def, veid); virUUIDFormat(dom->def->uuid, uuidstr); if (virHashAddEntry(driver->domains.objs, uuidstr, dom) < 0) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index e8b6915..91f5d49 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -54,6 +54,7 @@ #include "nodeinfo.h" #include "memory.h" #include "virfile.h" +#include "virtypedparam.h" #include "logging.h" #include "command.h" #include "viruri.h" @@ -65,6 +66,8 @@ #define CMDBUF_LEN 1488 #define CMDOP_LEN 288 +#define OPENVZ_NB_MEM_PARAM 3 + static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid); static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type); static int openvzDomainGetMaxVcpus(virDomainPtr dom); @@ -1631,6 +1634,223 @@ cleanup: return -1; } + +static int +openvzDomainGetBarrierLimit(virDomainPtr domain, + const char *param, + unsigned long long *barrier, + unsigned long long *limit) +{ + int status, ret = -1; + char *endp, *output = NULL; + const char *tmp; + virCommandPtr cmd = virCommandNewArgList(VZLIST, "--no-header", NULL); + + virCommandSetOutputBuffer(cmd, &output); + virCommandAddArgFormat(cmd, "-o%s.b,%s.l", param, param); + virCommandAddArg(cmd, domain->name); + if (virCommandRun(cmd, &status)) { + openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to get %s for %s: %d"), param, domain->name, + status); + goto cleanup; + } + + tmp = output; + virSkipSpaces(&tmp); + if (virStrToLong_ull(tmp, &endp, 10, barrier) < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't parse limit from "VZLIST" output '%s'"), output); + goto cleanup; + } + tmp = endp; + virSkipSpaces(&tmp); + if (virStrToLong_ull(tmp, &endp, 10, limit) < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't parse barrier from "VZLIST" output '%s'"), output); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(output); + virCommandFree(cmd); + return ret; +} + + +static int +openvzDomainSetBarrierLimit(virDomainPtr domain, + const char *param, + unsigned long long barrier, + unsigned long long limit) +{ + int status, ret = -1; + virCommandPtr cmd = virCommandNewArgList(VZCTL, "--quiet", "set", NULL); + + /* LONG_MAX indicates unlimited so reject larger values */ + if (barrier > LONG_MAX || limit > LONG_MAX) { + openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to set %s for %s: value too large"), param, + domain->name); + goto cleanup; + } + + virCommandAddArg(cmd, domain->name); + virCommandAddArgFormat(cmd, "--%s", param); + virCommandAddArgFormat(cmd, "%llu:%llu", barrier, limit); + virCommandAddArg(cmd, "--save"); + if (virCommandRun(cmd, &status)) { + openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to set %s for %s: %d"), param, domain->name, + status); + goto cleanup; + } + + ret = 0; +cleanup: + virCommandFree(cmd); + return ret; +} + + +static int +openvzDomainGetMemoryParameters(virDomainPtr domain, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + int i, result = -1; + const char *name; + long kb_per_pages; + unsigned long long barrier, limit, val; + + virCheckFlags(0, -1); + + kb_per_pages = sysconf(_SC_PAGESIZE); + if (kb_per_pages > 0) { + kb_per_pages /= 1024; + } else { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + goto cleanup; + } + + if (*nparams == 0) { + *nparams = OPENVZ_NB_MEM_PARAM; + return 0; + } + + for (i = 0; i <= *nparams; i++) { + virMemoryParameterPtr param = ¶ms[i]; + + switch (i) { + case 0: + name = "privvmpages"; + if (openvzDomainGetBarrierLimit(domain, name, &barrier, &limit) < 0) + goto cleanup; + + val = (limit == LONG_MAX) ? 0ull : limit * kb_per_pages; + if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) + goto cleanup; + break; + + case 1: + name = "privvmpages"; + if (openvzDomainGetBarrierLimit(domain, name, &barrier, &limit) < 0) + goto cleanup; + + val = (barrier == LONG_MAX) ? 0ull : barrier * kb_per_pages; + if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) + goto cleanup; + break; + + case 2: + name = "vmguarpages"; + if (openvzDomainGetBarrierLimit(domain, name, &barrier, &limit) < 0) + goto cleanup; + + val = (barrier == LONG_MAX) ? 0ull : barrier * kb_per_pages; + if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_MIN_GUARANTEE, + VIR_TYPED_PARAM_ULLONG, val) < 0) + goto cleanup; + break; + } + } + + if (*nparams > OPENVZ_NB_MEM_PARAM) + *nparams = OPENVZ_NB_MEM_PARAM; + result = 0; + +cleanup: + return result; +} + + +static int +openvzDomainSetMemoryParameters(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + int i, result = -1; + long kb_per_pages; + + kb_per_pages = sysconf(_SC_PAGESIZE); + if (kb_per_pages > 0) { + kb_per_pages /= 1024; + } else { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + goto cleanup; + } + + virCheckFlags(0, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_MEMORY_MIN_GUARANTEE, + VIR_TYPED_PARAM_ULLONG, + NULL) < 0) + return -1; + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + unsigned long long barrier, limit; + + if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) { + if (openvzDomainGetBarrierLimit(domain, "privvmpages", + &barrier, &limit) < 0) + goto cleanup; + limit = params[i].value.ul / kb_per_pages; + if (openvzDomainSetBarrierLimit(domain, "privvmpages", + barrier, limit) < 0) + goto cleanup; + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { + if (openvzDomainGetBarrierLimit(domain, "privvmpages", + &barrier, &limit) < 0) + goto cleanup; + barrier = params[i].value.ul / kb_per_pages; + if (openvzDomainSetBarrierLimit(domain, "privvmpages", + barrier, limit) < 0) + goto cleanup; + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { + barrier = params[i].value.ul / kb_per_pages; + if (openvzDomainSetBarrierLimit(domain, "vmguarpages", + barrier, LONG_MAX) < 0) + goto cleanup; + } + } + result = 0; +cleanup: + return result; +} + + static int openvzGetVEStatus(virDomainObjPtr vm, int *status, int *reason) { @@ -1752,6 +1972,8 @@ static virDriver openvzDriver = { .domainDestroy = openvzDomainShutdown, /* 0.3.1 */ .domainDestroyFlags = openvzDomainShutdownFlags, /* 0.9.4 */ .domainGetOSType = openvzGetOSType, /* 0.3.1 */ + .domainGetMemoryParameters = openvzDomainGetMemoryParameters, /* 0.9.12 */ + .domainSetMemoryParameters = openvzDomainSetMemoryParameters, /* 0.9.12 */ .domainGetInfo = openvzDomainGetInfo, /* 0.3.1 */ .domainGetState = openvzDomainGetState, /* 0.9.2 */ .domainSetVcpus = openvzDomainSetVcpus, /* 0.4.6 */ diff --git a/tests/domainschemadata/domain-openvz-simple.xml b/tests/domainschemadata/domain-openvz-simple.xml new file mode 100644 index 0000000..aba64a4 --- /dev/null +++ b/tests/domainschemadata/domain-openvz-simple.xml @@ -0,0 +1,27 @@ +<domain type='openvz'> + <name>100</name> + <uuid>7109d234-f5a8-30a6-5dd2-39ca85ce3958</uuid> + <memory unit='KiB'>0</memory> + <currentMemory unit='KiB'>0</currentMemory> + <memtune> + <hard_limit unit='KiB'>278528</hard_limit> + <soft_limit unit='KiB'>262144</soft_limit> + <min_guarantee unit='KiB'>135168</min_guarantee> + </memtune> + <vcpu>1</vcpu> + <os> + <type>exe</type> + <init>/sbin/init</init> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>destroy</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <filesystem type='template' accessmode='passthrough'> + <source name='debian'/> + <target dir='/'/> + </filesystem> + </devices> +</domain> + -- 1.7.10
On 05/03/2012 11:13 AM, Guido Günther wrote:
--- Changes since last time: * check sysconf return value before division * use virStrToLong_ull instead of sscanf
O.k. to apply? Cheers, -- Guido
ACK.
+ long kb_per_pages; + unsigned long long barrier, limit, val; + + virCheckFlags(0, -1); + + kb_per_pages = sysconf(_SC_PAGESIZE);
A minor optimization is possible, since sysconf() won't change: static long kb_per_pages; if (!kb_per_pages) { kb_per_pages = sysconf(_SC_PAGESIZE): if (kb_per_pages > 0) kb_per_pages /= 1024; } if (kb_per_pages <= 0) error... for fewer syscalls over the life of the program. But what you have is correct, and not worth a respin just for that tweak, so feel free to push as-is if you'd like. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, May 03, 2012 at 02:37:00PM -0600, Eric Blake wrote:
On 05/03/2012 11:13 AM, Guido Günther wrote:
--- Changes since last time: * check sysconf return value before division * use virStrToLong_ull instead of sscanf
O.k. to apply? Cheers, -- Guido
ACK.
+ long kb_per_pages; + unsigned long long barrier, limit, val; + + virCheckFlags(0, -1); + + kb_per_pages = sysconf(_SC_PAGESIZE);
A minor optimization is possible, since sysconf() won't change:
static long kb_per_pages; if (!kb_per_pages) { kb_per_pages = sysconf(_SC_PAGESIZE): if (kb_per_pages > 0) kb_per_pages /= 1024; } if (kb_per_pages <= 0) error...
for fewer syscalls over the life of the program. But what you have is correct, and not worth a respin just for that tweak, so feel free to push as-is if you'd like. Pushed now as is (after correcting one "make syntax-check" failure, I'll change the code to the above in a follow up patch. Thanks! Cheers, -- Guido
to safe some syscalls (as suggested by Eric Blake) --- I've moved the code into an extra module since there's some more code to come that should be shared between openvz_conf.c and openvz_driver.c Cheers, -- Guido src/Makefile.am | 3 ++- src/openvz/openvz_conf.c | 12 ++++------- src/openvz/openvz_driver.c | 19 +++++------------ src/openvz/openvz_util.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ src/openvz/openvz_util.h | 28 ++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 src/openvz/openvz_util.c create mode 100644 src/openvz/openvz_util.h diff --git a/src/Makefile.am b/src/Makefile.am index 0dadc29..2ecd188 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -353,7 +353,8 @@ PHYP_DRIVER_SOURCES = \ OPENVZ_DRIVER_SOURCES = \ openvz/openvz_conf.c openvz/openvz_conf.h \ - openvz/openvz_driver.c openvz/openvz_driver.h + openvz/openvz_driver.c openvz/openvz_driver.h \ + openvz/openvz_util.c openvz/openvz_util.h VMWARE_DRIVER_SOURCES = \ vmware/vmware_driver.c vmware/vmware_driver.h \ diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index f691ae7..fedd6a8 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -45,6 +45,7 @@ #include "virterror_internal.h" #include "openvz_conf.h" +#include "openvz_util.h" #include "uuid.h" #include "buf.h" #include "memory.h" @@ -499,16 +500,11 @@ openvzReadMemConf(virDomainDefPtr def, int veid) char *temp = NULL; unsigned long long barrier, limit; const char *param; - unsigned long kb_per_pages; + long kb_per_pages; - kb_per_pages = sysconf(_SC_PAGESIZE); - if (kb_per_pages > 0) { - kb_per_pages /= 1024; - } else { - openvzError(VIR_ERR_INTERNAL_ERROR, - _("Can't determine page size")); + kb_per_pages = openvzKBPerPages(); + if (kb_per_pages < 0) goto error; - } /* Memory allocation guarantee */ param = "VMGUARPAGES"; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 2661d60..45ab262 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -48,6 +48,7 @@ #include "virterror_internal.h" #include "datatypes.h" #include "openvz_driver.h" +#include "openvz_util.h" #include "buf.h" #include "util.h" #include "openvz_conf.h" @@ -1735,14 +1736,9 @@ openvzDomainGetMemoryParameters(virDomainPtr domain, virCheckFlags(0, -1); - kb_per_pages = sysconf(_SC_PAGESIZE); - if (kb_per_pages > 0) { - kb_per_pages /= 1024; - } else { - openvzError(VIR_ERR_INTERNAL_ERROR, - _("Can't determine page size")); + kb_per_pages = openvzKBPerPages(); + if (kb_per_pages < 0) goto cleanup; - } if (*nparams == 0) { *nparams = OPENVZ_NB_MEM_PARAM; @@ -1806,14 +1802,9 @@ openvzDomainSetMemoryParameters(virDomainPtr domain, int i, result = -1; long kb_per_pages; - kb_per_pages = sysconf(_SC_PAGESIZE); - if (kb_per_pages > 0) { - kb_per_pages /= 1024; - } else { - openvzError(VIR_ERR_INTERNAL_ERROR, - _("Can't determine page size")); + kb_per_pages = openvzKBPerPages(); + if (kb_per_pages < 0) goto cleanup; - } virCheckFlags(0, -1); if (virTypedParameterArrayValidate(params, nparams, diff --git a/src/openvz/openvz_util.c b/src/openvz/openvz_util.c new file mode 100644 index 0000000..ecb7a42 --- /dev/null +++ b/src/openvz/openvz_util.c @@ -0,0 +1,51 @@ +/* + * openvz_driver.c: core driver methods for managing OpenVZ VEs + * + * Copyright (C) Guido Günther + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <config.h> + +#include <unistd.h> + +#include "internal.h" + +#include "virterror_internal.h" + +#include "openvz_conf.h" +#include "openvz_util.h" + + +long +openvzKBPerPages(void) +{ + static long kb_per_pages = 0; + + if (kb_per_pages == 0) { + kb_per_pages = sysconf(_SC_PAGESIZE); + if (kb_per_pages > 0) { + kb_per_pages /= 1024; + } else { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + kb_per_pages = 0; + return -1; + } + } + return kb_per_pages; +} diff --git a/src/openvz/openvz_util.h b/src/openvz/openvz_util.h new file mode 100644 index 0000000..a0d9bbb --- /dev/null +++ b/src/openvz/openvz_util.h @@ -0,0 +1,28 @@ +/* + * openvz_driver.h: common util functions for managing openvz VPEs + * + * Copyright (C) 2012 Guido Günther + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + + +#ifndef OPENVZ_UTIL_H +# define OPENVZ_UTIL_H + +long openvzKBPerPages(void); + +#endif -- 1.7.10
On 05/08/2012 02:12 PM, Guido Günther wrote:
to safe some syscalls (as suggested by Eric Blake)
s/safe/save/
--- I've moved the code into an extra module since there's some more code to come that should be shared between openvz_conf.c and openvz_driver.c Cheers, -- Guido
src/Makefile.am | 3 ++- src/openvz/openvz_conf.c | 12 ++++------- src/openvz/openvz_driver.c | 19 +++++------------ src/openvz/openvz_util.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ src/openvz/openvz_util.h | 28 ++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 src/openvz/openvz_util.c create mode 100644 src/openvz/openvz_util.h
This is quite a bit of churn for a micro-optimization, since it is not fixing any actual bugs. I'll review the code, but let's avoid applying it until after 0.9.12 is released.
+++ b/src/openvz/openvz_util.c @@ -0,0 +1,51 @@ +/* + * openvz_driver.c: core driver methods for managing OpenVZ VEs + * + * Copyright (C) Guido Günther
Year?
+ static long kb_per_pages = 0; + + if (kb_per_pages == 0) { + kb_per_pages = sysconf(_SC_PAGESIZE); + if (kb_per_pages > 0) { + kb_per_pages /= 1024; + } else { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + kb_per_pages = 0; + return -1;
This says that in an error situation, every caller will repeat the sysconf() call and issue a new error. Seems reasonable. ACK with copyright nit fixed, post-release. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, May 10, 2012 at 04:54:59PM -0600, Eric Blake wrote:
On 05/08/2012 02:12 PM, Guido Günther wrote:
to safe some syscalls (as suggested by Eric Blake)
s/safe/save/
--- I've moved the code into an extra module since there's some more code to come that should be shared between openvz_conf.c and openvz_driver.c Cheers, -- Guido
src/Makefile.am | 3 ++- src/openvz/openvz_conf.c | 12 ++++------- src/openvz/openvz_driver.c | 19 +++++------------ src/openvz/openvz_util.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ src/openvz/openvz_util.h | 28 ++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 src/openvz/openvz_util.c create mode 100644 src/openvz/openvz_util.h
This is quite a bit of churn for a micro-optimization, since it is not fixing any actual bugs. I'll review the code, but let's avoid applying it until after 0.9.12 is released.
+++ b/src/openvz/openvz_util.c @@ -0,0 +1,51 @@ +/* + * openvz_driver.c: core driver methods for managing OpenVZ VEs + * + * Copyright (C) Guido Günther
Year?
+ static long kb_per_pages = 0; + + if (kb_per_pages == 0) { + kb_per_pages = sysconf(_SC_PAGESIZE); + if (kb_per_pages > 0) { + kb_per_pages /= 1024; + } else { + openvzError(VIR_ERR_INTERNAL_ERROR, + _("Can't determine page size")); + kb_per_pages = 0; + return -1;
This says that in an error situation, every caller will repeat the sysconf() call and issue a new error. Seems reasonable.
ACK with copyright nit fixed, post-release.
Pushed now that 0.9.12 is out. Thanks! -- Guido
participants (2)
-
Eric Blake -
Guido Günther