[libvirt] [PATCH v3 0/2] vz: implement memory setting functions in driver

Changes from v2: 1. factor out common code first and reuse utility code to resolve 'current' flag. 2. drop supporting 'maximum' flag Nikolay Shirokovskiy (2): vz: factor out config update flags checks vz: implement memory setting functions src/vz/vz_driver.c | 98 +++++++++++++++++++++++++++++++++--------------------- src/vz/vz_sdk.c | 24 +++++++++++++ src/vz/vz_sdk.h | 2 ++ 3 files changed, 86 insertions(+), 38 deletions(-) -- 1.8.3.1

Actually this is not pure refactoring. Part of common code is replaced with virDomainObjUpdateModificationImpact and this a good replacement. It includes removed check of inactive domain and active flags set. Additionally we resolve current flag in accordance with current state of domain. Thus it becames possible to attach/detach devices for inactive domains if this flag is set. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_driver.c | 62 +++++++++++++++++++++--------------------------------- 1 file changed, 24 insertions(+), 38 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index e12a95a..08447d9 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1037,6 +1037,28 @@ vzDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) return ret; } +static int vzCheckConfigUpdateFlags(virDomainObjPtr dom, unsigned int *flags) +{ + if (virDomainObjUpdateModificationImpact(dom, flags) < 0) + return -1; + + if (!(*flags & VIR_DOMAIN_AFFECT_CONFIG)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain config update needs VIR_DOMAIN_AFFECT_CONFIG " + "flag to be set")); + return -1; + } + + if (virDomainObjIsActive(dom) && !(*flags & VIR_DOMAIN_AFFECT_LIVE)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Updates on a running domain need " + "VIR_DOMAIN_AFFECT_LIVE flag")); + return -1; + } + + return 0; +} + static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { @@ -1044,7 +1066,6 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, vzConnPtr privconn = dom->conn->privateData; virDomainDeviceDefPtr dev = NULL; virDomainObjPtr privdom = NULL; - bool domactive = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -1052,25 +1073,8 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!(privdom = vzDomObjFromDomain(dom))) return -1; - if (!(flags & VIR_DOMAIN_AFFECT_CONFIG)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("device attach needs VIR_DOMAIN_AFFECT_CONFIG " - "flag to be set")); + if (vzCheckConfigUpdateFlags(privdom, &flags) < 0) goto cleanup; - } - - domactive = virDomainObjIsActive(privdom); - if (!domactive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot do live update a device on " - "inactive domain")); - goto cleanup; - } - if (domactive && !(flags & VIR_DOMAIN_AFFECT_LIVE)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Updates on a running domain need " - "VIR_DOMAIN_AFFECT_LIVE flag")); - } dev = virDomainDeviceDefParse(xml, privdom->def, privconn->caps, privconn->xmlopt, VIR_DOMAIN_XML_INACTIVE); @@ -1120,7 +1124,6 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, vzConnPtr privconn = dom->conn->privateData; virDomainDeviceDefPtr dev = NULL; virDomainObjPtr privdom = NULL; - bool domactive = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -1129,25 +1132,8 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (privdom == NULL) return -1; - if (!(flags & VIR_DOMAIN_AFFECT_CONFIG)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("device detach needs VIR_DOMAIN_AFFECT_CONFIG " - "flag to be set")); + if (vzCheckConfigUpdateFlags(privdom, &flags) < 0) goto cleanup; - } - - domactive = virDomainObjIsActive(privdom); - if (!domactive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot do live update a device on " - "inactive domain")); - goto cleanup; - } - if (domactive && !(flags & VIR_DOMAIN_AFFECT_LIVE)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Updates on a running domain need " - "VIR_DOMAIN_AFFECT_LIVE flag")); - } dev = virDomainDeviceDefParse(xml, privdom->def, privconn->caps, privconn->xmlopt, VIR_DOMAIN_XML_INACTIVE); -- 1.8.3.1

08.04.2016 12:36, Nikolay Shirokovskiy пишет:
Actually this is not pure refactoring. Part of common code is replaced with virDomainObjUpdateModificationImpact and this a good replacement. It includes removed check of inactive domain and active flags set. Additionally we resolve current flag in accordance with current state of domain. Thus it becames possible to attach/detach devices for inactive domains if this flag is set.
ACK

