[pve-devel] [RFC qemu-server] rewrite description for vm_config

[Oguz Bektas]

hi,

On Thu, Sep 05, 2019 at 09:54:22AM +0200, Fabian Grünbichler wrote:
> On September 4, 2019 3:59 pm, Oguz Bektas wrote:
> > the description doesn't match the default behaviour, which is to replace
> > the current values with pending ones in the returned config, unless the
> > 'current' option is passed.
> > 
> > Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> > ---
> > 
> > i tried to come up with a reasonable description, but sending it as RFC
> > just in case someone else comes up with something better
> > 
> >  PVE/API2/Qemu.pm | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> > index b30931d..36487ee 100644
> > --- a/PVE/API2/Qemu.pm
> > +++ b/PVE/API2/Qemu.pm
> > @@ -827,7 +827,8 @@ __PACKAGE__->register_method({
> >      path => '{vmid}/config',
> >      method => 'GET',
> >      proxyto => 'node',
> > -    description => "Get current virtual machine configuration. This does not include pending configuration changes (see 'pending' API).",
> > +    description => "Get virtual machine configuration. If the 'current' option is" .
> > +		    " not passed, pending changes will replace the current ones.",
> 
> besides style, this is also incorrect since you can pass '0' for 
> 'current' ;)

passing 0 to 'current' means the default behaviour, i don't think anyone
will do that :P

but we could rephrase like:

...
If the current option is not set, pending changes will replace the
current ones.

> 
> Get virtual machine configuration. If 'current' is set, any pending 
> configuration changes will not be included in the returned value. 
> Otherwise, the returned configuration values will represent the state 
> with all pending changes applied.
> 
> A bit verbose, but matching what it does. Alternatively, just
a bit? :O
> 
> Get virtual machine configuration. See 'pending' API for current and 
> pending configuration retrieval.

i find this one insufficient, since it doesn't explain the fact that the
current options will be replaced with the pending ones.

> 
> and leave the rest to the parameter description?
> 
> current -> "Get current values only (ignoring pending changes)."
> 
> >      permissions => {
> >  	check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
> >      },
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel at pve.proxmox.com
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> > 
> > 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>