[libvirt] [PATCH 1/4] qemu: Make Set*Mem commands hotplug only

[Daniel P. Berrange]

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 :|