On Tue, Mar 20, 2018 at 04:00:51PM +0100, Thomas Lamprecht wrote:
> a bit strange to be honest, there's already:
> # pct unlock VMID
>
> doing the same thing in a completely other matter,
> using LXC::Config->remove_lock (inherited from AbstractConfig)
yes, but that one is a single-use convenience command.
> Ideally, both the API and the CLI helper share their code, after your patch.
they do, but you need to look at the right API / CLI :-P
its 'pct set' and 'update_vm', not 'pct unlock' and 'update_vm'. we
could of course (also?) introduce a new 'unlock_vm' API call - but that
seems kind of redundant to me.
> I'd not allow changing params on a locked VM, to much strange side effects..
it's only possible for root anyhow, and I don't see how the side effects
get stranger than if the user does 'pct unlock XX; pct set XX -foo bar'?
we are already in manual error recovery territory here anyway..
> Allow to remove the lock as its own operation, then a user can do everything
> again.
>
> What about VMs?
the patch is modeled after the qemu API, which has the exact same
behaviour (including all of the negative points you mention above).
we already have
- qm unlock / pct unlock (consistent)
- qm set / pct set (inconsistent)
- update_vm / update_vm (inconsistent)
this patch makes the latter two variants consistent again (and also
consistent with the automatically generated API documentation, see the
bug report).
> On 3/16/18 5:30 PM, Alwin Antreich wrote:
> > Signed-off-by: Alwin Antreich <a.antre...@proxmox.com>
> > ---
> > src/PVE/API2/LXC/Config.pm | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
> > index 2b622b3..2d69049 100644
> > --- a/src/PVE/API2/LXC/Config.pm
> > +++ b/src/PVE/API2/LXC/Config.pm
> > @@ -80,6 +80,7 @@ __PACKAGE__->register_method({
> > {
> > node => get_standard_option('pve-node'),
> > vmid => get_standard_option('pve-vmid', { completion =>
> > \&PVE::LXC::complete_ctid }),
> > + skiplock => get_standard_option('skiplock'),
> > delete => {
> > type => 'string', format => 'pve-configid-list',
> > description => "A list of settings you want to delete.",
> > @@ -107,6 +108,10 @@ __PACKAGE__->register_method({
> >
> > my $digest = extract_param($param, 'digest');
> >
> > + my $skiplock = extract_param($param, 'skiplock');
> > + raise_param_exc({ skiplock => "Only root may use this option." })
> > + if $skiplock && $authuser ne 'root@pam';
> > +
> > die "no options specified\n" if !scalar(keys %$param);
> >
> > my $delete_str = extract_param($param, 'delete');
> > @@ -155,7 +160,7 @@ __PACKAGE__->register_method({
> > my $code = sub {
> >
> > my $conf = PVE::LXC::Config->load_config($vmid);
> > - PVE::LXC::Config->check_lock($conf);
> > + PVE::LXC::Config->check_lock($conf) if !$skiplock;
> >
> > PVE::Tools::assert_if_modified($digest, $conf->{digest});
> >
> >
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
