Re: [PATCH v8 3/3] qapi/monitor: allow VNC display id in set/expire_password

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Fabian Ebner <f.ebner@proxmox.com> writes:
> 
> > Am 09.02.22 um 15:07 schrieb Markus Armbruster:
> >> Fabian Ebner <f.ebner@proxmox.com> writes:
> >> 
> >>> From: Stefan Reiter <s.reiter@proxmox.com>
> >>>
> >>> It is possible to specify more than one VNC server on the command line,
> >>> either with an explicit ID or the auto-generated ones à la "default",
> >>> "vnc2", "vnc3", ...
> >>>
> >>> It is not possible to change the password on one of these extra VNC
> >>> displays though. Fix this by adding a "display" parameter to the
> >>> "set_password" and "expire_password" QMP and HMP commands.
> >>>
> >>> For HMP, the display is specified using the "-d" value flag.
> >>>
> >>> For QMP, the schema is updated to explicitly express the supported
> >>> variants of the commands with protocol-discriminated unions.
> 
> [...]
> 
> >>> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >>> index 70a9136ac2..cc2f4bdeba 100644
> >>> --- a/hmp-commands.hx
> >>> +++ b/hmp-commands.hx
> >>> @@ -1514,33 +1514,35 @@ ERST
> >>>  
> >>>      {
> >>>          .name       = "set_password",
> >>> -        .args_type  = "protocol:s,password:s,connected:s?",
> >>> -        .params     = "protocol password action-if-connected",
> >>> +        .args_type  = "protocol:s,password:s,display:-dV,connected:s?",
> >>> +        .params     = "protocol password [-d display] 
> >>> [action-if-connected]",
> >>>          .help       = "set spice/vnc password",
> >>>          .cmd        = hmp_set_password,
> >>>      },
> >>>  
> >>>  SRST
> >>> -``set_password [ vnc | spice ] password [ action-if-connected ]``
> >>> -  Change spice/vnc password.  *action-if-connected* specifies what
> >>> -  should happen in case a connection is established: *fail* makes the
> >>> -  password change fail.  *disconnect* changes the password and
> >>> +``set_password [ vnc | spice ] password [ -d display ] [ 
> >>> action-if-connected ]``
> >> 
> >> This is the first flag with an argument in HMP.  The alternative is
> >> another optional argument.
> >> 
> >> PRO optional argument: no need for PATCH 1.
> >> 
> >> PRO flag with argument: can specify the display without
> >> action-if-connected.
> >> 
> >> Dave, this is your call to make.
> >> 
> >
> > I'll go ahead with v9 once the decision is made.
> 
> Dave?

I think the flag with argument is clearer; HMP has a problem of
having a lot of optional arguments that get very order dependent which
is messy; so I'd go with the flag with argument.

Dave

> [...]
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK