From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50926) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aB1Fx-0008Sr-2x for qemu-devel@nongnu.org; Mon, 21 Dec 2015 09:16:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aB1Fr-0001QZ-66 for qemu-devel@nongnu.org; Mon, 21 Dec 2015 09:16:08 -0500 Received: from relay.parallels.com ([195.214.232.42]:47964) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aB1Fq-0001QH-UW for qemu-devel@nongnu.org; Mon, 21 Dec 2015 09:16:03 -0500 References: <1450680006-21959-1-git-send-email-den@openvz.org> <56780611.6050201@redhat.com> From: "Denis V. Lunev" Message-ID: <56780991.7000306@openvz.org> Date: Mon, 21 Dec 2015 17:15:45 +0300 MIME-Version: 1.0 In-Reply-To: <56780611.6050201@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] qga: guest-set-user-password - added ability to create new user List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Yuri Pudgorodskiy , qemu-devel@nongnu.org, Michael Roth On 12/21/2015 05:00 PM, Eric Blake wrote: > On 12/20/2015 11:40 PM, Denis V. Lunev wrote: >> From: Yuri Pudgorodskiy >> >> Added optional 'create' flag to guest-set-user-password command. >> When it is specified, a new user will be created if it is not >> exists yet. > s/is not exists/does not exist/ > >> The option to the existing command is added as password for newly created >> user should be set as specified. >> >> Signed-off-by: Yuri Pudgorodskiy >> Signed-off-by: Denis V. Lunev >> CC: Michael Roth >> --- >> qga/commands-posix.c | 36 ++++++++++++++++++++++++++++++++++++ >> qga/commands-win32.c | 25 ++++++++++++++++++++++++- >> qga/qapi-schema.json | 3 ++- >> 3 files changed, 62 insertions(+), 2 deletions(-) >> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> @@ -1993,6 +1995,40 @@ void qmp_guest_set_user_password(const char *username, >> goto out; >> } >> >> + /* create new user if requested */ >> + if (has_create && create) { >> + pid = fork(); >> + if (pid == 0) { >> + char *str = g_shell_quote(username); >> + char *cmd = g_strdup_printf("id %s || useradd -m %s", str, str); > useradd is Linux-specific; should we be trying harder to make this > command portable to all POSIX-y guests? this code is inside #if defined(__linux__) as implemented by the original author. >> + setsid(); >> + reopen_fd_to_null(0); >> + reopen_fd_to_null(1); >> + reopen_fd_to_null(2); >> + execle("/bin/sh", "sh", "-c", cmd, NULL, environ); > By redirecting stderr to /dev/null, you've lost any error messages that > useradd tries to report... > >> + _exit(EXIT_FAILURE); >> + } else if (pid < 0) { >> + error_setg_errno(errp, errno, "failed to create child process"); >> + goto out; >> + } >> + >> + 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 add new user"); > ...and replaced it with a less-helpful message. Should you try harder to > pass through the real reason for failure? > >> +++ b/qga/qapi-schema.json >> @@ -787,6 +787,7 @@ >> # @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: #optional user will be created if not exists (since 2.6) > s/if not exists/if it does not exist/ > > may want to mention that it defaults to 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 +807,7 @@ >> # Since 2.3 >> ## >> { 'command': 'guest-set-user-password', >> - 'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool' } } >> + 'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool', '*create': 'bool' } } > Long line; please wrap to keep things under 80 columns. > the rest is clear. We will try.