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@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
> 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);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list