[pve-devel] [PATCH manager 1/2] pve5to6: check ceph config for mon_host line

[Fabian Grünbichler]

On Thu, Jul 04, 2019 at 10:16:13AM +0200, Thomas Lamprecht wrote:
> On 7/4/19 10:07 AM, Fabian Grünbichler wrote:
> > On Thu, Jul 04, 2019 at 09:35:12AM +0200, Dominik Csapak wrote:
> >> this already works on luminous, so it does not harm to add it already,
> >> and is recommended when both msgr1 and msgr2 is activated in nautilus
> >>
> >> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> > 
> > check for global->keyring would be good as well.
> > 
> >> ---
> >>  PVE/CLI/pve5to6.pm | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/PVE/CLI/pve5to6.pm b/PVE/CLI/pve5to6.pm
> >> index c167ebca..bb54c735 100644
> >> --- a/PVE/CLI/pve5to6.pm
> >> +++ b/PVE/CLI/pve5to6.pm
> >> @@ -421,6 +421,19 @@ sub check_ceph {
> >>  	}
> >>      }
> >>  
> >> +    log_info("checking Ceph config..");
> >> +    my $conf = PVE::Cluster::cfs_read_file('ceph.conf');
> >> +    if (defined($conf)) {
> > 
> > should always be defined, as neither cfs_read_file should return undef
> > itself, nor can the parser for ceph.conf.
> 
> FYI: if the file does not exists you get undef...
> https://git.proxmox.com/?p=pve-cluster.git;a=blob;f=data/PVE/Cluster.pm;h=b53dcd726f9d42cd8e94c59fb584818e79c94ad7;hb=HEAD#l873
> also it does not harm, it's just a safe guard.

no, the parser is called with $raw set to undef, and this parser returns
an empty hash if $raw is undef ;)

> 
> > 
> >> +	my $global = $conf->{global};
> > 
> > but this might be undef ?
> 
> which then returns just undef, it is not created by this, thus
> the checks on $global->{subkey} does not result in any auto-vivification
> so just fine.

true, I always forget that undef->{foo} does not trigger a warning, just
printing it would.

> >> +	if (!defined($global->{mon_host}) && !defined($global->{"mon host"})) {
> >> +	    log_warn("No mon_host entry found in ceph config.\n  It is recommended to add mon_host with all monitor addresses(without ports) to the global section.");
> >> +	} else {
> >> +	    log_pass("Found mon_host entry.");
> >> +	}
> >> +    } else {
> >> +	log_skip("no ceph config found");
> > 
> > this else branch should be dead code, since we checked ceph_inited right at the
> > start of this sub ;)
> 
> that one should be true, though ;)
> 
> > 
> >> +    }
> >> +
> >>      my $local_ceph_ver = PVE::Ceph::Tools::get_local_version(1);
> >>      if (defined($local_ceph_ver)) {
> >>  	if ($local_ceph_ver == 14) {
> >> -- 
> >> 2.20.1
> >>
> >>
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel