[Qemu-devel] [PATCH v2 0/2] qga: guest-set-user-password - added ability to create new user

[Qemu-devel] [PATCH v2 0/2] qga: guest-set-user-password - added ability to create new user

[Denis V. Lunev]

These patches add optional 'create' flag to guest-set-user-password command.
When it is specified, a new user will be created if it does not
exist yet.

Since v1:
- fixed english language mistakes in comments
- json description now mentions 'create' as default to false
- capture stdout/stderr from useradd/chpasswd and send iti back with the
  error message to caller
- split to two patches

Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>

Yuriy Pudgorodskiy (2):
  create ga_run_program() helper for guest-set-user-password
  guest-set-user-password - added ability to create new user

 qga/commands-posix.c | 215 +++++++++++++++++++++++++++++++++++++--------------
 qga/commands-win32.c |  25 +++++-
 qga/qapi-schema.json |   5 +-
 3 files changed, 186 insertions(+), 59 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH 1/2] create ga_run_program() helper for guest-set-user-password

[Denis V. Lunev]

From: Yuriy Pudgorodskiy <yur@virtuozzo.com>

This helper properly starts chpasswd and collects stdout/stderr of this
program to report it as error to the caller.

The code will be reused later to run useradd in addition to chpasswd.

This code is made specifically for Linux and is inside ifdef Linux braces.

Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 201 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 141 insertions(+), 60 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 8fe708f..53e8d3b 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -75,6 +75,28 @@ static void ga_wait_child(pid_t pid, int *status, Error **errp)
     g_assert(rpid == pid);
 }
 
+static void ga_pipe_read_str(int fd[2], char **str, size_t *len)
+{
+    ssize_t n;
+    char buf[1024];
+    close(fd[1]);
+    fd[1] = -1;
+    while ((n = read(fd[0], buf, sizeof(buf))) != 0) {
+        if (n < 0) {
+            if (errno == EINTR) {
+                continue;
+            } else {
+                break;
+            }
+        }
+        *str = g_realloc(*str, *len + n);
+        memcpy(*str + *len, buf, n);
+        *len += n;
+    }
+    close(fd[0]);
+    fd[0] = -1;
+}
+
 void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
 {
     const char *shutdown_flag;
@@ -1952,20 +1974,128 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
     return processed;
 }
 