Quite straigthforward as vz sdk memory setting function makes just what we want to that is set "amount of physical memory allocated to a domain". 'useflags' is introduced for non flag function implementation. We can't just use combination of flags like "live | config" or we fail for inactive domains. Other combinations have drawbacks too. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_driver.c | 36 ++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 24 ++++++++++++++++++++++++ src/vz/vz_sdk.h | 2 ++ 3 files changed, 62 insertions(+) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 08447d9..85f17f7 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1479,6 +1479,40 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb) return ret; } +static int vzDomainSetMemoryFlagsImpl(virDomainPtr domain, unsigned long memory, + unsigned int flags, bool useflags) +{ + virDomainObjPtr dom = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!(dom = vzDomObjFromDomain(domain))) + return -1; + + if (useflags && vzCheckConfigUpdateFlags(dom, &flags) < 0) + goto cleanup; + + ret = prlsdkSetMemsize(dom, memory >> 10); + + cleanup: + + virObjectUnlock(dom); + return ret; +} + +static int vzDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, + unsigned int flags) +{ + return vzDomainSetMemoryFlagsImpl(domain, memory, flags, true); +} + +static int vzDomainSetMemory(virDomainPtr domain, unsigned long memory) +{ + return vzDomainSetMemoryFlagsImpl(domain, memory, 0, false); +} + static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ @@ -1543,6 +1577,8 @@ static virHypervisorDriver vzDriver = { .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ .connectRegisterCloseCallback = vzConnectRegisterCloseCallback, /* 1.3.2 */ .connectUnregisterCloseCallback = vzConnectUnregisterCloseCallback, /* 1.3.2 */ + .domainSetMemoryFlags = vzDomainSetMemoryFlags, /* 1.3.3 */ + .domainSetMemory = vzDomainSetMemory, /* 1.3.3 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index c006517..85ea0d7 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4302,3 +4302,27 @@ prlsdkGetMemoryStats(virDomainObjPtr dom, return ret; } + +/* memsize is in MiB */ +int prlsdkSetMemsize(virDomainObjPtr dom, unsigned int memsize) +{ + vzDomObjPtr privdom = dom->privateData; + PRL_HANDLE job; + PRL_RESULT pret; + + job = PrlVm_BeginEdit(privdom->sdkdom); + if (PRL_FAILED(waitJob(job))) + goto error; + + pret = PrlVmCfg_SetRamSize(privdom->sdkdom, memsize); + prlsdkCheckRetGoto(pret, error); + + job = PrlVm_CommitEx(privdom->sdkdom, 0); + if (PRL_FAILED(waitJob(job))) + goto error; + + return 0; + + error: + return -1; +} diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 2f11d4f..4621868 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -80,3 +80,5 @@ int prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats); void prlsdkDomObjFreePrivate(void *p); +/* memsize is in MiB */ +int prlsdkSetMemsize(virDomainObjPtr dom, unsigned int memsize); -- 1.8.3.1

08.04.2016 12:36, Nikolay Shirokovskiy пишет:
Quite straigthforward as vz sdk memory setting function makes just what we want to that is set "amount of physical memory allocated to a domain".
'useflags' is introduced for non flag function implementation. We can't just use combination of flags like "live | config" or we fail for inactive domains. Other combinations have drawbacks too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_driver.c | 36 ++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 24 ++++++++++++++++++++++++ src/vz/vz_sdk.h | 2 ++ 3 files changed, 62 insertions(+)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 08447d9..85f17f7 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1479,6 +1479,40 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb) return ret; }
+static int vzDomainSetMemoryFlagsImpl(virDomainPtr domain, unsigned long memory, + unsigned int flags, bool useflags) +{ + virDomainObjPtr dom = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!(dom = vzDomObjFromDomain(domain))) + return -1; + + if (useflags && vzCheckConfigUpdateFlags(dom, &flags) < 0) + goto cleanup; + + ret = prlsdkSetMemsize(dom, memory >> 10); + + cleanup: + + virObjectUnlock(dom); + return ret; +} + +static int vzDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, + unsigned int flags) +{ + return vzDomainSetMemoryFlagsImpl(domain, memory, flags, true); +} + +static int vzDomainSetMemory(virDomainPtr domain, unsigned long memory) +{ + return vzDomainSetMemoryFlagsImpl(domain, memory, 0, false); +} + static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ @@ -1543,6 +1577,8 @@ static virHypervisorDriver vzDriver = { .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ .connectRegisterCloseCallback = vzConnectRegisterCloseCallback, /* 1.3.2 */ .connectUnregisterCloseCallback = vzConnectUnregisterCloseCallback, /* 1.3.2 */ + .domainSetMemoryFlags = vzDomainSetMemoryFlags, /* 1.3.3 */ + .domainSetMemory = vzDomainSetMemory, /* 1.3.3 */
s/1.3.3/1.3.4
};
static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index c006517..85ea0d7 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4302,3 +4302,27 @@ prlsdkGetMemoryStats(virDomainObjPtr dom,
return ret; } + +/* memsize is in MiB */ +int prlsdkSetMemsize(virDomainObjPtr dom, unsigned int memsize) +{ + vzDomObjPtr privdom = dom->privateData; + PRL_HANDLE job; + PRL_RESULT pret; + + job = PrlVm_BeginEdit(privdom->sdkdom); + if (PRL_FAILED(waitJob(job))) + goto error; + + pret = PrlVmCfg_SetRamSize(privdom->sdkdom, memsize); + prlsdkCheckRetGoto(pret, error); + + job = PrlVm_CommitEx(privdom->sdkdom, 0); + if (PRL_FAILED(waitJob(job))) + goto error; + + return 0; + + error: + return -1; +} diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 2f11d4f..4621868 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -80,3 +80,5 @@ int prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats); void prlsdkDomObjFreePrivate(void *p); +/* memsize is in MiB */ +int prlsdkSetMemsize(virDomainObjPtr dom, unsigned int memsize);
Fixed a minor version nit and pushed.
participants (2)
-
Maxim Nestratov
-
Nikolay Shirokovskiy