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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list