+/* Helper to run command with input/output redirection,
+ * sending string to stdin and taking error message from
+ * stdout/err
+ */
+static void ga_run_program(const char *argv[],
+                           const char *in_str,
+                           const char *action, Error **errp)
+{
+    pid_t pid;
+    int status;
+    int infd[2] = { -1, -1 };
+    int outfd[2] = { -1, -1 };
+    char *str = NULL;
+    size_t len = 0;
+
+    if (in_str) {
+        if (pipe(infd) < 0) {
+            error_setg(errp, "cannot create pipe FDs");
+            goto out;
+        }
+    }
+
+    if (pipe(outfd) < 0) {
+        error_setg(errp, "cannot create pipe FDs");
+        goto out;
+    }
+
+    pid = fork();
+    if (pid == 0) {
+        /* child */
+        setsid();
+        if (in_str) {
+            /* redirect stdin to infd */
+            close(infd[1]);
+            dup2(infd[0], 0);
+            close(infd[0]);
+        } else {
+            reopen_fd_to_null(0);
+        }
+
+        /* redirect stdout/stderr to outfd */
+        close(outfd[0]);
+        dup2(outfd[1], 1);
+        dup2(outfd[1], 2);
+        close(outfd[1]);
+
+        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;
+    }
+
+    if (in_str) {
+        close(infd[0]);
+        infd[0] = -1;
+        if (qemu_write_full(infd[1],
+                    in_str, strlen(in_str)) != strlen(in_str)) {
+            error_setg_errno(errp, errno,
+                    "%s: cannot write to stdin pipe", action);
+            goto out;
+        }
+        close(infd[1]);
+        infd[1] = -1;
+    }
+
+
+    ga_pipe_read_str(outfd, &str, &len);
+
+    ga_wait_child(pid, &status, errp);
+    if (*errp) {
+        goto out;
+    }
+
+    if (!WIFEXITED(status)) {
+        if (len) {
+            error_setg(errp, "child process has terminated abnormally: "
+                    "%s", str);
+        } else {
+            error_setg(errp, "child process has terminated abnormally");
+        }
+        goto out;
+    }
+
+    if (WEXITSTATUS(status)) {
+        if (len) {
+            error_setg(errp, "child process has failed to %s: %s",
+                    action, str);
+        } else {
+            error_setg(errp, "child process has failed to %s: exit status %d",
+                    action, WEXITSTATUS(status));
+        }
+        goto out;
+    }
+
+out:
+    g_free(str);
+
+    if (infd[0] != -1) {
+        close(infd[0]);
+    }
+    if (infd[1] != -1) {
+        close(infd[1]);
+    }
+    if (outfd[0] != -1) {
+        close(outfd[0]);
+    }
+    if (outfd[1] != -1) {
+        close(outfd[1]);
+    }
+}
+
 void qmp_guest_set_user_password(const char *username,
                                  const char *password,
                                  bool crypted,
                                  Error **errp)
 {
-    Error *local_err = NULL;
     char *passwd_path = NULL;
-    pid_t pid;
-    int status;
-    int datafd[2] = { -1, -1 };
     char *rawpasswddata = NULL;
     size_t rawpasswdlen;
     char *chpasswddata = NULL;
-    size_t chpasswdlen;
+    const char *chpasswd_argv[] = { NULL /*path*/, "chpasswd", "-e", NULL };
 
     rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, errp);
     if (!rawpasswddata) {
@@ -1986,75 +2116,26 @@ void qmp_guest_set_user_password(const char *username,
     }
 
     chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata);
-    chpasswdlen = strlen(chpasswddata);
 
     passwd_path = g_find_program_in_path("chpasswd");
-
     if (!passwd_path) {
         error_setg(errp, "cannot find 'passwd' program in PATH");
         goto out;
     }
 
-    if (pipe(datafd) < 0) {
-        error_setg(errp, "cannot create pipe FDs");
-        goto out;
-    }
+    chpasswd_argv[0] = passwd_path;
 
