[libvirt] [PATCH v2 0/4] configure inactive domains' maximum memory size

Hi all, This patchset enables us to configure inactive domain's maximum memory size. I redesigned according to Daniel-san's commnet. *[PATCH 1/4] [RESEND] setmaxmem: remove the code to invoke virDomainSetMemory in cmdSetmaxmem => http://www.redhat.com/archives/libvir-list/2011-March/msg00747.html *[PATCH 2/4] maxmem: introduces VIR_DOMAIN_MEM_MAXIMUM flag *[PATCH 3/4] maxmem: implement virDomainSetMaxMemory API of the qemu driver *[PATCH 4/4] setmaxmem: add the new options to "virsh setmaxmem" command The following patchset is the prerequisite of these. => http://www.redhat.com/archives/libvir-list/2011-March/msg01057.html Best regards, Taku Izumi

This patch removes the code which invokes virDomainSetMemory() in cmdSetmaxmem(). When the new maximum memory size becomes less than the current memory size, I think it is not the libvirt client but the each driver that decides the behavior (reject the operation or shrink the current memory size). Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- tools/virsh.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -3047,15 +3047,7 @@ cmdSetmaxmem(vshControl *ctl, const vshC if (virDomainSetMaxMemory(dom, kilobytes) != 0) { vshError(ctl, "%s", _("Unable to change MaxMemorySize")); - virDomainFree(dom); - return FALSE; - } - - if (kilobytes < info.memory) { - if (virDomainSetMemory(dom, kilobytes) != 0) { - vshError(ctl, "%s", _("Unable to shrink current MemorySize")); - ret = FALSE; - } + ret = FALSE; } virDomainFree(dom);

This patch introduces VIR_DOMAIN_MEM_MAXIMUM flag. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 2 ++ 2 files changed, 3 insertions(+) Index: libvirt/include/libvirt/libvirt.h.in =================================================================== --- libvirt.orig/include/libvirt/libvirt.h.in +++ libvirt/include/libvirt/libvirt.h.in @@ -857,6 +857,7 @@ typedef enum { VIR_DOMAIN_MEM_CURRENT= 0, /* affect current domain */ VIR_DOMAIN_MEM_LIVE = (1 << 0), /* affect active domain */ VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */ + VIR_DOMAIN_MEM_MAXIMUM= (1 << 2), /* affect Max rather than current */ } virDomainMemoryModFlags; Index: libvirt/src/libvirt.c =================================================================== --- libvirt.orig/src/libvirt.c +++ libvirt/src/libvirt.c @@ -2869,6 +2869,8 @@ error: * the XML description on the domain. If neither flag is specified * (=VIR_DOMAIN_MEM_CURRENT), behavior is different depending on hypervisor. * + * If @flags incluces VIR_DOMAIN_MEM_MAXIMUM, + * * Returns 0 in case of success, -1 in case of failure. */

On 03/30/2011 11:30 PM, Taku Izumi wrote:
This patch introduces VIR_DOMAIN_MEM_MAXIMUM flag.
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 2 ++ 2 files changed, 3 insertions(+)
Index: libvirt/include/libvirt/libvirt.h.in =================================================================== --- libvirt.orig/include/libvirt/libvirt.h.in +++ libvirt/include/libvirt/libvirt.h.in @@ -857,6 +857,7 @@ typedef enum { VIR_DOMAIN_MEM_CURRENT= 0, /* affect current domain */
This part of the patch doesn't exist. Am I missing some other patch series that should have been applied first? [goes back and re-reads - yep, you said so in mail 0/4]. OK, I'll review that first. But I've now applied 1/4 from this series from when it was first acked.
VIR_DOMAIN_MEM_LIVE = (1 << 0), /* affect active domain */ VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */ + VIR_DOMAIN_MEM_MAXIMUM= (1 << 2), /* affect Max rather than current */
I'll probably reindent things to put a space before = and line up the other lines, but that's trivial.
Index: libvirt/src/libvirt.c =================================================================== --- libvirt.orig/src/libvirt.c +++ libvirt/src/libvirt.c @@ -2869,6 +2869,8 @@ error: * the XML description on the domain. If neither flag is specified * (=VIR_DOMAIN_MEM_CURRENT), behavior is different depending on hypervisor. * + * If @flags incluces VIR_DOMAIN_MEM_MAXIMUM, + *
Incomplete sentence. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

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@jp.fujitsu.com> --- src/qemu/qemu_driver.c | 85 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 24 deletions(-) Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -1578,7 +1578,9 @@ static int qemudDomainSetMemoryFlags(vir int ret = -1, r, isActive; virCheckFlags(VIR_DOMAIN_MEM_LIVE | - VIR_DOMAIN_MEM_CONFIG, -1); + VIR_DOMAIN_MEM_CONFIG | + VIR_DOMAIN_MEM_MAXIMUM, -1); + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1591,12 +1593,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; - } - if (qemuDomainObjBeginJob(vm) < 0) goto cleanup; @@ -1608,6 +1604,15 @@ static int qemudDomainSetMemoryFlags(vir else flags = VIR_DOMAIN_MEM_CONFIG; } + if (flags == VIR_DOMAIN_MEM_MAXIMUM) { + if (isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("cannot resize the maximum memory on an active domain")); + goto endjob; + } else { + flags = VIR_DOMAIN_MEM_CONFIG | VIR_DOMAIN_MEM_MAXIMUM; + } + } if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) { qemuReportError(VIR_ERR_OPERATION_INVALID, @@ -1625,27 +1630,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")); 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); + 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")); + 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; + + /* 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; @@ -1663,6 +1695,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); +} + static int qemudDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { struct qemud_driver *driver = dom->conn->privateData; @@ -6843,7 +6880,7 @@ static virDriver qemuDriver = { qemudDomainDestroy, /* domainDestroy */ qemudDomainGetOSType, /* domainGetOSType */ qemudDomainGetMaxMemory, /* domainGetMaxMemory */ - NULL, /* domainSetMaxMemory */ + qemudDomainSetMaxMemory, /* domainSetMaxMemory */ qemudDomainSetMemory, /* domainSetMemory */ qemudDomainSetMemoryFlags, /* domainSetMemoryFlags */ qemuDomainSetMemoryParameters, /* domainSetMemoryParameters */

