[pve-devel] [PATCH v2 cluster 2/2] cluster: use lock for legacy authkey generation

[Fabian Grünbichler]

On Thu, Mar 07, 2019 at 07:59:01AM +0100, Thomas Lamprecht wrote:
> On 3/6/19 11:30 AM, Fabian Grünbichler wrote:
> > Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> > ---
> > Notes:
> >     unchanged since v1
> > 
> >  data/PVE/Cluster.pm | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
> > index 83b401c..b31dfa5 100644
> > --- a/data/PVE/Cluster.pm
> > +++ b/data/PVE/Cluster.pm
> > @@ -158,11 +158,15 @@ sub gen_auth_key {
> >  
> >      check_cfs_is_mounted();
> >  
> > -    mkdir $authdir || $! == EEXIST || die "unable to create dir '$authdir' - $!\n";
> > +    my $res = cfs_lock_authkey(undef, sub {
> > +	mkdir $authdir || $! == EEXIST || die "unable to create dir '$authdir' - $!\n";
> >  
> > -    run_silent_cmd(['openssl', 'genrsa', '-out', $authprivkeyfn, '2048']);
> > +	run_silent_cmd(['openssl', 'genrsa', '-out', $authprivkeyfn, '2048']);
> >  
> > -    run_silent_cmd(['openssl', 'rsa', '-in', $authprivkeyfn, '-pubout', '-out', $authpubkeyfn]);
> > +	run_silent_cmd(['openssl', 'rsa', '-in', $authprivkeyfn, '-pubout', '-out', $authpubkeyfn]);
> > +    });
> > +
> > +    die "$@\n" if !defined($res);
> 
> this is  a bit strange to me, there's no explicit return in the $code, so the
> last statement's result will be used? 
> If you use "$@" already, and thus assume that it's set here, can't you just do
> an "normal":
> > die "$@\n" if $@;
> thingy here?

I got confused by the "set $@ and return undef" semantics of cfs_lock_*,
but indeed we always get "defined $@ and return undef" in case of an
error, and "undefined $@ and return whatever the code returned" in case
of success - so we can just use $@ and drop $res here, even without a
visible eval ;)

will adapt both here and in AccessControl.pm accordingly for v3

> 
> >  }
> >  
> >  sub gen_pveca_key {
> > 
> 
>