Re: [PATCH v3 2/3] update qemuDomainGetBlkioParameters to use flags

[Daniel Veillard <veillard@xxxxxxxxxx>]

On Tue, Jun 07, 2011 at 02:03:08PM +0800, Hu Tao wrote:
> ---
>  src/qemu/qemu_driver.c |  124 +++++++++++++++++++++++++++++++++---------------
>  1 files changed, 86 insertions(+), 38 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2957467..8ebb7d4 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4824,18 +4824,16 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
>      int i;
>      virCgroupPtr group = NULL;
>      virDomainObjPtr vm = NULL;
> +    virDomainDefPtr persistentDef = NULL;
>      unsigned int val;
>      int ret = -1;
>      int rc;
> +    bool isActive;
>  
> -    virCheckFlags(0, -1);
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
>      qemuDriverLock(driver);
>  
> -    if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
> -        qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't mounted"));
> -        goto cleanup;
> -    }
> -
>      vm = virDomainFindByUUID(&driver->domains, dom->uuid);
>  
>      if (vm == NULL) {
> @@ -4844,12 +4842,6 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (!virDomainObjIsActive(vm)) {
> -        qemuReportError(VIR_ERR_OPERATION_INVALID,
> -                        "%s", _("domain is not running"));
> -        goto cleanup;
> -    }
> -
>      if ((*nparams) == 0) {
>          /* Current number of blkio parameters supported by cgroups */
>          *nparams = QEMU_NB_BLKIO_PARAM;
> @@ -4863,37 +4855,93 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) {
> -        qemuReportError(VIR_ERR_INTERNAL_ERROR,
> -                        _("cannot find cgroup for domain %s"), vm->def->name);
> -        goto cleanup;
> +    isActive = virDomainObjIsActive(vm);
> +
> +    if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
> +        if (isActive)
> +            flags = VIR_DOMAIN_AFFECT_LIVE;
> +        else
> +            flags = VIR_DOMAIN_AFFECT_CONFIG;
>      }
>  
> -    for (i = 0; i < *nparams; i++) {
> -        virTypedParameterPtr param = &params[i];
> -        val = 0;
> -        param->value.ui = 0;
> -        param->type = VIR_TYPED_PARAM_UINT;
> +    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> +        if (!isActive) {
> +            qemuReportError(VIR_ERR_OPERATION_INVALID,
> +                            "%s", _("domain is not running"));
> +            goto cleanup;
> +        }
>  
> -        switch (i) {
> -        case 0: /* fill blkio weight here */
> -            rc = virCgroupGetBlkioWeight(group, &val);
> -            if (rc != 0) {
> -                virReportSystemError(-rc, "%s",
> -                                     _("unable to get blkio weight"));
> -                goto cleanup;
> -            }
> -            if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) {
> -                qemuReportError(VIR_ERR_INTERNAL_ERROR,
> -                                "%s", _("Field blkio weight too long for destination"));
> -                goto cleanup;
> +        if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
> +            qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't mounted"));
> +            goto cleanup;
> +        }
> +
> +        if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) {
> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                            _("cannot find cgroup for domain %s"), vm->def->name);
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> +        if (!vm->persistent) {
> +            qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                            _("cannot change persistent config of a transient domain"));
> +            goto cleanup;
> +        }
> +        if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm)))
> +            goto cleanup;
> +    }
> +
> +    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> +        for (i = 0; i < *nparams; i++) {
> +            virTypedParameterPtr param = &params[i];
> +            val = 0;
> +            param->value.ui = 0;
> +            param->type = VIR_TYPED_PARAM_UINT;
> +
> +            switch (i) {
> +            case 0: /* fill blkio weight here */
> +                rc = virCgroupGetBlkioWeight(group, &val);
> +                if (rc != 0) {
> +                    virReportSystemError(-rc, "%s",
> +                                         _("unable to get blkio weight"));
> +                    goto cleanup;
> +                }
> +                if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) {
> +                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                                    "%s", _("Field blkio weight too long for destination"));
> +                    goto cleanup;
> +                }
> +                param->value.ui = val;
> +                break;
> +
> +            default:
> +                break;
> +                /* should not hit here */
>              }
> -            param->value.ui = val;
> -            break;
> +        }
> +    } else if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> +        for (i = 0; i < *nparams; i++) {
> +            virTypedParameterPtr param = &params[i];
> +            val = 0;
> +            param->value.ui = 0;
> +            param->type = VIR_TYPED_PARAM_UINT;
>  
> -        default:
> -            break;
> -            /* should not hit here */
> +            switch (i) {
> +            case 0: /* fill blkio weight here */
> +                if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) {
> +                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                                    "%s", _("Field blkio weight too long for destination"));
> +                    goto cleanup;
> +                }
> +                param->value.ui = persistentDef->blkio.weight;
> +                break;
> +
> +            default:
> +                break;
> +                /* should not hit here */
> +            }
>          }
>      }

  Okay, the for() loop with only 0 index being useful is a bit confusing
but matches other similar parmeter filling pieces of code,

  ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list