On 04/07/2011 11:08 PM, Taku Izumi wrote:
>
> This patch implements the code to support virDomainSetMaxMemory API,
> and to support VIR_DOMAIN_MEM_MAXIMUM flag in qemudDomainSetMemoryFlags function.
> As a result, we can change the maximum memory size of inactive QEMU guests.
>
> Signed-off-by: Taku Izumi <izumi.taku@xxxxxxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 81 ++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 57 insertions(+), 24 deletions(-)
> @@ -1593,12 +1594,6 @@ static int qemudDomainSetMemoryFlags(vir
> goto cleanup;
> }
>
> - if (newmem > vm->def->mem.max_balloon) {
> - qemuReportError(VIR_ERR_INVALID_ARG,
> - "%s", _("cannot set memory higher than max memory"));
> - goto cleanup;
> - }
> -
Why are you dropping this check? Oh, I should read the whole patch first...
> @@ -1610,6 +1605,12 @@ static int qemudDomainSetMemoryFlags(vir
> else
> flags = VIR_DOMAIN_MEM_CONFIG;
> }
> + if (flags == VIR_DOMAIN_MEM_MAXIMUM) {
> + if (isActive)
> + flags = VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_MAXIMUM;
qemu can't change the maximum memory allocation of an active domain.
Oh, but I see you catch that later...
> + else
> + flags = VIR_DOMAIN_MEM_CONFIG | VIR_DOMAIN_MEM_MAXIMUM;
> + }
>
> if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
> qemuReportError(VIR_ERR_OPERATION_INVALID,
> @@ -1627,27 +1628,54 @@ static int qemudDomainSetMemoryFlags(vir
> goto endjob;
> }
>
> - if (flags & VIR_DOMAIN_MEM_LIVE) {
> - priv = vm->privateData;
> - qemuDomainObjEnterMonitor(vm);
> - r = qemuMonitorSetBalloon(priv->mon, newmem);
> - qemuDomainObjExitMonitor(vm);
> - qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 1);
> - if (r < 0)
> - goto endjob;
> + if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
> + /* resize the maximum memory */
>
> - /* Lack of balloon support is a fatal error */
> - if (r == 0) {
> + if (flags & VIR_DOMAIN_MEM_LIVE) {
> qemuReportError(VIR_ERR_OPERATION_INVALID,
> - "%s", _("cannot set memory of an active domain"));
> + _("cannot resize the maximum memory on an active domain"));
Needs a "%s" to keep compilation without gettext happy (that's the only
case where gcc starts warning about a format string with no % in it).
But this answers my second question.
> goto endjob;
> }
> - }
>
> - if (flags& VIR_DOMAIN_MEM_CONFIG) {
> - persistentDef->mem.cur_balloon = newmem;
> - ret = virDomainSaveConfig(driver->configDir, persistentDef);
> - goto endjob;
> + if (flags & VIR_DOMAIN_MEM_CONFIG) {
> + persistentDef->mem.max_balloon = newmem;
> + if (persistentDef->mem.cur_balloon > newmem)
> + persistentDef->mem.cur_balloon = newmem;
> + ret = virDomainSaveConfig(driver->configDir, persistentDef);
Good - you cap the current memory if the maximum dropped.
> + goto endjob;
> + }
> +
> + } else {
> + /* resize the current memory */
> +
> + if (newmem > vm->def->mem.max_balloon) {
> + qemuReportError(VIR_ERR_INVALID_ARG,
> + "%s", _("cannot set memory higher than max memory"));
Ah, you moved it down lower, answering my first question.
> + goto endjob;
> + }
> +
> + if (flags & VIR_DOMAIN_MEM_LIVE) {
> + priv = vm->privateData;
> + qemuDomainObjEnterMonitor(vm);
> + r = qemuMonitorSetBalloon(priv->mon, newmem);
> + qemuDomainObjExitMonitor(vm);
> + qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r == 1);
This indentation caused lines longer than 80 columns, so I tweaked that.
> + if (r < 0)
> + goto endjob;
> +
> + /* Lack of balloon support is a fatal error */
> + if (r == 0) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("cannot set memory of an active domain"));
> + goto endjob;
> + }
> + }
> +
> + if (flags & VIR_DOMAIN_MEM_CONFIG) {
> + persistentDef->mem.cur_balloon = newmem;
> + ret = virDomainSaveConfig(driver->configDir, persistentDef);
> + goto endjob;
> + }
> }
>
> ret = 0;
> @@ -1665,6 +1693,11 @@ static int qemudDomainSetMemory(virDomai
> return qemudDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_MEM_LIVE);
> }
>
> +static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) {
> + return qemudDomainSetMemoryFlags(dom, memory,
> + VIR_DOMAIN_MEM_MAXIMUM | VIR_DOMAIN_MEM_LIVE);
Hmm. Given the above implementation, this will _always_ fail. Then
again, the failure message will be nicer (the function used to fail with
"not implemented", now it fails with "cannot resize memory on an active
domain"), so I guess it's okay to provide this stub. And given the
documentation in libvirt.c, this is accurate (that documentation
explicitly stated that it only works on live domains).
But the real clincher is how xen behaves. I just tested, and my RHEL 5
machine with xen:/// was able to change persistent max memory of an
inactive domain, so the documentation in libvirt.c is wrong. I then
tried an active domain, which (surprisingly) changed the max cap, then
promptly forgot the change when the domain went inactive again proving
that it behaved like _LIVE and not _LIVE|_CONFIG. Qemu can't change max
mem on a live domain (it's capped at qemu startup time) even if xen can,
so if we tweak the libvirt.c wording to accommodate both xen and qemu
behaviors (since it was already inaccurate for xen), then we can make
this function succeed some of the time by defaulting to
_CURRENT|_MAXIMUM instead of _LIVE|_MAXIMUM.
> +}
> +
> static int qemudDomainGetInfo(virDomainPtr dom,
> virDomainInfoPtr info) {
> struct qemud_driver *driver = dom->conn->privateData;
> @@ -6849,7 +6882,7 @@ static virDriver qemuDriver = {
> qemudDomainDestroy, /* domainDestroy */
> qemudDomainGetOSType, /* domainGetOSType */
> qemudDomainGetMaxMemory, /* domainGetMaxMemory */
> - NULL, /* domainSetMaxMemory */
> + qemudDomainSetMaxMemory, /* domainSetMaxMemory */
> qemudDomainSetMemory, /* domainSetMemory */
> qemudDomainSetMemoryFlags, /* domainSetMemoryFlags */
> qemuDomainSetMemoryParameters, /* domainSetMemoryParameters */
>
So, here's what I squashed in before pushing:
diff --git i/src/libvirt.c w/src/libvirt.c
index 4a39695..0da9885 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -2714,8 +2714,9 @@ error:
* to Domain0 i.e. the domain where the application runs.
* This function requires privileged access to the hypervisor.
*
- * This command only changes the runtime configuration of the domain,
- * so can only be called on an active domain.
+ * This command is hypervisor-specific for whether active, persistent,
+ * or both configurations are changed; for more control, use
+ * virDomainSetMemoryFlags().
*
* Returns 0 in case of success and -1 in case of failure.
*/
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index a547faf..8a8b55d 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -1632,8 +1632,9 @@ static int qemudDomainSetMemoryFlags(virDomainPtr
dom, unsigned long newmem,
/* resize the maximum memory */
if (flags & VIR_DOMAIN_MEM_LIVE) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- _("cannot resize the maximum memory on an
active domain"));
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot resize the maximum memory on an "
+ "active domain"));
goto endjob;
}
@@ -1649,8 +1650,8 @@ static int qemudDomainSetMemoryFlags(virDomainPtr
dom, unsigned long newmem,
/* resize the current memory */
if (newmem > vm->def->mem.max_balloon) {
- qemuReportError(VIR_ERR_INVALID_ARG,
- "%s", _("cannot set memory higher than max
memory"));
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("cannot set memory higher than max memory"));
goto endjob;
}
@@ -1659,14 +1660,15 @@ static int
qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
qemuDomainObjEnterMonitor(vm);
r = qemuMonitorSetBalloon(priv->mon, newmem);
qemuDomainObjExitMonitor(vm);
- qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem,
"update", r == 1);
+ qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update",
+ r == 1);
if (r < 0)
goto endjob;
/* Lack of balloon support is a fatal error */
if (r == 0) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot set memory of an active
domain"));
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot set memory of an active
domain"));
goto endjob;
}
}
@@ -1695,7 +1697,8 @@ static int qemudDomainSetMemory(virDomainPtr dom,
unsigned long newmem) {
static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long
memory) {
return qemudDomainSetMemoryFlags(dom, memory,
- VIR_DOMAIN_MEM_MAXIMUM |
VIR_DOMAIN_MEM_LIVE);
+ (VIR_DOMAIN_MEM_MAXIMUM |
+ VIR_DOMAIN_MEM_CURRENT));
}
static int qemudDomainGetInfo(virDomainPtr dom,
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list