[libvirt] [PATCH 0/3] openvz: allow for runtime quota updates

Hi, attached patches allow to change disk quota at runtime. This can later be extended to update network configuration as well. The patch revealed a problem of the openvz driver in genral: the openvzLoadDomains only reads the on disk configuration but doesn't check if running domains have (non persistent) diverging values due to openvz calls without the "--save" option but that's part of another patch. Cheers, -- Guido Guido Günther (3): Introduce virDomainFSIndexByName openvz: add persist parameter to openvzSetDiskQuota openvz: wire up domainUpdateDeviceFlags src/conf/domain_conf.c | 16 +++++++ src/conf/domain_conf.h | 1 + src/openvz/openvz_driver.c | 105 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 118 insertions(+), 4 deletions(-) -- 1.7.10

for containers matching virDomainDiskIndexByName. --- src/conf/domain_conf.c | 16 ++++++++++++++++ src/conf/domain_conf.h | 1 + 2 files changed, 17 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd7b520..c34ce26 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11265,6 +11265,22 @@ virDomainControllerDefFormat(virBufferPtr buf, return 0; } + +int +virDomainFSIndexByName(virDomainDefPtr def, const char *name) +{ + virDomainFSDefPtr fs; + int i; + + for (i = 0; i < def->nfss; i++) { + fs = def->fss[i]; + if (STREQ(fs->dst, name)) + return i; + } + return -1; +} + + static int virDomainFSDefFormat(virBufferPtr buf, virDomainFSDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7642720..e2f56fb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2087,6 +2087,7 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk, int *devIdx); virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def); +int virDomainFSIndexByName(virDomainDefPtr def, const char *name); int virDomainVideoDefaultType(virDomainDefPtr def); int virDomainVideoDefaultRAM(virDomainDefPtr def, int type); -- 1.7.10

On 03.06.2012 22:55, Guido Günther wrote:
for containers matching virDomainDiskIndexByName. --- src/conf/domain_conf.c | 16 ++++++++++++++++ src/conf/domain_conf.h | 1 + 2 files changed, 17 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd7b520..c34ce26 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11265,6 +11265,22 @@ virDomainControllerDefFormat(virBufferPtr buf, return 0; }
+ +int +virDomainFSIndexByName(virDomainDefPtr def, const char *name) +{ + virDomainFSDefPtr fs; + int i; + + for (i = 0; i < def->nfss; i++) { + fs = def->fss[i]; + if (STREQ(fs->dst, name)) + return i; + } + return -1; +} + + static int virDomainFSDefFormat(virBufferPtr buf, virDomainFSDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7642720..e2f56fb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2087,6 +2087,7 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk, int *devIdx);
virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def); +int virDomainFSIndexByName(virDomainDefPtr def, const char *name); int virDomainVideoDefaultType(virDomainDefPtr def); int virDomainVideoDefaultRAM(virDomainDefPtr def, int type);
ACK with adding virDomainFSIndexByName into libvirt_private.syms: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fdf2186..a452e62 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -319,6 +319,7 @@ virDomainDiskSnapshotTypeToString; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainFSDefFree; +virDomainFSIndexByName; virDomainFSTypeFromString; virDomainFSTypeToString; virDomainFSWrpolicyTypeFromString;

