[libvirt] [PATCHv3 05/10] Implement lxcDomainBlockStats* for lxc driver

Michal Privoznik mprivozn at redhat.com
Thu Feb 6 11:46:25 UTC 2014


On 03.02.2014 18:44, Thorsten Behrens wrote:
> Adds lxcDomainBlockStatsFlags and lxcDomainBlockStats functions.
> ---
>
> Notes v3:
>   - merged patch, both api methods now added in one
>   - addressed comments from v2 review
>   - check for cgroup controllers actually being mounted
>
>   src/lxc/lxc_driver.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 196 insertions(+)
>
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 39cd981..103352c 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -75,6 +75,7 @@
>
>
>   #define LXC_NB_MEM_PARAM  3
> +#define LXC_NB_DOMAIN_BLOCK_STAT_PARAM 4
>
>
>   static int lxcStateInitialize(bool privileged,
> @@ -2202,6 +2203,199 @@ lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array,
>
>
>   static int
> +lxcDomainBlockStats(virDomainPtr dom,
> +                    const char *path,
> +                    struct _virDomainBlockStats *stats)
> +{
> +    int ret = -1, idx;
> +    virDomainObjPtr vm;
> +    virDomainDiskDefPtr disk = NULL;
> +    virLXCDomainObjPrivatePtr priv;
> +
> +    if (!(vm = lxcDomObjFromDomain(dom)))
> +        return ret;
> +
> +    priv = vm->privateData;
> +
> +    if (virDomainBlockStatsEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
> +        goto cleanup;
> +    }
> +
> +    if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("blkio cgroup isn't mounted"));
> +        goto cleanup;
> +    }
> +
> +    if (!*path) {
> +        /* empty path - return entire domain blkstats instead */
> +        ret = virCgroupGetBlkioIoServiced(priv->cgroup,
> +                                          &stats->rd_bytes,
> +                                          &stats->wr_bytes,
> +                                          &stats->rd_req,
> +                                          &stats->wr_req);
> +        goto cleanup;
> +    }

This is interesting. In the qemu driver we don't allow this, and neither 
do we in the public API (well, at least in API documentation), nor 
virsh. I'm okay with this as long as we relax these restrictions in the 
places mentioned earlier.

> +
> +    if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("invalid path: %s"), path);
> +        goto cleanup;
> +    }
> +    disk = vm->def->disks[idx];
> +
> +    if (!disk->info.alias) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("missing disk device alias name for %s"), disk->dst);
> +        goto cleanup;
> +    }
> +
> +    ret = virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
> +                                            disk->info.alias,
> +                                            &stats->rd_bytes,
> +                                            &stats->wr_bytes,
> +                                            &stats->rd_req,
> +                                            &stats->wr_req);
> +cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);
> +    return ret;
> +}
> +
> +
> +static int
> +lxcDomainBlockStatsFlags(virDomainPtr dom,
> +                         const char * path,
> +                         virTypedParameterPtr params,
> +                         int * nparams,
> +                         unsigned int flags)
> +{
> +    int tmp, ret = -1, idx;
> +    virDomainObjPtr vm;
> +    virDomainDiskDefPtr disk = NULL;
> +    virLXCDomainObjPrivatePtr priv;
> +    long long rd_req, rd_bytes, wr_req, wr_bytes;
> +    virTypedParameterPtr param;
> +
> +    virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> +    /* We don't return strings, and thus trivially support this flag.  */
> +    flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
> +
> +    if (!params && !*nparams) {
> +        *nparams = LXC_NB_DOMAIN_BLOCK_STAT_PARAM;
> +        return 0;
> +    }
> +
> +    if (!(vm = lxcDomObjFromDomain(dom)))
> +        return ret;
> +
> +    priv = vm->privateData;
> +
> +    if (virDomainBlockStatsFlagsEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
> +        goto cleanup;
> +    }
> +
> +    if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("blkio cgroup isn't mounted"));
> +        goto cleanup;
> +    }
> +
> +    if (!*path) {
> +        /* empty path - return entire domain blkstats instead */
> +        if (virCgroupGetBlkioIoServiced(priv->cgroup,
> +                                        &rd_bytes,
> +                                        &wr_bytes,
> +                                        &rd_req,
> +                                        &wr_req) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s", _("domain stats query failed"));
> +            goto cleanup;
> +        }
> +    }
> +    else {

We tend to write this as '} else {'

> +        if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("invalid path: %s"), path);
> +            goto cleanup;
> +        }
> +        disk = vm->def->disks[idx];
> +
> +        if (!disk->info.alias) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("missing disk device alias name for %s"), disk->dst);
> +            goto cleanup;
> +        }
> +
> +        if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
> +                                              disk->info.alias,
> +                                              &rd_bytes,
> +                                              &wr_bytes,
> +                                              &rd_req,
> +                                              &wr_req) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s", _("domain stats query failed"));
> +            goto cleanup;
> +        }
> +    }
> +
> +    tmp = 0;
> +    ret = -1;
> +
> +    if (tmp < *nparams && wr_bytes != -1) {
> +        param = &params[tmp];
> +        if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES,
> +                                    VIR_TYPED_PARAM_LLONG, wr_bytes) < 0)
> +            goto cleanup;
> +        tmp++;
> +    }
> +
> +    if (tmp < *nparams && wr_req != -1) {
> +        param = &params[tmp];
> +        if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ,
> +                                    VIR_TYPED_PARAM_LLONG, wr_req) < 0)
> +            goto cleanup;
> +        tmp++;
> +    }
> +
> +    if (tmp < *nparams && rd_bytes != -1) {
> +        param = &params[tmp];
> +        if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_BYTES,
> +                                    VIR_TYPED_PARAM_LLONG, rd_bytes) < 0)
> +            goto cleanup;
> +        tmp++;
> +    }
> +
> +    if (tmp < *nparams && rd_req != -1) {
> +        param = &params[tmp];
> +        if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_REQ,
> +                                    VIR_TYPED_PARAM_LLONG, rd_req) < 0)
> +            goto cleanup;
> +        tmp++;
> +    }
> +
> +    ret = 0;
> +    *nparams = tmp;
> +
> +cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);
> +    return ret;
> +}
> +
> +
> +static int
>   lxcDomainSetBlkioParameters(virDomainPtr dom,
>                               virTypedParameterPtr params,
>                               int nparams,
> @@ -5495,6 +5689,8 @@ static virDriver lxcDriver = {
>       .domainGetSchedulerParametersFlags = lxcDomainGetSchedulerParametersFlags, /* 0.9.2 */
>       .domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */
>       .domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2 */
> +    .domainBlockStats = lxcDomainBlockStats, /* 1.2.2 */
> +    .domainBlockStatsFlags = lxcDomainBlockStatsFlags, /* 1.2.2 */
>       .domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */
>       .domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */
>       .nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */
>

Other than that, the patch looks good to me.

Michal




More information about the libvir-list mailing list