2:04 p.m.
Update the current or max memory, on the persistent or live definition
depending on the flags which are currently ignored.
Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
---
src/test/test_driver.c | 51 +++++++++++++++++++++++++++++++++++-------
1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index c10344f6cd..90910060ed 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2473,24 +2473,59 @@ static int testDomainSetMemoryFlags(virDomainPtr domain,
unsigned long memory,
unsigned int flags)
{
- virDomainObjPtr privdom;
+ virDomainObjPtr vm;
+ virDomainDefPtr def;
int ret = -1;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG |
+ VIR_DOMAIN_MEM_MAXIMUM, -1);
- if (!(privdom = testDomObjFromDomain(domain)))
+ if (!(vm = testDomObjFromDomain(domain)))
return -1;
- if (memory > virDomainDefGetMemoryTotal(privdom->def)) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ if (!(def = virDomainObjGetOneDef(vm, flags)))
goto cleanup;
+
+ if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
+ if (virDomainObjCheckActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot resize the maximum memory on an "
+ "active domain"));
+ goto cleanup;
+ }
+
+ if (virDomainNumaGetNodeCount(def->numa) > 0) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("initial memory size of a domain with NUMA "
+ "nodes cannot be modified with this API"));
+ goto cleanup;
+ }
+
+ if (def->mem.max_memory && def->mem.max_memory < memory) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot set initial memory size greater than "
+ "the maximum memory size"));
+ goto cleanup;
+ }
+
+ virDomainDefSetMemoryTotal(def, memory);
+
+ if (def->mem.cur_balloon > memory)
+ def->mem.cur_balloon = memory;
+ } else {
+ if (memory > virDomainDefGetMemoryTotal(def)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("cannot set memory higher than max memory"));
+ goto cleanup;
+ }
+
+ def->mem.cur_balloon = memory;
}
- privdom->def->mem.cur_balloon = memory;
ret = 0;
-
cleanup:
- virDomainObjEndAPI(&privdom);
+ virDomainObjEndAPI(&vm);
return ret;
}
--
2.22.0
5:02 p.m.
On 7/11/19 6:04 AM, Ilias Stamatis wrote:
Update the current or max memory, on the persistent or live
definition
depending on the flags which are currently ignored.
Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
---
src/test/test_driver.c | 51 +++++++++++++++++++++++++++++++++++-------
1 file changed, 43 insertions(+), 8 deletions(-)
Incomplete. You also need to fix what I missed in commit 667ac11e, in
that testDomainSetMemory() should now forward to
testDomainSetMemoryFlags(, VIR_DOMAIN_AFFECT_LIVE); and it would also be
worth implementing testDomainSetMaxMemory() to forward to
testDomainSetMemoryFlags(, VIR_DOMAIN_MEM_MAXIMUM).
Oh, and thinking about it, maybe everywhere we have
xxxDomainSetMaxMemory forwarding on, it seems odd that it is affecting
VIR_DOMAIN_AFFECT_CURRENT instead of VIR_DOMAIN_AFFECT_LIVE; but then we
have to start worrying about back-compat issues. :(
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
5:15 p.m.
On Thu, Jul 11, 2019 at 4:03 PM Eric Blake <eblake(a)redhat.com> wrote:
On 7/11/19 6:04 AM, Ilias Stamatis wrote:
> Update the current or max memory, on the persistent or live definition
> depending on the flags which are currently ignored.
>
> Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
> ---
> src/test/test_driver.c | 51 +++++++++++++++++++++++++++++++++++-------
> 1 file changed, 43 insertions(+), 8 deletions(-)
>
Incomplete. You also need to fix what I missed in commit 667ac11e, in
that testDomainSetMemory() should now forward to
testDomainSetMemoryFlags(, VIR_DOMAIN_AFFECT_LIVE); and it would also be
worth implementing testDomainSetMaxMemory() to forward to
testDomainSetMemoryFlags(, VIR_DOMAIN_MEM_MAXIMUM).
Sure. But do you think they should be on the same patch?
Maybe in a subsequent patch on the same series?
Oh, and thinking about it, maybe everywhere we have
xxxDomainSetMaxMemory forwarding on, it seems odd that it is affecting
VIR_DOMAIN_AFFECT_CURRENT instead of VIR_DOMAIN_AFFECT_LIVE; but then we
have to start worrying about back-compat issues. :(
According to the documentation this is hypervisor-dependent so in the
case of the test driver I think it's fine to call it with
VIR_DOMAIN_AFFECT_LIVE.
However, changing it on other drivers raises compatibility issues as
you mentioned.
Ilias
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
5:30 p.m.
On 7/11/19 9:15 AM, Ilias Stamatis wrote:
On Thu, Jul 11, 2019 at 4:03 PM Eric Blake <eblake(a)redhat.com>
wrote:
>
> On 7/11/19 6:04 AM, Ilias Stamatis wrote:
>> Update the current or max memory, on the persistent or live definition
>> depending on the flags which are currently ignored.
>>
>> Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
>> ---
>> src/test/test_driver.c | 51 +++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 43 insertions(+), 8 deletions(-)
>>
>
> Incomplete. You also need to fix what I missed in commit 667ac11e, in
> that testDomainSetMemory() should now forward to
> testDomainSetMemoryFlags(, VIR_DOMAIN_AFFECT_LIVE); and it would also be
> worth implementing testDomainSetMaxMemory() to forward to
> testDomainSetMemoryFlags(, VIR_DOMAIN_MEM_MAXIMUM).
Sure. But do you think they should be on the same patch?
Maybe in a subsequent patch on the same series?
Separate patches in a series is fine.
>
> Oh, and thinking about it, maybe everywhere we have
> xxxDomainSetMaxMemory forwarding on, it seems odd that it is affecting
> VIR_DOMAIN_AFFECT_CURRENT instead of VIR_DOMAIN_AFFECT_LIVE; but then we
> have to start worrying about back-compat issues. :(
According to the documentation this is hypervisor-dependent so in the
case of the test driver I think it's fine to call it with
VIR_DOMAIN_AFFECT_LIVE.
My eventual goal is to git rid of multiple driver callbacks, and have
JUST .drvDomainSetMemoryFlags() for drivers to implement. But for that
to work, we have to teach the remote driver to call the OLD RPC function
for every scenario where the old function is a trivial wrapper around
the new (in some cases, that's when flags==0, in others, when
flags==VIR_DOMAIN_AFFECT_CURRENT), in case the remote driver is talking
to an older libvirt that did not yet have the implementation of the new
callback function. That is, if libvirt 5.7 as client calls
virDomainSetMaxMemory(), that will call into the remote driver as
.drvDomainSetMemoryFlags(, default_flags), then the remote driver will
see the default flags and call the RPC for DOMAIN_SET_MAX_MEMORY, in
case on the remote side is running 5.5 or older and has the older
.drvDomainSetMaxMemory() for a driver that did not yet have
.drvDomainSetMemoryFlags() support. We don't want to fail the call just
because the new callback was missing in the older server. But to do
that, we have to audit ALL of the drivers with paired APIs and ensure
that, at least in new libvirt, the pairings all use the same
default_flags (for knowing when to fall back to the old API), and that
for old libvirt, that falling back to the old API is not going to subtly
change semantics for whehter a libvirt 5.5 or libvirt 5.7 client made
the request.
However, changing it on other drivers raises compatibility issues as
you mentioned.
That's why it is a bigger issue, and needs some careful thought. I
started on the topic, but it is now lower-priority for me than getting
incremental backup into 5.6, so it might not happen until 5.7.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2199
days inactive
2199
days old
3 comments
2 participants
participants (2)
-
Eric Blake -
Ilias Stamatis