* [PATCH 1/6] qga: Add process termination functionality
2023-10-25 14:00 [PATCH 0/6] qga: Assorted patches, let us discuss Alexander Ivanov
@ 2023-10-25 14:00 ` Alexander Ivanov
2023-10-26 8:24 ` Konstantin Kostiuk
2023-10-25 14:00 ` [PATCH 2/6] qga: Move command execution code to a separate function Alexander Ivanov
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Alexander Ivanov @ 2023-10-25 14:00 UTC (permalink / raw)
To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau
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);
+ }
+ }
+
+ 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/6] qga: Add process termination functionality
2023-10-25 14:00 ` [PATCH 1/6] qga: Add process termination functionality Alexander Ivanov
@ 2023-10-26 8:24 ` Konstantin Kostiuk
0 siblings, 0 replies; 14+ messages in thread
From: Konstantin Kostiuk @ 2023-10-26 8:24 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: qemu-devel, den, michael.roth, marcandre.lureau
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6] qga: Move command execution code to a separate function
2023-10-25 14:00 [PATCH 0/6] qga: Assorted patches, let us discuss Alexander Ivanov
2023-10-25 14:00 ` [PATCH 1/6] qga: Add process termination functionality Alexander Ivanov
@ 2023-10-25 14:00 ` Alexander Ivanov
2023-10-26 8:28 ` Konstantin Kostiuk
2023-10-25 14:00 ` [PATCH 3/6] qga: Let run_command() work without input data Alexander Ivanov
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Alexander Ivanov @ 2023-10-25 14:00 UTC (permalink / raw)
To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau
In qmp_guest_set_user_password() we have a part of code that we can reuse
in the future commits. Move this code to a separate function.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
qga/commands-posix.c | 139 ++++++++++++++++++++++---------------------
1 file changed, 72 insertions(+), 67 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 6169bbf7a0..e7b82aaf37 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2114,20 +2114,78 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
#endif /* __linux__ */
#if defined(__linux__) || defined(__FreeBSD__)
-void qmp_guest_set_user_password(const char *username,
- const char *password,
- bool crypted,
- Error **errp)
+
+static void run_command(const char *argv[], const char *in_str, Error **errp)
{
Error *local_err = NULL;
- char *passwd_path = NULL;
pid_t pid;
- int status;
+ int in_len, status;
int datafd[2] = { -1, -1 };
+
+ if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
+ error_setg(errp, "cannot create pipe FDs");
+ goto out;
+ }
+
+ pid = fork();
+ if (pid == 0) {
+ close(datafd[1]);
+ setsid();
+ dup2(datafd[0], 0);
+ reopen_fd_to_null(1);
+ reopen_fd_to_null(2);
+
+ execve(argv[0], (char *const *)argv, environ);
+ _exit(EXIT_FAILURE);
+ } else if (pid < 0) {
+ error_setg_errno(errp, errno, "failed to create child process");
+ goto out;
+ }
+ close(datafd[0]);
+ datafd[0] = -1;
+
+ in_len = strlen(in_str);
+
+ if (qemu_write_full(datafd[1], in_str, in_len) != in_len) {
+ error_setg_errno(errp, errno, "cannot write new account password");
+ goto out;
+ }
+ close(datafd[1]);
+ datafd[1] = -1;
+
+ ga_wait_child(pid, &status, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ if (!WIFEXITED(status)) {
+ error_setg(errp, "child process has terminated abnormally");
+ goto out;
+ }
+
+ if (WEXITSTATUS(status)) {
+ error_setg(errp, "child process has failed: %s", argv[0]);
+ }
+
+out:
+ if (datafd[0] != -1) {
+ close(datafd[0]);
+ }
+ if (datafd[1] != -1) {
+ close(datafd[1]);
+ }
+}
+
+void qmp_guest_set_user_password(const char *username,
+ const char *password,
+ bool crypted,
+ Error **errp)
+{
+ char *passwd_path = NULL;
char *rawpasswddata = NULL;
- size_t rawpasswdlen;
char *chpasswddata = NULL;
- size_t chpasswdlen;
+ size_t rawpasswdlen;
rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, errp);
if (!rawpasswddata) {
@@ -2155,79 +2213,26 @@ void qmp_guest_set_user_password(const char *username,
passwd_path = g_find_program_in_path("chpasswd");
#endif
- chpasswdlen = strlen(chpasswddata);
-
if (!passwd_path) {
error_setg(errp, "cannot find 'passwd' program in PATH");
goto out;
}
- if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
- error_setg(errp, "cannot create pipe FDs");
- goto out;
- }
-
- pid = fork();
- if (pid == 0) {
- close(datafd[1]);
- /* child */
- setsid();
- dup2(datafd[0], 0);
- reopen_fd_to_null(1);
- reopen_fd_to_null(2);
-
+ const char *argv[] = {
#ifdef __FreeBSD__
- const char *h_arg;
- h_arg = (crypted) ? "-H" : "-h";
- execl(passwd_path, "pw", "usermod", "-n", username, h_arg, "0", NULL);
+ passwd_path, "pw", "usermod", "-n", username,
+ (crypted) ? "-H" : "-h", "0", NULL};
#else
- if (crypted) {
- execl(passwd_path, "chpasswd", "-e", NULL);
- } else {
- execl(passwd_path, "chpasswd", NULL);
- }
+ passwd_path, "chpasswd", (crypted) ? "-e" : NULL, NULL
#endif
- _exit(EXIT_FAILURE);
- } else if (pid < 0) {
- error_setg_errno(errp, errno, "failed to create child process");
- goto out;
- }
- close(datafd[0]);
- datafd[0] = -1;
+ };
- if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != chpasswdlen) {
- error_setg_errno(errp, errno, "cannot write new account password");
- goto out;
- }
- close(datafd[1]);
- datafd[1] = -1;
-
- ga_wait_child(pid, &status, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- if (!WIFEXITED(status)) {
- error_setg(errp, "child process has terminated abnormally");
- goto out;
- }
-
- if (WEXITSTATUS(status)) {
- error_setg(errp, "child process has failed to set user password");
- goto out;
- }
+ run_command(argv, chpasswddata, errp);
out:
g_free(chpasswddata);
g_free(rawpasswddata);
g_free(passwd_path);
- if (datafd[0] != -1) {
- close(datafd[0]);
- }
- if (datafd[1] != -1) {
- close(datafd[1]);
- }
}
#else /* __linux__ || __FreeBSD__ */
void qmp_guest_set_user_password(const char *username,
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/6] qga: Move command execution code to a separate function
2023-10-25 14:00 ` [PATCH 2/6] qga: Move command execution code to a separate function Alexander Ivanov
@ 2023-10-26 8:28 ` Konstantin Kostiuk
0 siblings, 0 replies; 14+ messages in thread
From: Konstantin Kostiuk @ 2023-10-26 8:28 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: qemu-devel, den, michael.roth, marcandre.lureau
[-- Attachment #1: Type: text/plain, Size: 6071 bytes --]
On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:
> In qmp_guest_set_user_password() we have a part of code that we can reuse
> in the future commits. Move this code to a separate function.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> qga/commands-posix.c | 139 ++++++++++++++++++++++---------------------
> 1 file changed, 72 insertions(+), 67 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 6169bbf7a0..e7b82aaf37 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2114,20 +2114,78 @@ int64_t
> qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
> #endif /* __linux__ */
>
> #if defined(__linux__) || defined(__FreeBSD__)
> -void qmp_guest_set_user_password(const char *username,
> - const char *password,
> - bool crypted,
> - Error **errp)
> +
> +static void run_command(const char *argv[], const char *in_str, Error
> **errp)
> {
> Error *local_err = NULL;
> - char *passwd_path = NULL;
> pid_t pid;
> - int status;
> + int in_len, status;
> int datafd[2] = { -1, -1 };
> +
> + if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
> + error_setg(errp, "cannot create pipe FDs");
> + goto out;
> + }
> +
> + pid = fork();
> + if (pid == 0) {
> + close(datafd[1]);
> + setsid();
> + dup2(datafd[0], 0);
> + reopen_fd_to_null(1);
> + reopen_fd_to_null(2);
> +
> + execve(argv[0], (char *const *)argv, environ);
> + _exit(EXIT_FAILURE);
> + } else if (pid < 0) {
> + error_setg_errno(errp, errno, "failed to create child process");
> + goto out;
> + }
> + close(datafd[0]);
> + datafd[0] = -1;
> +
> + in_len = strlen(in_str);
> +
> + if (qemu_write_full(datafd[1], in_str, in_len) != in_len) {
> + error_setg_errno(errp, errno, "cannot write new account
> password");
>
As this is a generic `run_command` function, please generalize errors too.
> + goto out;
> + }
> + close(datafd[1]);
> + datafd[1] = -1;
> +
> + ga_wait_child(pid, &status, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + goto out;
> + }
> +
> + if (!WIFEXITED(status)) {
> + error_setg(errp, "child process has terminated abnormally");
> + goto out;
> + }
> +
> + if (WEXITSTATUS(status)) {
> + error_setg(errp, "child process has failed: %s", argv[0]);
> + }
> +
> +out:
> + if (datafd[0] != -1) {
> + close(datafd[0]);
> + }
> + if (datafd[1] != -1) {
> + close(datafd[1]);
> + }
> +}
> +
> +void qmp_guest_set_user_password(const char *username,
> + const char *password,
> + bool crypted,
> + Error **errp)
> +{
> + char *passwd_path = NULL;
> char *rawpasswddata = NULL;
> - size_t rawpasswdlen;
> char *chpasswddata = NULL;
> - size_t chpasswdlen;
> + size_t rawpasswdlen;
>
> rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen,
> errp);
> if (!rawpasswddata) {
> @@ -2155,79 +2213,26 @@ void qmp_guest_set_user_password(const char
> *username,
> passwd_path = g_find_program_in_path("chpasswd");
> #endif
>
> - chpasswdlen = strlen(chpasswddata);
> -
> if (!passwd_path) {
> error_setg(errp, "cannot find 'passwd' program in PATH");
> goto out;
> }
>
> - if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
> - error_setg(errp, "cannot create pipe FDs");
> - goto out;
> - }
> -
> - pid = fork();
> - if (pid == 0) {
> - close(datafd[1]);
> - /* child */
> - setsid();
> - dup2(datafd[0], 0);
> - reopen_fd_to_null(1);
> - reopen_fd_to_null(2);
> -
> + const char *argv[] = {
> #ifdef __FreeBSD__
> - const char *h_arg;
> - h_arg = (crypted) ? "-H" : "-h";
> - execl(passwd_path, "pw", "usermod", "-n", username, h_arg, "0",
> NULL);
> + passwd_path, "pw", "usermod", "-n", username,
> + (crypted) ? "-H" : "-h", "0", NULL};
> #else
> - if (crypted) {
> - execl(passwd_path, "chpasswd", "-e", NULL);
> - } else {
> - execl(passwd_path, "chpasswd", NULL);
> - }
> + passwd_path, "chpasswd", (crypted) ? "-e" : NULL, NULL
> #endif
> - _exit(EXIT_FAILURE);
> - } else if (pid < 0) {
> - error_setg_errno(errp, errno, "failed to create child process");
> - goto out;
> - }
> - close(datafd[0]);
> - datafd[0] = -1;
> + };
>
> - if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) !=
> chpasswdlen) {
> - error_setg_errno(errp, errno, "cannot write new account
> password");
> - goto out;
> - }
> - close(datafd[1]);
> - datafd[1] = -1;
> -
> - ga_wait_child(pid, &status, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - goto out;
> - }
> -
> - if (!WIFEXITED(status)) {
> - error_setg(errp, "child process has terminated abnormally");
> - goto out;
> - }
> -
> - if (WEXITSTATUS(status)) {
> - error_setg(errp, "child process has failed to set user password");
> - goto out;
> - }
> + run_command(argv, chpasswddata, errp);
>
> out:
> g_free(chpasswddata);
> g_free(rawpasswddata);
> g_free(passwd_path);
> - if (datafd[0] != -1) {
> - close(datafd[0]);
> - }
> - if (datafd[1] != -1) {
> - close(datafd[1]);
> - }
> }
> #else /* __linux__ || __FreeBSD__ */
> void qmp_guest_set_user_password(const char *username,
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 7799 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/6] qga: Let run_command() work without input data
2023-10-25 14:00 [PATCH 0/6] qga: Assorted patches, let us discuss Alexander Ivanov
2023-10-25 14:00 ` [PATCH 1/6] qga: Add process termination functionality Alexander Ivanov
2023-10-25 14:00 ` [PATCH 2/6] qga: Move command execution code to a separate function Alexander Ivanov
@ 2023-10-25 14:00 ` Alexander Ivanov
2023-10-25 14:00 ` [PATCH 4/6] qga: Add user creation functionality Alexander Ivanov
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Alexander Ivanov @ 2023-10-25 14:00 UTC (permalink / raw)
To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau
run_command() has in_str argument that specifies the input string.
Let it be NULL if there is no input.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
qga/commands-posix.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index e7b82aaf37..461b4d7bb6 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2122,16 +2122,22 @@ static void run_command(const char *argv[], const char *in_str, Error **errp)
int in_len, status;
int datafd[2] = { -1, -1 };
- if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
- error_setg(errp, "cannot create pipe FDs");
- goto out;
+ if (in_str) {
+ if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
+ error_setg(errp, "cannot create pipe FDs");
+ goto out;
+ }
}
pid = fork();
if (pid == 0) {
- close(datafd[1]);
setsid();
- dup2(datafd[0], 0);
+ if (in_str) {
+ close(datafd[1]);
+ dup2(datafd[0], 0);
+ } else {
+ reopen_fd_to_null(0);
+ }
reopen_fd_to_null(1);
reopen_fd_to_null(2);
@@ -2141,17 +2147,20 @@ static void run_command(const char *argv[], const char *in_str, Error **errp)
error_setg_errno(errp, errno, "failed to create child process");
goto out;
}
- close(datafd[0]);
- datafd[0] = -1;
- in_len = strlen(in_str);
+ if (in_str) {
+ close(datafd[0]);
+ datafd[0] = -1;
- if (qemu_write_full(datafd[1], in_str, in_len) != in_len) {
- error_setg_errno(errp, errno, "cannot write new account password");
- goto out;
+ in_len = strlen(in_str);
+ if (qemu_write_full(datafd[1], in_str, in_len) != in_len) {
+ error_setg_errno(errp, errno, "cannot write new account password");
+ goto out;
+ }
+
+ close(datafd[1]);
+ datafd[1] = -1;
}
- close(datafd[1]);
- datafd[1] = -1;
ga_wait_child(pid, &status, &local_err);
if (local_err) {
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/6] qga: Add user creation functionality
2023-10-25 14:00 [PATCH 0/6] qga: Assorted patches, let us discuss Alexander Ivanov
` (2 preceding siblings ...)
2023-10-25 14:00 ` [PATCH 3/6] qga: Let run_command() work without input data Alexander Ivanov
@ 2023-10-25 14:00 ` Alexander Ivanov
2023-10-25 14:00 ` [PATCH 5/6] qga: Add timeout for fsfreeze Alexander Ivanov
2023-10-25 14:00 ` [PATCH 6/6] qga: Cancel async snapshot before abort Alexander Ivanov
5 siblings, 0 replies; 14+ messages in thread
From: Alexander Ivanov @ 2023-10-25 14:00 UTC (permalink / raw)
To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau
Add an optional argument "create" to guest-set-user-password command to
create a user with provided username and password.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
qga/commands-posix.c | 19 +++++++++++++++++++
qga/commands-win32.c | 22 ++++++++++++++++++++++
qga/qapi-schema.json | 5 ++++-
3 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 461b4d7bb6..26711a1a72 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2189,6 +2189,7 @@ out:
void qmp_guest_set_user_password(const char *username,
const char *password,
bool crypted,
+ bool has_create, bool create,
Error **errp)
{
char *passwd_path = NULL;
@@ -2227,6 +2228,24 @@ void qmp_guest_set_user_password(const char *username,
goto out;
}
+ /* create new user if requested */
+ if (has_create && create) {
+ char *str = g_shell_quote(username);
+ char *cmd = g_strdup_printf(
+ /* we want output only from useradd command */
+ "id -u %s >/dev/null 2>&1 || useradd -m %s",
+ str, str);
+ const char *argv[] = {
+ "/bin/sh", "-c", cmd, NULL
+ };
+ run_command(argv, NULL, errp);
+ g_free(str);
+ g_free(cmd);
+ if (*errp) {
+ goto out;
+ }
+ }
+
const char *argv[] = {
#ifdef __FreeBSD__
passwd_path, "pw", "usermod", "-n", username,
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 5aa43a9ed7..618d862c00 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1921,6 +1921,7 @@ get_net_error_message(gint error)
void qmp_guest_set_user_password(const char *username,
const char *password,
bool crypted,
+ bool has_create, bool create,
Error **errp)
{
NET_API_STATUS nas;
@@ -1952,6 +1953,27 @@ void qmp_guest_set_user_password(const char *username,
goto done;
}
+ if (has_create && create) {
+ USER_INFO_1 ui = { 0 };
+
+ ui.usri1_name = user;
+ ui.usri1_password = wpass;
+ ui.usri1_priv = USER_PRIV_USER;
+ ui.usri1_flags = UF_SCRIPT | UF_DONT_EXPIRE_PASSWD;
+ nas = NetUserAdd(NULL, 1, (LPBYTE) & ui, NULL);
+
+ if (nas == NERR_Success) {
+ goto done;
+ }
+
+ if (nas != NERR_UserExists) {
+ gchar *msg = get_net_error_message(nas);
+ error_setg(errp, "failed to add user: %s", msg);
+ g_free(msg);
+ goto done;
+ }
+ }
+
pi1003.usri1003_password = wpass;
nas = NetUserSetInfo(NULL, user,
1003, (LPBYTE)&pi1003,
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index b39be4cdc2..e96d463639 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1059,6 +1059,8 @@
# @password: the new password entry string, base64 encoded
#
# @crypted: true if password is already crypt()d, false if raw
+# @create: #optinal user will be created if it does not exist yet.
+# The default value is false. (since 8.2)
#
# If the @crypted flag is true, it is the caller's responsibility to
# ensure the correct crypt() encryption scheme is used. This command
@@ -1078,7 +1080,8 @@
# Since: 2.3
##
{ 'command': 'guest-set-user-password',
- 'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool' } }
+ 'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool',
+ '*create': 'bool' } }
##
# @GuestMemoryBlock:
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/6] qga: Add timeout for fsfreeze
2023-10-25 14:00 [PATCH 0/6] qga: Assorted patches, let us discuss Alexander Ivanov
` (3 preceding siblings ...)
2023-10-25 14:00 ` [PATCH 4/6] qga: Add user creation functionality Alexander Ivanov
@ 2023-10-25 14:00 ` Alexander Ivanov
2023-10-26 9:16 ` Konstantin Kostiuk
2023-10-25 14:00 ` [PATCH 6/6] qga: Cancel async snapshot before abort Alexander Ivanov
5 siblings, 1 reply; 14+ messages in thread
From: Alexander Ivanov @ 2023-10-25 14:00 UTC (permalink / raw)
To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau
In some cases it would be useful to thaw a filesystem by timeout after
freezing this filesystem by guest-fsfreeze-freeze-list. Add an optional
argument "timeout" to the command.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
qga/commands-posix.c | 21 ++++++++++++++++++---
qga/commands-win32.c | 16 ++++++++++++++--
qga/guest-agent-core.h | 3 ++-
qga/main.c | 19 ++++++++++++++++++-
qga/qapi-schema.json | 9 ++++++++-
5 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 26711a1a72..e8a79e0a41 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -707,13 +707,17 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
return GUEST_FSFREEZE_STATUS_THAWED;
}
-int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
+ Error **errp)
{
- return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+ return qmp_guest_fsfreeze_freeze_list(false, NULL, has_timeout, timeout,
+ errp);
}
int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
strList *mountpoints,
+ bool has_timeout,
+ int64_t timeout,
Error **errp)
{
int ret;
@@ -734,8 +738,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
return -1;
}
+ if (!has_timeout || timeout < 0) {
+ timeout = 0;
+ }
/* cannot risk guest agent blocking itself on a write in this state */
- ga_set_frozen(ga_state);
+ ga_set_frozen(ga_state, timeout);
ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints, mountpoints,
mounts, errp);
@@ -780,6 +787,12 @@ static void guest_fsfreeze_cleanup(void)
}
}
}
+
+gboolean ga_frozen_timeout_cb(gpointer data)
+{
+ guest_fsfreeze_cleanup();
+ return G_SOURCE_REMOVE;
+}
#endif
/* linux-specific implementations. avoid this if at all possible. */
@@ -3119,6 +3132,8 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
strList *mountpoints,
+ bool has_timeout,
+ int64_t timeout,
Error **errp)
{
error_setg(errp, QERR_UNSUPPORTED);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 618d862c00..51fd6dcd58 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1221,13 +1221,16 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
* Freeze local file systems using Volume Shadow-copy Service.
* The frozen state is limited for up to 10 seconds by VSS.
*/
-int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
+ Error **errp)
{
return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
}
int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
strList *mountpoints,
+ bool has_timeout,
+ int64_t timeout,
Error **errp)
{
int i;
@@ -1240,8 +1243,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
slog("guest-fsfreeze called");
+ if (!has_timeout || timeout < 0) {
+ timeout = 0;
+ }
/* cannot risk guest agent blocking itself on a write in this state */
- ga_set_frozen(ga_state);
+ ga_set_frozen(ga_state, timeout);
qga_vss_fsfreeze(&i, true, mountpoints, &local_err);
if (local_err) {
@@ -1299,6 +1305,12 @@ static void guest_fsfreeze_cleanup(void)
vss_deinit(true);
}
+gboolean ga_frozen_timeout_cb(gpointer data)
+{
+ guest_fsfreeze_cleanup();
+ return G_SOURCE_REMOVE;
+}
+
/*
* Walk list of mounted file systems in the guest, and discard unused
* areas.
diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
index b4e7c52c61..d8d1bb9505 100644
--- a/qga/guest-agent-core.h
+++ b/qga/guest-agent-core.h
@@ -39,8 +39,9 @@ void ga_enable_logging(GAState *s);
void G_GNUC_PRINTF(1, 2) slog(const gchar *fmt, ...);
void ga_set_response_delimited(GAState *s);
bool ga_is_frozen(GAState *s);
-void ga_set_frozen(GAState *s);
+void ga_set_frozen(GAState *s, int64_t timeout);
void ga_unset_frozen(GAState *s);
+gboolean ga_frozen_timeout_cb(gpointer data);
const char *ga_fsfreeze_hook(GAState *s);
int64_t ga_get_fd_handle(GAState *s, Error **errp);
int ga_parse_whence(GuestFileWhence *whence, Error **errp);
diff --git a/qga/main.c b/qga/main.c
index 8668b9f3d3..6c7c7d68d8 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -94,6 +94,7 @@ struct GAState {
const char *pid_filepath;
} deferred_options;
#ifdef CONFIG_FSFREEZE
+ guint frozen_timeout_id;
const char *fsfreeze_hook;
#endif
gchar *pstate_filepath;
@@ -478,7 +479,7 @@ bool ga_is_frozen(GAState *s)
return s->frozen;
}
-void ga_set_frozen(GAState *s)
+void ga_set_frozen(GAState *s, int64_t timeout)
{
if (ga_is_frozen(s)) {
return;
@@ -492,6 +493,15 @@ void ga_set_frozen(GAState *s)
g_warning("unable to create %s, fsfreeze may not function properly",
s->state_filepath_isfrozen);
}
+#ifdef CONFIG_FSFREEZE
+ if (timeout) {
+ s->frozen_timeout_id = g_timeout_add_seconds(timeout,
+ ga_frozen_timeout_cb,
+ NULL);
+ } else {
+ s->frozen_timeout_id = 0;
+ }
+#endif
}
void ga_unset_frozen(GAState *s)
@@ -500,6 +510,13 @@ void ga_unset_frozen(GAState *s)
return;
}
+#ifdef CONFIG_FSFREEZE
+ /* remove timeout callback */
+ if (s->frozen_timeout_id) {
+ g_source_remove(s->frozen_timeout_id);
+ }
+#endif
+
/* if we delayed creation/opening of pid/log files due to being
* in a frozen state at start up, do it now
*/
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index e96d463639..29ad342f0a 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -440,6 +440,9 @@
# command succeeded, you may call @guest-fsfreeze-thaw later to
# unfreeze.
#
+# @timeout: after this period in seconds filesystems will be thawed
+# (since 8.2)
+#
# Note: On Windows, the command is implemented with the help of a
# Volume Shadow-copy Service DLL helper. The frozen state is
# limited for up to 10 seconds by VSS.
@@ -452,6 +455,7 @@
# Since: 0.15.0
##
{ 'command': 'guest-fsfreeze-freeze',
+ 'data': { '*timeout': 'int' },
'returns': 'int' }
##
@@ -464,13 +468,16 @@
# If omitted, every mounted filesystem is frozen. Invalid mount
# points are ignored.
#
+# @timeout: after this period in seconds filesystems will be thawed
+# (since 8.2)
+#
# Returns: Number of file systems currently frozen. On error, all
# filesystems will be thawed.
#
# Since: 2.2
##
{ 'command': 'guest-fsfreeze-freeze-list',
- 'data': { '*mountpoints': ['str'] },
+ 'data': { '*mountpoints': ['str'], '*timeout': 'int' },
'returns': 'int' }
##
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 5/6] qga: Add timeout for fsfreeze
2023-10-25 14:00 ` [PATCH 5/6] qga: Add timeout for fsfreeze Alexander Ivanov
@ 2023-10-26 9:16 ` Konstantin Kostiuk
2023-10-30 16:32 ` Alexander Ivanov
0 siblings, 1 reply; 14+ messages in thread
From: Konstantin Kostiuk @ 2023-10-26 9:16 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: qemu-devel, den, michael.roth, marcandre.lureau
[-- Attachment #1: Type: text/plain, Size: 8724 bytes --]
I think it is better to check that timeout <= 10 sec in the case of Windows.
Anyway this is a VSS limitation and FS will be unfrozen earlier if timeout
> 10 sec,
this can cause some misunderstanding from a user.
timeout option sounds good in the guest-fsfreeze-freeze command.
In guest-fsfreeze-freeze-list, it looks strange to me. Command returns
list but takes timeout option. Can you explain timeout usage in this
command?
On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:
> In some cases it would be useful to thaw a filesystem by timeout after
> freezing this filesystem by guest-fsfreeze-freeze-list. Add an optional
> argument "timeout" to the command.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> qga/commands-posix.c | 21 ++++++++++++++++++---
> qga/commands-win32.c | 16 ++++++++++++++--
> qga/guest-agent-core.h | 3 ++-
> qga/main.c | 19 ++++++++++++++++++-
> qga/qapi-schema.json | 9 ++++++++-
> 5 files changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 26711a1a72..e8a79e0a41 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -707,13 +707,17 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error
> **errp)
> return GUEST_FSFREEZE_STATUS_THAWED;
> }
>
> -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> +int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
> + Error **errp)
> {
> - return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
> + return qmp_guest_fsfreeze_freeze_list(false, NULL, has_timeout,
> timeout,
> + errp);
> }
>
> int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> strList *mountpoints,
> + bool has_timeout,
> + int64_t timeout,
> Error **errp)
> {
> int ret;
> @@ -734,8 +738,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
> has_mountpoints,
> return -1;
> }
>
> + if (!has_timeout || timeout < 0) {
> + timeout = 0;
> + }
> /* cannot risk guest agent blocking itself on a write in this state */
> - ga_set_frozen(ga_state);
> + ga_set_frozen(ga_state, timeout);
>
> ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints, mountpoints,
> mounts, errp);
> @@ -780,6 +787,12 @@ static void guest_fsfreeze_cleanup(void)
> }
> }
> }
> +
> +gboolean ga_frozen_timeout_cb(gpointer data)
> +{
> + guest_fsfreeze_cleanup();
> + return G_SOURCE_REMOVE;
> +}
> #endif
>
> /* linux-specific implementations. avoid this if at all possible. */
> @@ -3119,6 +3132,8 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
>
> int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> strList *mountpoints,
> + bool has_timeout,
> + int64_t timeout,
> Error **errp)
> {
> error_setg(errp, QERR_UNSUPPORTED);
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 618d862c00..51fd6dcd58 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1221,13 +1221,16 @@ GuestFsfreezeStatus
> qmp_guest_fsfreeze_status(Error **errp)
> * Freeze local file systems using Volume Shadow-copy Service.
> * The frozen state is limited for up to 10 seconds by VSS.
> */
> -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> +int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
> + Error **errp)
> {
> return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
> }
>
> int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> strList *mountpoints,
> + bool has_timeout,
> + int64_t timeout,
> Error **errp)
> {
> int i;
> @@ -1240,8 +1243,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
> has_mountpoints,
>
> slog("guest-fsfreeze called");
>
> + if (!has_timeout || timeout < 0) {
> + timeout = 0;
> + }
> /* cannot risk guest agent blocking itself on a write in this state */
> - ga_set_frozen(ga_state);
> + ga_set_frozen(ga_state, timeout);
>
> qga_vss_fsfreeze(&i, true, mountpoints, &local_err);
> if (local_err) {
> @@ -1299,6 +1305,12 @@ static void guest_fsfreeze_cleanup(void)
> vss_deinit(true);
> }
>
> +gboolean ga_frozen_timeout_cb(gpointer data)
> +{
> + guest_fsfreeze_cleanup();
> + return G_SOURCE_REMOVE;
> +}
> +
> /*
> * Walk list of mounted file systems in the guest, and discard unused
> * areas.
> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> index b4e7c52c61..d8d1bb9505 100644
> --- a/qga/guest-agent-core.h
> +++ b/qga/guest-agent-core.h
> @@ -39,8 +39,9 @@ void ga_enable_logging(GAState *s);
> void G_GNUC_PRINTF(1, 2) slog(const gchar *fmt, ...);
> void ga_set_response_delimited(GAState *s);
> bool ga_is_frozen(GAState *s);
> -void ga_set_frozen(GAState *s);
> +void ga_set_frozen(GAState *s, int64_t timeout);
> void ga_unset_frozen(GAState *s);
> +gboolean ga_frozen_timeout_cb(gpointer data);
> const char *ga_fsfreeze_hook(GAState *s);
> int64_t ga_get_fd_handle(GAState *s, Error **errp);
> int ga_parse_whence(GuestFileWhence *whence, Error **errp);
> diff --git a/qga/main.c b/qga/main.c
> index 8668b9f3d3..6c7c7d68d8 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -94,6 +94,7 @@ struct GAState {
> const char *pid_filepath;
> } deferred_options;
> #ifdef CONFIG_FSFREEZE
> + guint frozen_timeout_id;
> const char *fsfreeze_hook;
> #endif
> gchar *pstate_filepath;
> @@ -478,7 +479,7 @@ bool ga_is_frozen(GAState *s)
> return s->frozen;
> }
>
> -void ga_set_frozen(GAState *s)
> +void ga_set_frozen(GAState *s, int64_t timeout)
> {
> if (ga_is_frozen(s)) {
> return;
> @@ -492,6 +493,15 @@ void ga_set_frozen(GAState *s)
> g_warning("unable to create %s, fsfreeze may not function
> properly",
> s->state_filepath_isfrozen);
> }
> +#ifdef CONFIG_FSFREEZE
> + if (timeout) {
> + s->frozen_timeout_id = g_timeout_add_seconds(timeout,
> + ga_frozen_timeout_cb,
> + NULL);
> + } else {
> + s->frozen_timeout_id = 0;
> + }
> +#endif
> }
>
> void ga_unset_frozen(GAState *s)
> @@ -500,6 +510,13 @@ void ga_unset_frozen(GAState *s)
> return;
> }
>
> +#ifdef CONFIG_FSFREEZE
> + /* remove timeout callback */
> + if (s->frozen_timeout_id) {
> + g_source_remove(s->frozen_timeout_id);
> + }
> +#endif
> +
> /* if we delayed creation/opening of pid/log files due to being
> * in a frozen state at start up, do it now
> */
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index e96d463639..29ad342f0a 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -440,6 +440,9 @@
> # command succeeded, you may call @guest-fsfreeze-thaw later to
> # unfreeze.
> #
> +# @timeout: after this period in seconds filesystems will be thawed
> +# (since 8.2)
> +#
> # Note: On Windows, the command is implemented with the help of a
> # Volume Shadow-copy Service DLL helper. The frozen state is
> # limited for up to 10 seconds by VSS.
> @@ -452,6 +455,7 @@
> # Since: 0.15.0
> ##
> { 'command': 'guest-fsfreeze-freeze',
> + 'data': { '*timeout': 'int' },
> 'returns': 'int' }
>
> ##
> @@ -464,13 +468,16 @@
> # If omitted, every mounted filesystem is frozen. Invalid mount
> # points are ignored.
> #
> +# @timeout: after this period in seconds filesystems will be thawed
> +# (since 8.2)
> +#
> # Returns: Number of file systems currently frozen. On error, all
> # filesystems will be thawed.
> #
> # Since: 2.2
> ##
> { 'command': 'guest-fsfreeze-freeze-list',
> - 'data': { '*mountpoints': ['str'] },
> + 'data': { '*mountpoints': ['str'], '*timeout': 'int' },
> 'returns': 'int' }
>
> ##
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 10515 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 5/6] qga: Add timeout for fsfreeze
2023-10-26 9:16 ` Konstantin Kostiuk
@ 2023-10-30 16:32 ` Alexander Ivanov
2023-10-30 17:37 ` Konstantin Kostiuk
0 siblings, 1 reply; 14+ messages in thread
From: Alexander Ivanov @ 2023-10-30 16:32 UTC (permalink / raw)
To: Konstantin Kostiuk; +Cc: qemu-devel, den, michael.roth, marcandre.lureau
On 10/26/23 11:16, Konstantin Kostiuk wrote:
>
> I think it is better to check that timeout <= 10 sec in the case of
> Windows.
> Anyway this is a VSS limitation and FS will be unfrozen earlier if
> timeout > 10 sec,
> this can cause some misunderstanding from a user.
Thank you, will pay attention.
>
> timeout option sounds good in the guest-fsfreeze-freeze command.
> In guest-fsfreeze-freeze-list, it looks strange to me. Command returns
> list but takes timeout option. Can you explain timeout usage in this
> command?
The second command doesn't return a list, it takes a list of mountpoints.
Both of the commands freeze local guest filesystems. The first one
freezes all the FS,
the second one freeze only FS from the given list.
>
> On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov
> <alexander.ivanov@virtuozzo.com> wrote:
>
> In some cases it would be useful to thaw a filesystem by timeout after
> freezing this filesystem by guest-fsfreeze-freeze-list. Add an
> optional
> argument "timeout" to the command.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> qga/commands-posix.c | 21 ++++++++++++++++++---
> qga/commands-win32.c | 16 ++++++++++++++--
> qga/guest-agent-core.h | 3 ++-
> qga/main.c | 19 ++++++++++++++++++-
> qga/qapi-schema.json | 9 ++++++++-
> 5 files changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 26711a1a72..e8a79e0a41 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -707,13 +707,17 @@ GuestFsfreezeStatus
> qmp_guest_fsfreeze_status(Error **errp)
> return GUEST_FSFREEZE_STATUS_THAWED;
> }
>
> -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> +int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
> + Error **errp)
> {
> - return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
> + return qmp_guest_fsfreeze_freeze_list(false, NULL,
> has_timeout, timeout,
> + errp);
> }
>
> int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> strList *mountpoints,
> + bool has_timeout,
> + int64_t timeout,
> Error **errp)
> {
> int ret;
> @@ -734,8 +738,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
> has_mountpoints,
> return -1;
> }
>
> + if (!has_timeout || timeout < 0) {
> + timeout = 0;
> + }
> /* cannot risk guest agent blocking itself on a write in this
> state */
> - ga_set_frozen(ga_state);
> + ga_set_frozen(ga_state, timeout);
>
> ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints,
> mountpoints,
> mounts, errp);
> @@ -780,6 +787,12 @@ static void guest_fsfreeze_cleanup(void)
> }
> }
> }
> +
> +gboolean ga_frozen_timeout_cb(gpointer data)
> +{
> + guest_fsfreeze_cleanup();
> + return G_SOURCE_REMOVE;
> +}
> #endif
>
> /* linux-specific implementations. avoid this if at all possible. */
> @@ -3119,6 +3132,8 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
>
> int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> strList *mountpoints,
> + bool has_timeout,
> + int64_t timeout,
> Error **errp)
> {
> error_setg(errp, QERR_UNSUPPORTED);
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 618d862c00..51fd6dcd58 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1221,13 +1221,16 @@ GuestFsfreezeStatus
> qmp_guest_fsfreeze_status(Error **errp)
> * Freeze local file systems using Volume Shadow-copy Service.
> * The frozen state is limited for up to 10 seconds by VSS.
> */
> -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> +int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
> + Error **errp)
> {
> return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
> }
>
> int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> strList *mountpoints,
> + bool has_timeout,
> + int64_t timeout,
> Error **errp)
> {
> int i;
> @@ -1240,8 +1243,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
> has_mountpoints,
>
> slog("guest-fsfreeze called");
>
> + if (!has_timeout || timeout < 0) {
> + timeout = 0;
> + }
> /* cannot risk guest agent blocking itself on a write in this
> state */
> - ga_set_frozen(ga_state);
> + ga_set_frozen(ga_state, timeout);
>
> qga_vss_fsfreeze(&i, true, mountpoints, &local_err);
> if (local_err) {
> @@ -1299,6 +1305,12 @@ static void guest_fsfreeze_cleanup(void)
> vss_deinit(true);
> }
>
> +gboolean ga_frozen_timeout_cb(gpointer data)
> +{
> + guest_fsfreeze_cleanup();
> + return G_SOURCE_REMOVE;
> +}
> +
> /*
> * Walk list of mounted file systems in the guest, and discard unused
> * areas.
> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> index b4e7c52c61..d8d1bb9505 100644
> --- a/qga/guest-agent-core.h
> +++ b/qga/guest-agent-core.h
> @@ -39,8 +39,9 @@ void ga_enable_logging(GAState *s);
> void G_GNUC_PRINTF(1, 2) slog(const gchar *fmt, ...);
> void ga_set_response_delimited(GAState *s);
> bool ga_is_frozen(GAState *s);
> -void ga_set_frozen(GAState *s);
> +void ga_set_frozen(GAState *s, int64_t timeout);
> void ga_unset_frozen(GAState *s);
> +gboolean ga_frozen_timeout_cb(gpointer data);
> const char *ga_fsfreeze_hook(GAState *s);
> int64_t ga_get_fd_handle(GAState *s, Error **errp);
> int ga_parse_whence(GuestFileWhence *whence, Error **errp);
> diff --git a/qga/main.c b/qga/main.c
> index 8668b9f3d3..6c7c7d68d8 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -94,6 +94,7 @@ struct GAState {
> const char *pid_filepath;
> } deferred_options;
> #ifdef CONFIG_FSFREEZE
> + guint frozen_timeout_id;
> const char *fsfreeze_hook;
> #endif
> gchar *pstate_filepath;
> @@ -478,7 +479,7 @@ bool ga_is_frozen(GAState *s)
> return s->frozen;
> }
>
> -void ga_set_frozen(GAState *s)
> +void ga_set_frozen(GAState *s, int64_t timeout)
> {
> if (ga_is_frozen(s)) {
> return;
> @@ -492,6 +493,15 @@ void ga_set_frozen(GAState *s)
> g_warning("unable to create %s, fsfreeze may not function
> properly",
> s->state_filepath_isfrozen);
> }
> +#ifdef CONFIG_FSFREEZE
> + if (timeout) {
> + s->frozen_timeout_id = g_timeout_add_seconds(timeout,
> + ga_frozen_timeout_cb,
> + NULL);
> + } else {
> + s->frozen_timeout_id = 0;
> + }
> +#endif
> }
>
> void ga_unset_frozen(GAState *s)
> @@ -500,6 +510,13 @@ void ga_unset_frozen(GAState *s)
> return;
> }
>
> +#ifdef CONFIG_FSFREEZE
> + /* remove timeout callback */
> + if (s->frozen_timeout_id) {
> + g_source_remove(s->frozen_timeout_id);
> + }
> +#endif
> +
> /* if we delayed creation/opening of pid/log files due to being
> * in a frozen state at start up, do it now
> */
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index e96d463639..29ad342f0a 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -440,6 +440,9 @@
> # command succeeded, you may call @guest-fsfreeze-thaw later to
> # unfreeze.
> #
> +# @timeout: after this period in seconds filesystems will be thawed
> +# (since 8.2)
> +#
> # Note: On Windows, the command is implemented with the help of a
> # Volume Shadow-copy Service DLL helper. The frozen state is
> # limited for up to 10 seconds by VSS.
> @@ -452,6 +455,7 @@
> # Since: 0.15.0
> ##
> { 'command': 'guest-fsfreeze-freeze',
> + 'data': { '*timeout': 'int' },
> 'returns': 'int' }
>
> ##
> @@ -464,13 +468,16 @@
> # If omitted, every mounted filesystem is frozen. Invalid mount
> # points are ignored.
> #
> +# @timeout: after this period in seconds filesystems will be thawed
> +# (since 8.2)
> +#
> # Returns: Number of file systems currently frozen. On error, all
> # filesystems will be thawed.
> #
> # Since: 2.2
> ##
> { 'command': 'guest-fsfreeze-freeze-list',
> - 'data': { '*mountpoints': ['str'] },
> + 'data': { '*mountpoints': ['str'], '*timeout': 'int' },
> 'returns': 'int' }
>
> ##
> --
> 2.34.1
>
--
Best regards,
Alexander Ivanov
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 5/6] qga: Add timeout for fsfreeze
2023-10-30 16:32 ` Alexander Ivanov
@ 2023-10-30 17:37 ` Konstantin Kostiuk
0 siblings, 0 replies; 14+ messages in thread
From: Konstantin Kostiuk @ 2023-10-30 17:37 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: qemu-devel, den, michael.roth, marcandre.lureau
[-- Attachment #1: Type: text/plain, Size: 10672 bytes --]
On Mon, Oct 30, 2023 at 6:32 PM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:
>
>
> On 10/26/23 11:16, Konstantin Kostiuk wrote:
> >
> > I think it is better to check that timeout <= 10 sec in the case of
> > Windows.
> > Anyway this is a VSS limitation and FS will be unfrozen earlier if
> > timeout > 10 sec,
> > this can cause some misunderstanding from a user.
> Thank you, will pay attention.
> >
> > timeout option sounds good in the guest-fsfreeze-freeze command.
> > In guest-fsfreeze-freeze-list, it looks strange to me. Command returns
> > list but takes timeout option. Can you explain timeout usage in this
> > command?
> The second command doesn't return a list, it takes a list of mountpoints.
> Both of the commands freeze local guest filesystems. The first one
> freezes all the FS,
> the second one freeze only FS from the given list.
>
Oh, sorry, my mistake.
Just comment about VSS limitation.
Best Regards,
Konstantin Kostiuk.
> >
> > On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov
> > <alexander.ivanov@virtuozzo.com> wrote:
> >
> > In some cases it would be useful to thaw a filesystem by timeout
> after
> > freezing this filesystem by guest-fsfreeze-freeze-list. Add an
> > optional
> > argument "timeout" to the command.
> >
> > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> > ---
> > qga/commands-posix.c | 21 ++++++++++++++++++---
> > qga/commands-win32.c | 16 ++++++++++++++--
> > qga/guest-agent-core.h | 3 ++-
> > qga/main.c | 19 ++++++++++++++++++-
> > qga/qapi-schema.json | 9 ++++++++-
> > 5 files changed, 60 insertions(+), 8 deletions(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index 26711a1a72..e8a79e0a41 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -707,13 +707,17 @@ GuestFsfreezeStatus
> > qmp_guest_fsfreeze_status(Error **errp)
> > return GUEST_FSFREEZE_STATUS_THAWED;
> > }
> >
> > -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> > +int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
> > + Error **errp)
> > {
> > - return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
> > + return qmp_guest_fsfreeze_freeze_list(false, NULL,
> > has_timeout, timeout,
> > + errp);
> > }
> >
> > int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> > strList *mountpoints,
> > + bool has_timeout,
> > + int64_t timeout,
> > Error **errp)
> > {
> > int ret;
> > @@ -734,8 +738,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
> > has_mountpoints,
> > return -1;
> > }
> >
> > + if (!has_timeout || timeout < 0) {
> > + timeout = 0;
> > + }
> > /* cannot risk guest agent blocking itself on a write in this
> > state */
> > - ga_set_frozen(ga_state);
> > + ga_set_frozen(ga_state, timeout);
> >
> > ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints,
> > mountpoints,
> > mounts, errp);
> > @@ -780,6 +787,12 @@ static void guest_fsfreeze_cleanup(void)
> > }
> > }
> > }
> > +
> > +gboolean ga_frozen_timeout_cb(gpointer data)
> > +{
> > + guest_fsfreeze_cleanup();
> > + return G_SOURCE_REMOVE;
> > +}
> > #endif
> >
> > /* linux-specific implementations. avoid this if at all possible. */
> > @@ -3119,6 +3132,8 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> >
> > int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> > strList *mountpoints,
> > + bool has_timeout,
> > + int64_t timeout,
> > Error **errp)
> > {
> > error_setg(errp, QERR_UNSUPPORTED);
> > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > index 618d862c00..51fd6dcd58 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -1221,13 +1221,16 @@ GuestFsfreezeStatus
> > qmp_guest_fsfreeze_status(Error **errp)
> > * Freeze local file systems using Volume Shadow-copy Service.
> > * The frozen state is limited for up to 10 seconds by VSS.
> > */
> > -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> > +int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
> > + Error **errp)
> > {
> > return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
> > }
> >
> > int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
> > strList *mountpoints,
> > + bool has_timeout,
> > + int64_t timeout,
> > Error **errp)
> > {
> > int i;
> > @@ -1240,8 +1243,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
> > has_mountpoints,
> >
> > slog("guest-fsfreeze called");
> >
> > + if (!has_timeout || timeout < 0) {
> > + timeout = 0;
> > + }
> > /* cannot risk guest agent blocking itself on a write in this
> > state */
> > - ga_set_frozen(ga_state);
> > + ga_set_frozen(ga_state, timeout);
> >
> > qga_vss_fsfreeze(&i, true, mountpoints, &local_err);
> > if (local_err) {
> > @@ -1299,6 +1305,12 @@ static void guest_fsfreeze_cleanup(void)
> > vss_deinit(true);
> > }
> >
> > +gboolean ga_frozen_timeout_cb(gpointer data)
> > +{
> > + guest_fsfreeze_cleanup();
> > + return G_SOURCE_REMOVE;
> > +}
> > +
> > /*
> > * Walk list of mounted file systems in the guest, and discard
> unused
> > * areas.
> > diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> > index b4e7c52c61..d8d1bb9505 100644
> > --- a/qga/guest-agent-core.h
> > +++ b/qga/guest-agent-core.h
> > @@ -39,8 +39,9 @@ void ga_enable_logging(GAState *s);
> > void G_GNUC_PRINTF(1, 2) slog(const gchar *fmt, ...);
> > void ga_set_response_delimited(GAState *s);
> > bool ga_is_frozen(GAState *s);
> > -void ga_set_frozen(GAState *s);
> > +void ga_set_frozen(GAState *s, int64_t timeout);
> > void ga_unset_frozen(GAState *s);
> > +gboolean ga_frozen_timeout_cb(gpointer data);
> > const char *ga_fsfreeze_hook(GAState *s);
> > int64_t ga_get_fd_handle(GAState *s, Error **errp);
> > int ga_parse_whence(GuestFileWhence *whence, Error **errp);
> > diff --git a/qga/main.c b/qga/main.c
> > index 8668b9f3d3..6c7c7d68d8 100644
> > --- a/qga/main.c
> > +++ b/qga/main.c
> > @@ -94,6 +94,7 @@ struct GAState {
> > const char *pid_filepath;
> > } deferred_options;
> > #ifdef CONFIG_FSFREEZE
> > + guint frozen_timeout_id;
> > const char *fsfreeze_hook;
> > #endif
> > gchar *pstate_filepath;
> > @@ -478,7 +479,7 @@ bool ga_is_frozen(GAState *s)
> > return s->frozen;
> > }
> >
> > -void ga_set_frozen(GAState *s)
> > +void ga_set_frozen(GAState *s, int64_t timeout)
> > {
> > if (ga_is_frozen(s)) {
> > return;
> > @@ -492,6 +493,15 @@ void ga_set_frozen(GAState *s)
> > g_warning("unable to create %s, fsfreeze may not function
> > properly",
> > s->state_filepath_isfrozen);
> > }
> > +#ifdef CONFIG_FSFREEZE
> > + if (timeout) {
> > + s->frozen_timeout_id = g_timeout_add_seconds(timeout,
> > + ga_frozen_timeout_cb,
> > + NULL);
> > + } else {
> > + s->frozen_timeout_id = 0;
> > + }
> > +#endif
> > }
> >
> > void ga_unset_frozen(GAState *s)
> > @@ -500,6 +510,13 @@ void ga_unset_frozen(GAState *s)
> > return;
> > }
> >
> > +#ifdef CONFIG_FSFREEZE
> > + /* remove timeout callback */
> > + if (s->frozen_timeout_id) {
> > + g_source_remove(s->frozen_timeout_id);
> > + }
> > +#endif
> > +
> > /* if we delayed creation/opening of pid/log files due to being
> > * in a frozen state at start up, do it now
> > */
> > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > index e96d463639..29ad342f0a 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -440,6 +440,9 @@
> > # command succeeded, you may call @guest-fsfreeze-thaw later to
> > # unfreeze.
> > #
> > +# @timeout: after this period in seconds filesystems will be thawed
> > +# (since 8.2)
> > +#
> > # Note: On Windows, the command is implemented with the help of a
> > # Volume Shadow-copy Service DLL helper. The frozen state is
> > # limited for up to 10 seconds by VSS.
> > @@ -452,6 +455,7 @@
> > # Since: 0.15.0
> > ##
> > { 'command': 'guest-fsfreeze-freeze',
> > + 'data': { '*timeout': 'int' },
> > 'returns': 'int' }
> >
> > ##
> > @@ -464,13 +468,16 @@
> > # If omitted, every mounted filesystem is frozen. Invalid mount
> > # points are ignored.
> > #
> > +# @timeout: after this period in seconds filesystems will be thawed
> > +# (since 8.2)
> > +#
> > # Returns: Number of file systems currently frozen. On error, all
> > # filesystems will be thawed.
> > #
> > # Since: 2.2
> > ##
> > { 'command': 'guest-fsfreeze-freeze-list',
> > - 'data': { '*mountpoints': ['str'] },
> > + 'data': { '*mountpoints': ['str'], '*timeout': 'int' },
> > 'returns': 'int' }
> >
> > ##
> > --
> > 2.34.1
> >
>
> --
> Best regards,
> Alexander Ivanov
>
>
[-- Attachment #2: Type: text/html, Size: 14260 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 6/6] qga: Cancel async snapshot before abort
2023-10-25 14:00 [PATCH 0/6] qga: Assorted patches, let us discuss Alexander Ivanov
` (4 preceding siblings ...)
2023-10-25 14:00 ` [PATCH 5/6] qga: Add timeout for fsfreeze Alexander Ivanov
@ 2023-10-25 14:00 ` Alexander Ivanov
2023-10-26 8:47 ` Konstantin Kostiuk
2023-10-26 9:17 ` Konstantin Kostiuk
5 siblings, 2 replies; 14+ messages in thread
From: Alexander Ivanov @ 2023-10-25 14:00 UTC (permalink / raw)
To: qemu-devel; +Cc: den, michael.roth, kkostiuk, marcandre.lureau
VSS requestor calls abort after the timeout of the backup operation
expires. In the result later the process hangs on some internal VSS
lock. Cancel async snapshot before abort.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
qga/vss-win32/requester.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 9884c65e70..20680a42a1 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -533,6 +533,7 @@ void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
}
if (wait_status != WAIT_OBJECT_0) {
+ vss_ctx.pAsyncSnapshot->Cancel();
err_set(errset, E_FAIL,
"couldn't receive Frozen event from VSS provider");
goto out;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 6/6] qga: Cancel async snapshot before abort
2023-10-25 14:00 ` [PATCH 6/6] qga: Cancel async snapshot before abort Alexander Ivanov
@ 2023-10-26 8:47 ` Konstantin Kostiuk
2023-10-26 9:17 ` Konstantin Kostiuk
1 sibling, 0 replies; 14+ messages in thread
From: Konstantin Kostiuk @ 2023-10-26 8:47 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: qemu-devel, den, michael.roth, marcandre.lureau
[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:
> VSS requestor calls abort after the timeout of the backup operation
> expires. In the result later the process hangs on some internal VSS
> lock. Cancel async snapshot before abort.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> qga/vss-win32/requester.cpp | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> index 9884c65e70..20680a42a1 100644
> --- a/qga/vss-win32/requester.cpp
> +++ b/qga/vss-win32/requester.cpp
> @@ -533,6 +533,7 @@ void requester_freeze(int *num_vols, void
> *mountpoints, ErrorSet *errset)
> }
>
> if (wait_status != WAIT_OBJECT_0) {
> + vss_ctx.pAsyncSnapshot->Cancel();
> err_set(errset, E_FAIL,
> "couldn't receive Frozen event from VSS provider");
> goto out;
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 1552 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 6/6] qga: Cancel async snapshot before abort
2023-10-25 14:00 ` [PATCH 6/6] qga: Cancel async snapshot before abort Alexander Ivanov
2023-10-26 8:47 ` Konstantin Kostiuk
@ 2023-10-26 9:17 ` Konstantin Kostiuk
1 sibling, 0 replies; 14+ messages in thread
From: Konstantin Kostiuk @ 2023-10-26 9:17 UTC (permalink / raw)
To: Alexander Ivanov, Dehan Meng
Cc: qemu-devel, den, michael.roth, marcandre.lureau
[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]
On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:
> VSS requestor calls abort after the timeout of the backup operation
> expires. In the result later the process hangs on some internal VSS
> lock. Cancel async snapshot before abort.
>
Can you share some information on how to reproduce this issue?
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> qga/vss-win32/requester.cpp | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> index 9884c65e70..20680a42a1 100644
> --- a/qga/vss-win32/requester.cpp
> +++ b/qga/vss-win32/requester.cpp
> @@ -533,6 +533,7 @@ void requester_freeze(int *num_vols, void
> *mountpoints, ErrorSet *errset)
> }
>
> if (wait_status != WAIT_OBJECT_0) {
> + vss_ctx.pAsyncSnapshot->Cancel();
> err_set(errset, E_FAIL,
> "couldn't receive Frozen event from VSS provider");
> goto out;
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 1701 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread