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

["Denis V. Lunev" <den@openvz.org>]

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.