[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 1/5] cgroup: Introduce virCgroupNewThread



On Tue, Apr 07, 2015 at 09:23:13AM -0400, John Ferlan wrote:
> Create a new common API to replace the virCgroupNew{Vcpu|Emulator|IOThread}
> API's using an emum to generate the cgroup name
> 
> Signed-off-by: John Ferlan <jferlan redhat com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/vircgroup.c     | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/vircgroup.h     | 15 ++++++++++
>  3 files changed, 90 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9f82926..0800cb6 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1186,6 +1186,7 @@ virCgroupNewIOThread;
>  virCgroupNewMachine;
>  virCgroupNewPartition;
>  virCgroupNewSelf;
> +virCgroupNewThread;
>  virCgroupNewVcpu;
>  virCgroupPathOfController;
>  virCgroupRemove;
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index d42f433..7fec0cc 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -1422,6 +1422,67 @@ virCgroupNewDomainPartition(virCgroupPtr partition,
>  
>  
>  /**
> + * virCgroupNewThread:
> + *
> + * @domain: group for the domain
> + * @name: enum to generate the name for the new thread
> + * @id: id of the vcpu or iothread
> + * @create: true to create if not already existing
> + * @group: Pointer to returned virCgroupPtr
> + *
> + * Returns 0 on success, or -1 on error
> + */
> +int
> +virCgroupNewThread(virCgroupPtr domain,
> +                   virCgroupThreadName nameval,
> +                   int id,
> +                   bool create,
> +                   virCgroupPtr *group)
> +{
> +    int ret = -1;
> +    char *name = NULL;
> +    int controllers;
> +
> +    switch (nameval) {
> +    case VIR_CGROUP_VCPU_NAME:
> +        if (virAsprintf(&name, "vcpu%d", id) < 0)
> +            goto cleanup;
> +        break;
> +    case VIR_CGROUP_EMULATOR_NAME:
> +        if (VIR_STRDUP(name, "emulator") < 0)
> +            goto cleanup;
> +        break;
> +    case VIR_CGROUP_IOTHREAD_NAME:
> +        if (virAsprintf(&name, "iothread%d", id) < 0)
> +            goto cleanup;
> +        break;
> +    case VIR_CGROUP_NAME_LAST:
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("unexpected name value %d"), nameval);
> +        goto cleanup;
> +    }
> +
> +    controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) |
> +                   (1 << VIR_CGROUP_CONTROLLER_CPUACCT) |
> +                   (1 << VIR_CGROUP_CONTROLLER_CPUSET));
> +
> +    if (virCgroupNew(-1, name, domain, controllers, group) < 0)
> +        goto cleanup;
> +
> +    if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) {
> +        virCgroupRemove(*group);
> +        virCgroupFree(group);
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(name);
> +    return ret;
> +}
> +
> +
> +/**
>   * virCgroupNewVcpu:
>   *
>   * @domain: group for the domain
> @@ -4066,6 +4127,19 @@ virCgroupNewDomainPartition(virCgroupPtr partition ATTRIBUTE_UNUSED,
>  
>  
>  int
> +virCgroupNewThread(virCgroupPtr domain ATTRIBUTE_UNUSED,
> +                   virCgroupThreadName nameval ATTRIBUTE_UNUSED,
> +                   int id ATTRIBUTE_UNUSED,
> +                   bool create ATTRIBUTE_UNUSED,
> +                   virCgroupPtr *group ATTRIBUTE_UNUSED)
> +{
> +    virReportSystemError(ENXIO, "%s",
> +                         _("Control groups not supported on this platform"));
> +    return -1;
> +}
> +
> +
> +int
>  virCgroupNewVcpu(virCgroupPtr domain ATTRIBUTE_UNUSED,
>                   int vcpuid ATTRIBUTE_UNUSED,
>                   bool create ATTRIBUTE_UNUSED,
> diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
> index eee15ca..a756b03 100644
> --- a/src/util/vircgroup.h
> +++ b/src/util/vircgroup.h
> @@ -52,6 +52,14 @@ VIR_ENUM_DECL(virCgroupController);
>   * Make sure we will not overflow */
>  verify(VIR_CGROUP_CONTROLLER_LAST < 8 * sizeof(int));
>  
> +typedef enum {
> +    VIR_CGROUP_VCPU_NAME = 0,
> +    VIR_CGROUP_EMULATOR_NAME,
> +    VIR_CGROUP_IOTHREAD_NAME,
> +
> +    VIR_CGROUP_NAME_LAST
> +} virCgroupThreadName;
> +

The enum name does not match the prefix of its values
and even the prefix is not the same.

ACK if you change it to satisfy both conditions, e.g.:

typedef enum {
VIR_CGROUP_THREAD_VCPU
VIR_CGROUP_THREAD_EMULATOR
VIR_CGROUP_THREAD_IOTHREAD
VIR_CGROUP_THREAD_LAST
} virCgroupThread;

Jan

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]