-    pid = fork();
-    if (pid == 0) {
-        close(datafd[1]);
-        /* child */
-        setsid();
-        dup2(datafd[0], 0);
-        reopen_fd_to_null(1);
-        reopen_fd_to_null(2);
-
-        if (crypted) {
-            execle(passwd_path, "chpasswd", "-e", NULL, environ);
-        } else {
-            execle(passwd_path, "chpasswd", NULL, 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;
-
-    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;
+    /* set password for existed user */
+    if (!crypted) {
+        /* wipe -e option */
+        chpasswd_argv[2] = NULL;
     }
+    ga_run_program(chpasswd_argv, chpasswddata, "set user password", 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]);
-    }
 }
 
 static void ga_read_sysfs_file(int dirfd, const char *pathname, char *buf,
-- 
2.1.4

Re: [Qemu-devel] [PATCH 1/2] create ga_run_program() helper for guest-set-user-password

[Denis V. Lunev]

On 01/06/2016 03:01 PM, Denis V. Lunev wrote:
> From: Yuriy Pudgorodskiy <yur@virtuozzo.com>
>
> This helper properly starts chpasswd and collects stdout/stderr of this
> program to report it as error to the caller.
>
> The code will be reused later to run useradd in addition to chpasswd.
>
> This code is made specifically for Linux and is inside ifdef Linux braces.
>
> Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Eric Blake <eblake@redhat.com>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>   qga/commands-posix.c | 201 ++++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 141 insertions(+), 60 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 8fe708f..53e8d3b 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -75,6 +75,28 @@ static void ga_wait_child(pid_t pid, int *status, Error **errp)
>       g_assert(rpid == pid);
>   }
>   
> +static void ga_pipe_read_str(int fd[2], char **str, size_t *len)
> +{
> +    ssize_t n;
> +    char buf[1024];
> +    close(fd[1]);
> +    fd[1] = -1;
> +    while ((n = read(fd[0], buf, sizeof(buf))) != 0) {
> +        if (n < 0) {
> +            if (errno == EINTR) {
> +                continue;
> +            } else {
> +                break;
> +            }
> +        }
> +        *str = g_realloc(*str, *len + n);
> +        memcpy(*str + *len, buf, n);
> +        *len += n;
> +    }
> +    close(fd[0]);
> +    fd[0] = -1;
> +}
> +
>   void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
>   {
>       const char *shutdown_flag;
> @@ -1952,20 +1974,128 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>       return processed;
>   }
>   
> +/* Helper to run command with input/output redirection,
> + * sending string to stdin and taking error message from
> + * stdout/err
> + */
> +static void ga_run_program(const char *argv[],
> +                           const char *in_str,
> +                           const char *action, Error **errp)
> +{
> +    pid_t pid;
> +    int status;
> +    int infd[2] = { -1, -1 };
> +    int outfd[2] = { -1, -1 };
> +    char *str = NULL;
> +    size_t len = 0;
> +
> +    if (in_str) {
> +        if (pipe(infd) < 0) {
> +            error_setg(errp, "cannot create pipe FDs");
> +            goto out;
> +        }
> +    }
> +
> +    if (pipe(outfd) < 0) {
> +        error_setg(errp, "cannot create pipe FDs");
> +        goto out;
> +    }
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        /* child */
> +        setsid();
> +        if (in_str) {
> +            /* redirect stdin to infd */
> +            close(infd[1]);
> +            dup2(infd[0], 0);
> +            close(infd[0]);
> +        } else {
> +            reopen_fd_to_null(0);
> +        }
> +
> +        /* redirect stdout/stderr to outfd */
> +        close(outfd[0]);
> +        dup2(outfd[1], 1);
> +        dup2(outfd[1], 2);
> +        close(outfd[1]);
> +
> +        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;
> +    }
> +
> +    if (in_str) {
> +        close(infd[0]);
> +        infd[0] = -1;
> +        if (qemu_write_full(infd[1],
> +                    in_str, strlen(in_str)) != strlen(in_str)) {
> +            error_setg_errno(errp, errno,
> +                    "%s: cannot write to stdin pipe", action);
> +            goto out;
> +        }
> +        close(infd[1]);
> +        infd[1] = -1;
> +    }
> +
> +
> +    ga_pipe_read_str(outfd, &str, &len);
> +
> +    ga_wait_child(pid, &status, errp);
> +    if (*errp) {
> +        goto out;
> +    }
> +
> +    if (!WIFEXITED(status)) {
> +        if (len) {
> +            error_setg(errp, "child process has terminated abnormally: "
> +                    "%s", str);
> +        } else {
> +            error_setg(errp, "child process has terminated abnormally");
> +        }
> +        goto out;
> +    }
> +
> +    if (WEXITSTATUS(status)) {
> +        if (len) {
> +            error_setg(errp, "child process has failed to %s: %s",
> +                    action, str);
> +        } else {
> +            error_setg(errp, "child process has failed to %s: exit status %d",
> +                    action, WEXITSTATUS(status));
> +        }
> +        goto out;
> +    }
> +
> +out:
> +    g_free(str);
> +
> +    if (infd[0] != -1) {
> +        close(infd[0]);
> +    }
> +    if (infd[1] != -1) {
> +        close(infd[1]);
> +    }
> +    if (outfd[0] != -1) {
> +        close(outfd[0]);
> +    }
> +    if (outfd[1] != -1) {
> +        close(outfd[1]);
> +    }
> +}
> +
>   void qmp_guest_set_user_password(const char *username,
>                                    const char *password,
>                                    bool crypted,
>                                    Error **errp)
>   {
> -    Error *local_err = NULL;
>       char *passwd_path = NULL;
> -    pid_t pid;
> -    int status;
> -    int datafd[2] = { -1, -1 };
>       char *rawpasswddata = NULL;
>       size_t rawpasswdlen;
>       char *chpasswddata = NULL;
> -    size_t chpasswdlen;
> +    const char *chpasswd_argv[] = { NULL /*path*/, "chpasswd", "-e", NULL };
>   
>       rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, errp);
>       if (!rawpasswddata) {
> @@ -1986,75 +2116,26 @@ void qmp_guest_set_user_password(const char *username,
>       }
>   
>       chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata);
> -    chpasswdlen = strlen(chpasswddata);
>   
>       passwd_path = g_find_program_in_path("chpasswd");
> -
>       if (!passwd_path) {
>           error_setg(errp, "cannot find 'passwd' program in PATH");
>           goto out;
>       }
>   
> -    if (pipe(datafd) < 0) {
> -        error_setg(errp, "cannot create pipe FDs");
> -        goto out;
> -    }
> +    chpasswd_argv[0] = passwd_path;
>   
> -    pid = fork();
> -    if (pid == 0) {
> -        close(datafd[1]);
> -        /* child */
> -        setsid();
> -        dup2(datafd[0], 0);
> -        reopen_fd_to_null(1);
> -        reopen_fd_to_null(2);
> -
> -        if (crypted) {
> -            execle(passwd_path, "chpasswd", "-e", NULL, environ);
> -        } else {
> -            execle(passwd_path, "chpasswd", NULL, 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;
> -
> -    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;
> +    /* set password for existed user */
> +    if (!crypted) {
> +        /* wipe -e option */
> +        chpasswd_argv[2] = NULL;
>       }
> +    ga_run_program(chpasswd_argv, chpasswddata, "set user password", 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]);
> -    }
>   }
>   
>   static void ga_read_sysfs_file(int dirfd, const char *pathname, char *buf,
hmmm....

Yur,

it seems that you have re-invented the wheel with

gboolean 
<https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gboolean>
g_spawn_sync (/|const gchar 
<https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
*working_directory|/,
               /|gchar 
<https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
**argv|/,
               /|gchar 
<https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
**envp|/,
               /|GSpawnFlags 
<https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#GSpawnFlags> 
flags|/,
               /|GSpawnChildSetupFunc 
<https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#GSpawnChildSetupFunc> 
child_setup|/,
               /|gpointer 
<https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gpointer> 
user_data|/,
               /|gchar 
<https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
**standard_output|/,
               /|gchar 
<https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
**standard_error|/,
               /|gint 
<https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gint> 
*exit_status|/,
               /|GError 
<https://developer.gnome.org/glib/stable/glib-Error-Reporting.html#GError> 
**error|/);


Executes a child synchronously (waits for the child to exit before 
returning). All output from the child is stored 
in/|standard_output|/and/|standard_error|/, if those parameters are 
non-|NULL| 
<https://developer.gnome.org/glib/stable/glib-Standard-Macros.html#NULL:CAPS>. 
Note that you must set the|G_SPAWN_STDOUT_TO_DEV_NULL| 
<https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#G-SPAWN-STDOUT-TO-DEV-NULL:CAPS>and|G_SPAWN_STDERR_TO_DEV_NULL| 
<https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#G-SPAWN-STDERR-TO-DEV-NULL:CAPS>flags 
when passing|NULL| 
<https://developer.gnome.org/glib/stable/glib-Standard-Macros.html#NULL:CAPS>for/|standard_output|/and/|standard_error|/.

If/|exit_status|/is non-|NULL| 
<https://developer.gnome.org/glib/stable/glib-Standard-Macros.html#NULL:CAPS>, 
the platform-specific exit status of the child is stored there; see the 
documentation of|g_spawn_check_exit_status()| 
<https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#g-spawn-check-exit-status>for 
how to use and interpret this. Note that it is invalid to 
pass|G_SPAWN_DO_NOT_REAP_CHILD| 
<https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#G-SPAWN-DO-NOT-REAP-CHILD:CAPS>in/|flags|/.

If an error occurs, no data is returned 
in/|standard_output|/,/|standard_error|/, or/|exit_status|/.

Re: [Qemu-devel] [PATCH 1/2] create ga_run_program() helper for guest-set-user-password

[Yuriy Pudgorodskiy]

On 1/6/2016 4:50 PM, Denis V. Lunev wrote:
> hmmm....
>
> Yur,
>
> it seems that you have re-invented the wheel with
>
> gboolean 
> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gboolean>
> g_spawn_sync (/|const gchar 
> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
> *working_directory|/,
>               /|gchar 
> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
> **argv|/,
>               /|gchar 
> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
> **envp|/,
>               /|GSpawnFlags 
> <https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#GSpawnFlags> 
> flags|/,
>               /|GSpawnChildSetupFunc 
> <https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#GSpawnChildSetupFunc> 
> child_setup|/,
>               /|gpointer 
> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gpointer> 
> user_data|/,
>               /|gchar 
> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
> **standard_output|/,
>               /|gchar 
> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
> **standard_error|/,
>               /|gint 
> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gint> 
> *exit_status|/,
>               /|GError 
> <https://developer.gnome.org/glib/stable/glib-Error-Reporting.html#GError> 
> **error|/);
>
>

Not exactly.

g_spawn_sync() wrapper cannot be used directly instead of my wrapper,
because it does not support redirected stdin which is required to send 
data to chpasswd.
g_spawn_async_with_pipes() can be used (with stdin), but it will not 
make code simpler.

Most of command-posix.c calls fork/execv directly, and so does the 
wrapper I wrote.

Re: [Qemu-devel] [PATCH 1/2] create ga_run_program() helper for guest-set-user-password

[Denis V. Lunev]

On 01/11/2016 03:08 PM, Yuriy Pudgorodskiy wrote:
> On 1/6/2016 4:50 PM, Denis V. Lunev wrote:
>> hmmm....
>>
>> Yur,
>>
>> it seems that you have re-invented the wheel with
>>
>> gboolean 
>> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gboolean>
>> g_spawn_sync (/|const gchar 
>> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
>> *working_directory|/,
>>               /|gchar 
>> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
>> **argv|/,
>>               /|gchar 
>> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
>> **envp|/,
>>               /|GSpawnFlags 
>> <https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#GSpawnFlags> 
>> flags|/,
>>               /|GSpawnChildSetupFunc 
>> <https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#GSpawnChildSetupFunc> 
>> child_setup|/,
>>               /|gpointer 
>> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gpointer> 
>> user_data|/,
>>               /|gchar 
>> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
>> **standard_output|/,
>>               /|gchar 
>> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar> 
>> **standard_error|/,
>>               /|gint 
>> <https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gint> 
>> *exit_status|/,
>>               /|GError 
>> <https://developer.gnome.org/glib/stable/glib-Error-Reporting.html#GError> 
>> **error|/);
>>
>>
>
> Not exactly.
>
> g_spawn_sync() wrapper cannot be used directly instead of my wrapper,
> because it does not support redirected stdin which is required to send 
> data to chpasswd.
> g_spawn_async_with_pipes() can be used (with stdin), but it will not 
> make code simpler.
>
> Most of command-posix.c calls fork/execv directly, and so does the 
> wrapper I wrote.
>
>
this seems that you are right :)

