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  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.
> 
> 
> 
> 
> 
> 

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  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

[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  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

[Marc-André Lureau]

Hi

On Wed, Jan 6, 2016 at 1: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.
>

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 
> Signed-off-by: Denis V. Lunev 
> CC: Eric Blake 
> CC: Michael Roth 
>
> 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

[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  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

[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 
Signed-off-by: Denis V. Lunev 
CC: Eric Blake 
CC: Michael Roth 

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

[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 
Signed-off-by: Denis V. Lunev 
CC: Eric Blake 
CC: Michael Roth 

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