This patch adds the new options (--live, --config, and --current) to "virsh setmaxmem" command. The behavior of above options is the same as that of "virsh setmem". When the --config option is specified, a modofication is effective for the persistent domain, while the --live option is specified, a modification is effective for an active domain. The --current option is specified, it affects a current domain. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- tools/virsh.c | 35 ++++++++++++++++++++++++++++++++--- tools/virsh.pod | 14 ++++++++------ 2 files changed, 40 insertions(+), 9 deletions(-) Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -3011,6 +3011,9 @@ static const vshCmdInfo info_setmaxmem[] static const vshCmdOptDef opts_setmaxmem[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")}, + {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, + {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, + {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, {NULL, 0, 0, NULL} }; @@ -3021,6 +3024,25 @@ cmdSetmaxmem(vshControl *ctl, const vshC virDomainInfo info; int kilobytes = 0; int ret = TRUE; + int config = vshCommandOptBool(cmd, "config"); + int live = vshCommandOptBool(cmd, "live"); + int current = vshCommandOptBool(cmd, "current"); + int flags = VIR_DOMAIN_MEM_MAXIMUM; + + if (current) { + if(live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return FALSE; + } + } else { + if (config) + flags |= VIR_DOMAIN_MEM_CONFIG; + if (live) + flags |= VIR_DOMAIN_MEM_LIVE; + /* neither option is specified */ + if (!live && !config) + flags = -1; + } if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -3045,9 +3067,16 @@ cmdSetmaxmem(vshControl *ctl, const vshC return FALSE; } - if (virDomainSetMaxMemory(dom, kilobytes) != 0) { - vshError(ctl, "%s", _("Unable to change MaxMemorySize")); - ret = FALSE; + if (flags == -1) { + if (virDomainSetMaxMemory(dom, kilobytes) != 0) { + vshError(ctl, "%s", _("Unable to change MaxMemorySize")); + ret = FALSE; + } + } else { + if (virDomainSetMemoryFlags(dom, kilobytes, flags) < 0) { + vshError(ctl, "%s", _("Unable to change MaxMemorySize")); + ret = FALSE; + } } virDomainFree(dom); Index: libvirt/tools/virsh.pod =================================================================== --- libvirt.orig/tools/virsh.pod +++ libvirt/tools/virsh.pod @@ -596,12 +596,16 @@ rounds the parameter up unless the kB ar For Xen, you can only adjust the memory of a running domain if the domain is paravirtualized or running the PV balloon driver. -=item B<setmaxmem> I<domain-id> B<kilobytes> +=item B<setmaxmem> I<domain-id> B<kilobytes> optional I<--config> I<--live> I<--current> -Change the maximum memory allocation limit for an inactive guest domain. +Change the maximum memory allocation limit for a guest domain. +If I<--live> is specified, affect a running guest. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified, affect a current guest. +Both I<--live> and I<--current> flags may be given, but I<--current> is exclusive. +If neither flag is specified, behavior is different depending on hypervisor. -This command works for at least the Xen and vSphere/ESX hypervisors, -but not for QEMU/KVM. +This command works for at least the Xen, QEMU/KVM and vSphere/ESX hypervisors. Some hypervisors require a larger granularity than kilobytes, rounding up requests that are not an even multiple of the desired amount. vSphere/ESX @@ -609,8 +613,6 @@ is one of these, requiring the parameter vSphere/ESX, 263168 (257MB) would be rounded up because it's not a multiple of 4MB, while 266240 (260MB) is valid without rounding. -Note, to change the maximum memory allocation for a QEMU/KVM guest domain, -use the virsh B<edit> command instead to update its XML <memory> element. =item B<memtune> I<domain-id> optional I<--hard-limit> B<kilobytes> optional I<--soft-limit> B<kilobytes> optional I<--swap-hard-limit>

On 03/30/2011 11:19 PM, Taku Izumi wrote:
Hi all,
This patchset enables us to configure inactive domain's maximum memory size. I redesigned according to Daniel-san's commnet.
The following patchset is the prerequisite of these. => http://www.redhat.com/archives/libvir-list/2011-March/msg01057.html
My sincere apologies at the delay on these series. I've been so focused on locking bugs and libvirtd crashers that I haven't had a chance to properly review any of the persistent settings patches in time for inclusion in 0.9.0. My justification is that you can always achieve persistent settings (albeit with undue pain) by raw XML editing, so this is just an enhancement request rather than a bug fix. However, now that we've missed the feature freeze deadline for 0.9.0, you have my promise that I will be reviewing this next week for inclusion in 0.9.1. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Hi Eric, (2011/04/02 1:31), Eric Blake wrote:
On 03/30/2011 11:19 PM, Taku Izumi wrote:
Hi all,
This patchset enables us to configure inactive domain's maximum memory size. I redesigned according to Daniel-san's commnet.
The following patchset is the prerequisite of these. => http://www.redhat.com/archives/libvir-list/2011-March/msg01057.html
My sincere apologies at the delay on these series. I've been so focused on locking bugs and libvirtd crashers that I haven't had a chance to properly review any of the persistent settings patches in time for inclusion in 0.9.0. My justification is that you can always achieve persistent settings (albeit with undue pain) by raw XML editing, so this is just an enhancement request rather than a bug fix. However, now that we've missed the feature freeze deadline for 0.9.0, you have my promise that I will be reviewing this next week for inclusion in 0.9.1.
That's OK, I don't mind. Please review this after releasing 0.9.0. Best regards, Taku Izumi
participants (2)
-
Eric Blake
-
Taku Izumi