Message ID | 1450680006-21959-1-git-send-email-den@openvz.org |
---|---|
State | New |
Headers | show |
On 12/20/2015 11:40 PM, Denis V. Lunev wrote: > From: Yuri 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 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 <yur@virtuozzo.com> > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > 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? > + 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.
On 12/21/2015 05:00 PM, Eric Blake wrote: > On 12/20/2015 11:40 PM, Denis V. Lunev wrote: >> From: Yuri 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 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 <yur@virtuozzo.com> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Michael Roth <mdroth@linux.vnet.ibm.com> >> --- >> 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.
diff --git a/qga/commands-posix.c b/qga/commands-posix.c index c2ff970..5f60f08 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1951,6 +1951,8 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) void qmp_guest_set_user_password(const char *username, const char *password, bool crypted, + bool has_create, + bool create, Error **errp) { Error *local_err = NULL; @@ -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); + setsid(); + reopen_fd_to_null(0); + reopen_fd_to_null(1); + reopen_fd_to_null(2); + execle("/bin/sh", "sh", "-c", cmd, NULL, environ); + _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"); + goto out; + } + } + pid = fork(); if (pid == 0) { close(datafd[1]); diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 0654fe4..5269f8b 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -1281,6 +1281,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; @@ -1301,6 +1303,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, @@ -1311,7 +1334,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..2f97862 100644 --- a/qga/qapi-schema.json +++ 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) # # 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' } } # @GuestMemoryBlock: #