On Fri, Feb 12, 2010 at 10:32:14AM -0500, Cole Robinson wrote:

SetMem and SetMaxMem are hotplug only APIs, any persistent config changes are supposed to go via XML definition. The original implementation of these calls were incorrect and had the nasty side effect of making a psuedo persistent change that would be lost after libvirtd restart (I didn't know any better).

Fix these APIs to rightly reject non running domains. --- src/qemu/qemu_driver.c | 54 ++++++++++++++++++++++++++++------------------- 1 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a2be1a3..56a450c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3625,14 +3625,21 @@ static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) { goto cleanup; }

+ if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + if (newmax < vm->def->memory) { - qemuReportError(VIR_ERR_INVALID_ARG, - "%s", _("cannot set max memory lower than current memory")); - goto cleanup;; + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("cannot set max memory lower than current memory")); + goto cleanup; }

- vm->def->maxmem = newmax; - ret = 0; + /* There isn't any way to change this value for a running qemu guest */ + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cannot set max memory of an active domain"));

cleanup: if (vm) @@ -3643,8 +3650,9 @@ cleanup:

static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { struct qemud_driver *driver = dom->conn->privateData; + qemuDomainObjPrivatePtr priv; virDomainObjPtr vm; - int ret = -1; + int ret = -1, r;

qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -3657,6 +3665,12 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { goto cleanup; }

+ if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + if (newmem > vm->def->maxmem) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot set memory higher than max memory")); @@ -3666,25 +3680,21 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { if (qemuDomainObjBeginJob(vm) < 0) goto cleanup;

- if (virDomainObjIsActive(vm)) { - qemuDomainObjPrivatePtr priv = vm->privateData; - qemuDomainObjEnterMonitor(vm); - int r = qemuMonitorSetBalloon(priv->mon, newmem); - qemuDomainObjExitMonitor(vm); - if (r < 0) - goto endjob; + priv = vm->privateData; + qemuDomainObjEnterMonitor(vm); + r = qemuMonitorSetBalloon(priv->mon, newmem); + qemuDomainObjExitMonitor(vm); + if (r < 0) + goto endjob;

- /* Lack of balloon support is a fatal error */ - if (r == 0) { - qemuReportError(VIR_ERR_NO_SUPPORT, - "%s", _("cannot set memory of an active domain")); - goto endjob; - } - } else { - vm->def->memory = newmem; + /* Lack of balloon support is a fatal error */ + if (r == 0) { + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cannot set memory of an active domain")); + goto endjob; } - ret = 0;

+ ret = 0; endjob: if (qemuDomainObjEndJob(vm) == 0) vm = NULL;

ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|