[libvirt] [PATCH 6/9] qemu_cgroup: use virCgroupAddTask instead of virCgroupMoveTask

[Henning Schild]

Is it ok do drop unused code in the same patch that makes the code
obsolete, or should i split that up?

On Tue, 23 Feb 2016 16:58:41 +0100
Henning Schild <henning.schild at siemens.com> wrote:

> qemuSetupCgroupForEmulator runs at a point in time where there is only
> the qemu main thread. Use virCgroupAddTask to put just that one task
> into the emulator cgroup. That patch makes virCgroupMoveTask and
> virCgroupAddTaskStrController obsolete.
> 
> Signed-off-by: Henning Schild <henning.schild at siemens.com>
> ---
>  src/libvirt_private.syms |   1 -
>  src/qemu/qemu_cgroup.c   |   4 +-
>  src/util/vircgroup.c     | 102
> -----------------------------------------------
> src/util/vircgroup.h     |   3 -- 4 files changed, 2 insertions(+),
> 108 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index cf93d06..c5e57bf 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1234,7 +1234,6 @@ virCgroupIsolateMount;
>  virCgroupKill;
>  virCgroupKillPainfully;
>  virCgroupKillRecursive;
> -virCgroupMoveTask;
>  virCgroupNewDetect;
>  virCgroupNewDetectMachine;
>  virCgroupNewDomainPartition;
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 66dc782..d410a66 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -1145,8 +1145,8 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm)
>                             true, &cgroup_emulator) < 0)
>          goto cleanup;
>  
> -    if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0)
> -        goto cleanup;
> +    if (virCgroupAddTask(cgroup_emulator, vm->pid) < 0)
> +       goto cleanup;
>  
>      if (def->cputune.emulatorpin)
>          cpumask = def->cputune.emulatorpin;
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index aef8e8c..c31c83b 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -1209,50 +1209,6 @@ virCgroupAddTaskController(virCgroupPtr group,
> pid_t pid, int controller) }
>  
>  
> -static int
> -virCgroupAddTaskStrController(virCgroupPtr group,
> -                              const char *pidstr,
> -                              int controller)
> -{
> -    char *str = NULL, *cur = NULL, *next = NULL;
> -    unsigned long long p = 0;
> -    int rc = 0;
> -    char *endp;
> -
> -    if (VIR_STRDUP(str, pidstr) < 0)
> -        return -1;
> -
> -    cur = str;
> -    while (*cur != '\0') {
> -        if (virStrToLong_ull(cur, &endp, 10, &p) < 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           _("Cannot parse '%s' as an integer"),
> cur);
> -            goto cleanup;
> -        }
> -
> -        if (virCgroupAddTaskController(group, p, controller) < 0) {
> -            /* A thread that exits between when we first read the
> source
> -             * tasks and now is not fatal.  */
> -            if (virLastErrorIsSystemErrno(ESRCH))
> -                virResetLastError();
> -            else
> -                goto cleanup;
> -        }
> -
> -        next = strchr(cur, '\n');
> -        if (next) {
> -            cur = next + 1;
> -            *next = '\0';
> -        } else {
> -            break;
> -        }
> -    }
> -
> - cleanup:
> -    VIR_FREE(str);
> -    return rc;
> -}
> -
>  void
>  virCgroupSetAssertEmpty(virCgroupPtr group, int mask) {
>      group->assert_empty = mask;
> @@ -1264,54 +1220,6 @@ virCgroupGetAssertEmpty(virCgroupPtr group) {
>  }
>  
>  
> -/**
> - * virCgroupMoveTask:
> - *
> - * @src_group: The source cgroup where all tasks are removed from
> - * @dest_group: The destination where all tasks are added to
> - *
> - * Returns: 0 on success or -1 on failure
> - */
> -int
> -virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group)
> -{
> -    int ret = -1;
> -    char *content = NULL;
> -    size_t i;
> -
> -    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
> -        if (!src_group->controllers[i].mountPoint ||
> -            !dest_group->controllers[i].mountPoint)
> -            continue;
> -
> -        /* We must never move tasks in systemd's hierarchy */
> -        if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
> -            continue;
> -
> -        /* New threads are created in the same group as their parent;
> -         * but if a thread is created after we first read we aren't
> -         * aware that it needs to move.  Therefore, we must iterate
> -         * until content is empty.  */
> -        while (1) {
> -            VIR_FREE(content);
> -            if (virCgroupGetValueStr(src_group, i, "tasks",
> &content) < 0)
> -                return -1;
> -
> -            if (!*content)
> -                break;
> -
> -            if (virCgroupAddTaskStrController(dest_group, content,
> i) < 0)
> -                goto cleanup;
> -        }
> -    }
> -
> -    ret = 0;
> - cleanup:
> -    VIR_FREE(content);
> -    return ret;
> -}
> -
> -
>  static int
>  virCgroupSetPartitionSuffix(const char *path, char **res)
>  {
> @@ -4309,16 +4217,6 @@ virCgroupAddTaskController(virCgroupPtr group
> ATTRIBUTE_UNUSED, 
>  
>  int
> -virCgroupMoveTask(virCgroupPtr src_group ATTRIBUTE_UNUSED,
> -                  virCgroupPtr dest_group ATTRIBUTE_UNUSED)
> -{
> -    virReportSystemError(ENXIO, "%s",
> -                         _("Control groups not supported on this
> platform"));
> -    return -1;
> -}
> -
> -
> -int
>  virCgroupGetBlkioIoServiced(virCgroupPtr group ATTRIBUTE_UNUSED,
>                              long long *bytes_read ATTRIBUTE_UNUSED,
>                              long long *bytes_write ATTRIBUTE_UNUSED,
> diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
> index f71aed5..0286d56 100644
> --- a/src/util/vircgroup.h
> +++ b/src/util/vircgroup.h
> @@ -141,9 +141,6 @@ int virCgroupAddTaskController(virCgroupPtr group,
>  void virCgroupSetAssertEmpty(virCgroupPtr group, int mask);
>  int virCgroupGetAssertEmpty(virCgroupPtr group);
>  
> -int virCgroupMoveTask(virCgroupPtr src_group,
> -                      virCgroupPtr dest_group);
> -
>  int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight);
>  int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int
> *weight);