On Wed, Jan 23, 2019 at 03:34:56PM +0100, Dominik Csapak wrote:
> On 1/23/19 3:27 PM, Fabian Grünbichler wrote:
> > On Mon, Jan 21, 2019 at 09:44:35AM +0100, Dominik Csapak wrote:
> > > this adds a new config option for it, and executes it on four
> > > points in time:
> > >
> > > 'pre-start'
> > > 'post-start'
> > > 'pre-stop'
> > > 'post-stop'
> > >
> > > on pre-start we abort if the script fails
> > > and pre-stop will not be called if the vm crashes or if
> > > the vm gets powered off from inside the guest
> > >
> > > Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
> > > ---
> > > PVE/API2/Qemu.pm | 8 ++++++++
> > > PVE/CLI/qm.pm | 2 ++
> > > PVE/QemuServer.pm | 12 ++++++++++++
> > > 3 files changed, 22 insertions(+)
> > >
> > > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> > > index b55fd13..13aa021 100644
> > > --- a/PVE/API2/Qemu.pm
> > > +++ b/PVE/API2/Qemu.pm
> > > @@ -1101,6 +1101,14 @@ my $update_vm_api = sub {
> > > if ($param->{$opt} eq '1') {
> > > $param->{$opt} = PVE::QemuServer::generate_uuid();
> > > }
> > > + } elsif ($opt eq 'hookscript') {
> > > + my ($path, undef, $type) = PVE::Storage::path($storecfg,
> > > $param->{$opt});
> >
> > IMHO this should be limited to root@pam here as well (just because an
> > admin put an executable in some scripts directory does not mean they
> > want every user that can modify their VM config to execute that script
> > as root - potentially that script then decides stuff based on VM config
> > content as well, etc. pp.). once we establish a ACL scheme for this
> > scripts we might relax this here, but for now let's keep it simple &
> > safe by default.
>
> any new config option is by default root@pam only
> (see PVE/API2/Qemu.pm:335)
>
> so this is already done?
fair enough - as discussed off-disk ;)
> > > + raise_param_exc({ $opt => "is not in the scripts directory" })
> > > + if $type ne 'scripts';
> > > +
> > > + warn "script '$path' not found, setting anyway\n"
> > > + if ! -f $path;
> >
> > does this make sense?
>
> what exactly? i did not want the user to be able to set anything
> (e.g. 'local:5' which is a valid volume-id but nonsense in this
> context), but i did not want the user to be restricted
> to first put the script in the storage and then set it
sorry - not failing if the hook script does not exist. we don't allow
adding non-existing volumes as disks either, and IMHO the usual
assumption would be that referencing a hook script to be executed which
does not exist is an error (e.g., a typo, or something like that).
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