with persist=false the domain config file will not be updated. --- src/openvz/openvz_driver.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index fb72cde..02a0133 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -191,7 +191,8 @@ cleanup: static int openvzSetDiskQuota(virDomainDefPtr vmdef, - virDomainFSDefPtr fss) + virDomainFSDefPtr fss, + bool persist) { int ret = -1; unsigned long long sl, hl; @@ -199,8 +200,9 @@ openvzSetDiskQuota(virDomainDefPtr vmdef, "--quiet", "set", vmdef->name, - "--save", NULL); + if (persist) + virCommandAddArg(cmd, "--save"); if (fss->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) { if (fss->space_hard_limit) { @@ -938,7 +940,7 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml) } if (vm->def->nfss == 1) { - if (openvzSetDiskQuota(vm->def, vm->def->fss[0]) < 0) { + if (openvzSetDiskQuota(vm->def, vm->def->fss[0], true) < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set disk quota")); goto cleanup; @@ -1026,7 +1028,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, } if (vm->def->nfss == 1) { - if (openvzSetDiskQuota(vm->def, vm->def->fss[0]) < 0) { + if (openvzSetDiskQuota(vm->def, vm->def->fss[0], true) < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set disk quota")); goto cleanup; -- 1.7.10

On 03.06.2012 22:55, Guido Günther wrote:
with persist=false the domain config file will not be updated. --- src/openvz/openvz_driver.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index fb72cde..02a0133 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -191,7 +191,8 @@ cleanup:
static int openvzSetDiskQuota(virDomainDefPtr vmdef, - virDomainFSDefPtr fss) + virDomainFSDefPtr fss, + bool persist) { int ret = -1; unsigned long long sl, hl; @@ -199,8 +200,9 @@ openvzSetDiskQuota(virDomainDefPtr vmdef, "--quiet", "set", vmdef->name, - "--save", NULL); + if (persist) + virCommandAddArg(cmd, "--save");
if (fss->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) { if (fss->space_hard_limit) { @@ -938,7 +940,7 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml) }
if (vm->def->nfss == 1) { - if (openvzSetDiskQuota(vm->def, vm->def->fss[0]) < 0) { + if (openvzSetDiskQuota(vm->def, vm->def->fss[0], true) < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set disk quota")); goto cleanup; @@ -1026,7 +1028,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, }
if (vm->def->nfss == 1) { - if (openvzSetDiskQuota(vm->def, vm->def->fss[0]) < 0) { + if (openvzSetDiskQuota(vm->def, vm->def->fss[0], true) < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set disk quota")); goto cleanup;
ACK Michal

so we can update file system quota --- src/openvz/openvz_driver.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 02a0133..8a1fb9a 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1948,6 +1948,100 @@ cleanup: } +static int +openvzUpdateDevice(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + bool persist) +{ + virDomainFSDefPtr fs; + int pos; + + if (dev->type == VIR_DOMAIN_DEVICE_FS) { + fs = dev->data.fs; + pos = virDomainFSIndexByName(vmdef, fs->dst); + + if (pos < 0) { + openvzError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), fs->dst); + return -1; + } + + /* We only allow updating the quota */ + if (openvzSetDiskQuota(vmdef, dev->data.fs, persist) < 0) { + return -1; + } + vmdef->fss[pos]->space_hard_limit = fs->space_hard_limit; + vmdef->fss[pos]->space_soft_limit = fs->space_soft_limit; + } else { + openvzError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Can't modify device type '%s'"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + + return 0; +} + + +static int +openvzDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + int ret = -1; + int veid; + struct openvz_driver *driver = dom->conn->privateData; + virDomainDeviceDefPtr dev = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr vmdef = NULL; + bool persist = false; + + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + + openvzDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + vmdef = vm->def; + + if (!vm) { + openvzError(VIR_ERR_NO_DOMAIN, "%s", + _("no domain with matching uuid")); + goto cleanup; + } + + if (virStrToLong_i(vmdef->name, NULL, 10, &veid) < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not convert domain name to VEID")); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, + vm, + &flags, + &vmdef) < 0) + goto cleanup; + + dev = virDomainDeviceDefParse(driver->caps, vmdef, xml, + VIR_DOMAIN_XML_INACTIVE); + if (!dev) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) + persist = true; + + if (openvzUpdateDevice(vmdef, dev, persist) < 0) + goto cleanup; + + ret = 0; + +cleanup: + openvzDriverUnlock(driver); + virDomainDeviceDefFree(dev); + if (vm) + virDomainObjUnlock(vm); + return ret; +} + + static virDriver openvzDriver = { .no = VIR_DRV_OPENVZ, .name = "OPENVZ", @@ -2002,6 +2096,7 @@ static virDriver openvzDriver = { .domainIsPersistent = openvzDomainIsPersistent, /* 0.7.3 */ .domainIsUpdated = openvzDomainIsUpdated, /* 0.8.6 */ .isAlive = openvzIsAlive, /* 0.9.8 */ + .domainUpdateDeviceFlags = openvzDomainUpdateDeviceFlags, /* 0.9.14 */ }; int openvzRegister(void) { -- 1.7.10

On 03.06.2012 22:55, Guido Günther wrote:
so we can update file system quota --- src/openvz/openvz_driver.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 02a0133..8a1fb9a 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1948,6 +1948,100 @@ cleanup: }
+static int +openvzUpdateDevice(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + bool persist) +{ + virDomainFSDefPtr fs; + int pos; + + if (dev->type == VIR_DOMAIN_DEVICE_FS) { + fs = dev->data.fs; + pos = virDomainFSIndexByName(vmdef, fs->dst); + + if (pos < 0) { + openvzError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), fs->dst); + return -1; + } + + /* We only allow updating the quota */
so shouldn't we shout if user tries changing something else like wrpolicy or something?
+ if (openvzSetDiskQuota(vmdef, dev->data.fs, persist) < 0) { + return -1; + } + vmdef->fss[pos]->space_hard_limit = fs->space_hard_limit; + vmdef->fss[pos]->space_soft_limit = fs->space_soft_limit; + } else { + openvzError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Can't modify device type '%s'"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + + return 0; +} + + +static int +openvzDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + int ret = -1; + int veid; + struct openvz_driver *driver = dom->conn->privateData; + virDomainDeviceDefPtr dev = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr vmdef = NULL; + bool persist = false; + + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + + openvzDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + vmdef = vm->def; + + if (!vm) { + openvzError(VIR_ERR_NO_DOMAIN, "%s", + _("no domain with matching uuid")); + goto cleanup; + } + + if (virStrToLong_i(vmdef->name, NULL, 10, &veid) < 0) { + openvzError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not convert domain name to VEID")); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, + vm, + &flags, + &vmdef) < 0) + goto cleanup; + + dev = virDomainDeviceDefParse(driver->caps, vmdef, xml, + VIR_DOMAIN_XML_INACTIVE); + if (!dev) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) + persist = true; + + if (openvzUpdateDevice(vmdef, dev, persist) < 0) + goto cleanup; + + ret = 0; + +cleanup: + openvzDriverUnlock(driver); + virDomainDeviceDefFree(dev); + if (vm) + virDomainObjUnlock(vm); + return ret; +} + + static virDriver openvzDriver = { .no = VIR_DRV_OPENVZ, .name = "OPENVZ", @@ -2002,6 +2096,7 @@ static virDriver openvzDriver = { .domainIsPersistent = openvzDomainIsPersistent, /* 0.7.3 */ .domainIsUpdated = openvzDomainIsUpdated, /* 0.8.6 */ .isAlive = openvzIsAlive, /* 0.9.8 */ + .domainUpdateDeviceFlags = openvzDomainUpdateDeviceFlags, /* 0.9.14 */ };
int openvzRegister(void) {
Otherwise looking good. Michal
participants (2)
-
Guido Günther
-
Michal Privoznik