Then nothing needs to be changed and patches are OK.

Den

[Qemu-devel] [PATCH 2/2] guest-set-user-password - added ability to create new user

[Denis V. Lunev]

From: Yuriy Pudgorodskiy <yur@virtuozzo.com>

Added optional 'create' flag to guest-set-user-password command.
When it is specified, a new user will be created if it does not
exist yet.

The option to the existing command is added as password for newly created
user should be set as specified.

This code is made specifically for Linux/Windows and is inside proper
ifdef braces.

Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 20 ++++++++++++++++++++
 qga/commands-win32.c | 25 ++++++++++++++++++++++++-
 qga/qapi-schema.json |  5 ++++-
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 53e8d3b..2cd10df 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2089,6 +2089,8 @@ 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;
@@ -2125,6 +2127,24 @@ void qmp_guest_set_user_password(const char *username,
 
     chpasswd_argv[0] = passwd_path;
 
+    /* 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
+        };
+        ga_run_program(argv, NULL, "add new user", errp);
+        g_free(str);
+        g_free(cmd);
+        if (*errp) {
+            goto out;
+        }
+    }
+
     /* set password for existed user */
     if (!crypted) {
         /* wipe -e option */
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 61ffbdf..83fbf17 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1285,6 +1285,8 @@ 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;
@@ -1308,6 +1310,27 @@ void qmp_guest_set_user_password(const char *username,
     user = g_utf8_to_utf16(username, -1, NULL, NULL, NULL);
     wpass = g_utf8_to_utf16(rawpasswddata, -1, NULL, NULL, NULL);
 
+    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 out;
+        }
+
+        if (nas != NERR_UserExists) {
+            gchar *msg = get_net_error_message(nas);
+            error_setg(errp, "failed to add user: %s", msg);
+            g_free(msg);
+            goto out;
+        }
+    }
+
     pi1003.usri1003_password = wpass;
     nas = NetUserSetInfo(NULL, user,
                          1003, (LPBYTE)&pi1003,
@@ -1318,7 +1341,7 @@ void qmp_guest_set_user_password(const char *username,
         error_setg(errp, "failed to set password: %s", msg);
         g_free(msg);
     }
-
+out:
     g_free(user);
     g_free(wpass);
     g_free(rawpasswddata);
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 01c9ee4..53a9f6a 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -787,6 +787,8 @@
 # @username: the user account whose password to change
 # @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 (since 2.6).
+#          The default value is false.
 #
 # If the @crypted flag is true, it is the caller's responsibility
 # to ensure the correct crypt() encryption scheme is used. This
@@ -806,7 +808,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.1.4

Re: [Qemu-devel] [PATCH v2 0/2] qga: guest-set-user-password - added ability to create new user

[Denis V. Lunev]

On 01/06/2016 03:01 PM, Denis V. Lunev wrote:
> These patches add optional 'create' flag to guest-set-user-password command.
> When it is specified, a new user will be created if it does not
> exist yet.
>
> Since v1:
> - fixed english language mistakes in comments
> - json description now mentions 'create' as default to false
> - capture stdout/stderr from useradd/chpasswd and send iti back with the
>    error message to caller
> - split to two patches
>
> Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Eric Blake <eblake@redhat.com>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
>
> Yuriy Pudgorodskiy (2):
>    create ga_run_program() helper for guest-set-user-password
>    guest-set-user-password - added ability to create new user
>
>   qga/commands-posix.c | 215 +++++++++++++++++++++++++++++++++++++--------------
>   qga/commands-win32.c |  25 +++++-
>   qga/qapi-schema.json |   5 +-
>   3 files changed, 186 insertions(+), 59 deletions(-)
>
ping

Re: [Qemu-devel] [PATCH v2 0/2] qga: guest-set-user-password - added ability to create new user

[Marc-André Lureau]

Hi

On Wed, Jan 6, 2016 at 1:01 PM, Denis V. Lunev <den@openvz.org> wrote:
> These patches add optional 'create' flag to guest-set-user-password command.
> When it is specified, a new user will be created if it does not
> exist yet.
>

What's the motivation to re-use set-password instead of a new command?

> Since v1:
> - fixed english language mistakes in comments
> - json description now mentions 'create' as default to false
> - capture stdout/stderr from useradd/chpasswd and send iti back with the
>   error message to caller
> - split to two patches
>
> Signed-off-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Eric Blake <eblake@redhat.com>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
>
> Yuriy Pudgorodskiy (2):
>   create ga_run_program() helper for guest-set-user-password
>   guest-set-user-password - added ability to create new user
>
>  qga/commands-posix.c | 215 +++++++++++++++++++++++++++++++++++++--------------
>  qga/commands-win32.c |  25 +++++-
>  qga/qapi-schema.json |   5 +-
>  3 files changed, 186 insertions(+), 59 deletions(-)
>
> --
> 2.1.4
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 0/2] qga: guest-set-user-password - added ability to create new user

[Denis V. Lunev]

On 01/14/2016 05:18 PM, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jan 6, 2016 at 1:01 PM, Denis V. Lunev <den@openvz.org> wrote:
>> These patches add optional 'create' flag to guest-set-user-password command.
>> When it is specified, a new user will be created if it does not
>> exist yet.
>>
> What's the motivation to re-use set-password instead of a new command?

because we will have to change the password later on after addition
of such user. Also this looks better for a case "create if not exists" and
force new password.

Den

Re: [Qemu-devel] [PATCH v2 0/2] qga: guest-set-user-password - added ability to create new user

[Daniel P. Berrange]

On Thu, Jan 14, 2016 at 05:22:39PM +0300, Denis V. Lunev wrote:
> On 01/14/2016 05:18 PM, Marc-André Lureau wrote:
> >Hi
> >
> >On Wed, Jan 6, 2016 at 1:01 PM, Denis V. Lunev <den@openvz.org> wrote:
> >>These patches add optional 'create' flag to guest-set-user-password command.
> >>When it is specified, a new user will be created if it does not
> >>exist yet.
> >>
> >What's the motivation to re-use set-password instead of a new command?
> 
> because we will have to change the password later on after addition
> of such user. Also this looks better for a case "create if not exists" and
> force new password.

I don't think that's very compelling honestly. In addition when creating
user accounts there's a whole bunch more parameters you potentially want
to set besides just the username - see how many options exist with the
'useradd' command. Also with some users you might not want to set any
password. So if we want to create users via QGA, I think that having a
separate command makes more sense.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 0/2] qga: guest-set-user-password - added ability to create new user

