Re: [PATCH 1/6] qga: Add process termination functionality

[Konstantin Kostiuk <kkostiuk@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5767 bytes --]

On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:

> We need to terminate processes executed with guest-exec command. Add
> guest-exec-terminate command for process termination by PID.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  qga/commands-common.h |  2 ++
>  qga/commands-win32.c  | 64 +++++++++++++++++++++++++++++++++++++++++++
>  qga/commands.c        | 34 +++++++++++++++++++++++
>  qga/qapi-schema.json  | 13 +++++++++
>  4 files changed, 113 insertions(+)
>
> diff --git a/qga/commands-common.h b/qga/commands-common.h
> index 8c1c56aac9..34b9a22578 100644
> --- a/qga/commands-common.h
> +++ b/qga/commands-common.h
> @@ -80,4 +80,6 @@ GuestFileRead *guest_file_read_unsafe(GuestFileHandle
> *gfh,
>   */
>  char *qga_get_host_name(Error **errp);
>
> +int kill_process_tree(int64_t pid);
> +
>  #endif
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 697c65507c..5aa43a9ed7 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -27,6 +27,7 @@
>  #include <lm.h>
>  #include <wtsapi32.h>
>  #include <wininet.h>
> +#include <tlhelp32.h>
>
>  #include "guest-agent-core.h"
>  #include "vss-win32.h"
> @@ -2522,3 +2523,66 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error
> **errp)
>      error_setg(errp, QERR_UNSUPPORTED);
>      return NULL;
>  }
> +
> +int kill_process_tree(int64_t pid)
> +{
> +    PROCESSENTRY32 proc_entry;
> +    HANDLE snapshot, process;
> +    GList *pid_entry, *pid_list = NULL;
> +    bool added, success;
> +    int res = 0;
> +
> +    snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
> +    if (snapshot == INVALID_HANDLE_VALUE) {
> +        return GetLastError();
> +    }
> +
> +    pid_list = g_list_append(pid_list, GUINT_TO_POINTER(pid));
> +
> +    proc_entry.dwSize = sizeof(PROCESSENTRY32);
> +    do {
> +        added = false;
> +        for (success = Process32First(snapshot, &proc_entry);
> +             success; success = Process32Next(snapshot, &proc_entry)) {
> +            gpointer ppid_p, pid_p;
> +            ppid_p = GUINT_TO_POINTER(proc_entry.th32ParentProcessID);
> +            pid_p = GUINT_TO_POINTER(proc_entry.th32ProcessID);
> +            if (g_list_find(pid_list, ppid_p) && !g_list_find(pid_list,
> pid_p)) {
> +                pid_list = g_list_append(pid_list, pid_p);
> +                added = true;
> +            }
> +        }
> +    } while (added);
>
+
> +    for (success = Process32First(snapshot, &proc_entry);
> +         success; success = Process32Next(snapshot, &proc_entry)) {
> +        if (g_list_find(pid_list,
> GUINT_TO_POINTER(proc_entry.th32ProcessID))) {
> +            g_debug("killing pid=%u ppid=%u name=%s",
> +                (guint)proc_entry.th32ProcessID,
> +                (guint)proc_entry.th32ParentProcessID,
> +                proc_entry.szExeFile);
> +        }
> +    }
> +
>


Why do we need to store these processes before termination?
I understand that we need to enumerate all processes to find children
but why we can't terminate it on the fly?


> +    CloseHandle(snapshot);
> +
> +    for (pid_entry = pid_list; pid_entry; pid_entry = pid_entry->next) {
> +        pid = GPOINTER_TO_UINT(pid_entry->data);
> +        process = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
> +        if (process == INVALID_HANDLE_VALUE) {
> +            if (!res) {
> +                res = GetLastError();
> +                if (res == ERROR_FILE_NOT_FOUND) {
> +                    res = 0;
> +                }
> +            }
> +            continue;
> +        }
> +        TerminateProcess(process, 255);
> +        CloseHandle(process);
> +    }
> +
> +    g_list_free(pid_list);
> +
> +    return res;
> +}
> diff --git a/qga/commands.c b/qga/commands.c
> index ce172edd2d..af8459c587 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -529,6 +529,40 @@ done:
>      return ge;
>  }
>
> +void qmp_guest_exec_terminate(int64_t pid, Error **errp)
> +{
> +    GuestExecInfo *gei;
> +
> +    slog("guest-exec-terminate called, pid: %u", (uint32_t)pid);
> +
> +    gei = guest_exec_info_find(pid);
> +    if (gei == NULL) {
> +        error_setg(errp, QERR_INVALID_PARAMETER, "pid");
> +        return;
> +    }
> +
> +    if (gei->finished) {
> +        return;
> +    }
> +
> +#ifdef G_OS_WIN32
> +    char buf[32];
> +    int res;
> +
> +    res = kill_process_tree(pid);
> +    if (res != 0) {
> +        snprintf(buf, sizeof(buf), "win32 err %d", res);
> +        error_setg(errp, QERR_QGA_COMMAND_FAILED, buf);
> +    }
> +#else
> +    if (kill(pid, SIGKILL) < 0) {
> +        if (errno != ESRCH) {
> +            error_setg(errp, QERR_QGA_COMMAND_FAILED, strerror(errno));
> +        }
> +    }
> +#endif
> +}
> +
>  /* Convert GuestFileWhence (either a raw integer or an enum value) into
>   * the guest's SEEK_ constants.  */
>  int ga_parse_whence(GuestFileWhence *whence, Error **errp)
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 876e2a8ea8..b39be4cdc2 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1326,6 +1326,19 @@
>                 '*input-data': 'str', '*capture-output':
> 'GuestExecCaptureOutput' },
>    'returns': 'GuestExec' }
>
> +##
> +# @guest-exec-terminate:
> +#
> +# Terminate process associated with PID retrieved via guest-exec.
> +#
> +# @pid: pid returned from guest-exec
> +#
> +# Returns: Nothing on success.
> +#
> +# Since: 8.2
> +##
> +{ 'command': 'guest-exec-terminate',
> +  'data':    { 'pid': 'int' } }
>
>  ##
>  # @GuestHostName:
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 7393 bytes --]