[Yuriy Pudgorodskiy]

On 1/14/2016 5:46 PM, Daniel P. Berrange wrote:
> On Thu, Jan 14, 2016 at 05:22:39PM +0300, Denis V. Lunev wrote:
>> On 01/14/2016 05:18 PM, Marc-André Lureau wrote:
>>> Hi
>>>
>>> On Wed, Jan 6, 2016 at 1:01 PM, Denis V. Lunev <den@openvz.org> wrote:
>>>> These patches add optional 'create' flag to guest-set-user-password command.
>>>> When it is specified, a new user will be created if it does not
>>>> exist yet.
>>>>
>>> What's the motivation to re-use set-password instead of a new command?
>> because we will have to change the password later on after addition
>> of such user. Also this looks better for a case "create if not exists" and
>> force new password.
> I don't think that's very compelling honestly. In addition when creating
> user accounts there's a whole bunch more parameters you potentially want
> to set besides just the username - see how many options exist with the
> 'useradd' command. Also with some users you might not want to set any
> password. So if we want to create users via QGA, I think that having a
> separate command makes more sense.
>
> Regards,
> Daniel
There is a problem with a whole bunch of create user parameters  - they 
are platform
specific. Windows and Unix 'create user' API are rather different - 
developing support for
all parameters will probably lead to two commands - 'create_user_posix' 
and 'create_user_windows'.

If so, callers that want full control over user creation may call 
platform specific commands
over generic guest-exec - e.g. 'useradd' with many options and 'net 
user', 'net localgroup',
respectively.

We, in contradiction to such callers, want to add simpler 
platform-independent functionality
much like the os installers provides during initial setup  - e.g. just 
username and password
with other parameters be a reasonable default.

If that sounds logical to you - we may talk about reasons for defaults 
and extends to a minimal
parameter set (user plus password).

But creating a full separate 'user add' command when it is platform 
specific and user has ability
to call 'useradd' via exec - sounds like an overkill to me.

Re: [Qemu-devel] [PATCH v2 0/2] qga: guest-set-user-password - added ability to create new user

[Michael Roth]

Quoting Yuriy Pudgorodskiy (2016-01-20 05:30:17)
> On 1/14/2016 5:46 PM, Daniel P. Berrange wrote:
> > On Thu, Jan 14, 2016 at 05:22:39PM +0300, Denis V. Lunev wrote:
> >> On 01/14/2016 05:18 PM, Marc-André Lureau wrote:
> >>> Hi
> >>>
> >>> On Wed, Jan 6, 2016 at 1:01 PM, Denis V. Lunev <den@openvz.org> wrote:
> >>>> These patches add optional 'create' flag to guest-set-user-password command.
> >>>> When it is specified, a new user will be created if it does not
> >>>> exist yet.
> >>>>
> >>> What's the motivation to re-use set-password instead of a new command?
> >> because we will have to change the password later on after addition
> >> of such user. Also this looks better for a case "create if not exists" and
> >> force new password.
> > I don't think that's very compelling honestly. In addition when creating
> > user accounts there's a whole bunch more parameters you potentially want
> > to set besides just the username - see how many options exist with the
> > 'useradd' command. Also with some users you might not want to set any
> > password. So if we want to create users via QGA, I think that having a
> > separate command makes more sense.
> >
> > Regards,
> > Daniel
> There is a problem with a whole bunch of create user parameters  - they 
> are platform
> specific. Windows and Unix 'create user' API are rather different - 
> developing support for
> all parameters will probably lead to two commands - 'create_user_posix' 
> and 'create_user_windows'.
> 
> If so, callers that want full control over user creation may call 
> platform specific commands
> over generic guest-exec - e.g. 'useradd' with many options and 'net 
> user', 'net localgroup',
> respectively.
> 
> We, in contradiction to such callers, want to add simpler 
> platform-independent functionality
> much like the os installers provides during initial setup  - e.g. just 
> username and password
> with other parameters be a reasonable default.

I think that's a good interface to have, but even if the
platform-independant aspect of it is fairly basic functionality like
user/password with default groups/directory/etc, I don't see any reason
not to give it it's own command.

But I'm not convinced that we can't come up with an interface that's
both cross-platform and useful for basic user creation tasks. It
would be nice if the initial implementation was created with this
goal in mind...

If we relegate things like group assignments and other tuneables
to a set of separate, future interfaces like guest-user-modify-groups,
guest-user-set-password-expiration, etc. etc, what's the bare-minimum
for a cross-platform, useable guest-user-create?

user name, full name?, home directory, logon script/shell...

(all common/relevant to both win32 and posix btw)

Any others we can think of that are absolutely necessary for basic
user creation, that couldn't be modified through other interfaces in
the future?

I think that's the sort of interface we should introduce initially.

Blatantly platform-specific stuff can be relegated to platform-specific
commands with smaller scope, not necessarily full-blown rich interfaces
like user-create-posix, user-create-win32, etc.

> 
> If that sounds logical to you - we may talk about reasons for defaults 
> and extends to a minimal
> parameter set (user plus password).
> 
> But creating a full separate 'user add' command when it is platform 
> specific and user has ability
> to call 'useradd' via exec - sounds like an overkill to me.
> 
> 
> 
> 
> 
>