[pve-devel] [PATCH storage v3 2/5] Cephfs storage plugin

Thomas Lamprecht t.lamprecht at proxmox.com
Tue Jun 12 14:48:48 CEST 2018


On 6/11/18 11:31 AM, Alwin Antreich wrote:
>  - ability to mount through kernel and fuse client
>  - allow mount options
>  - get MONs from ceph config if not in storage.cfg
>  - allow the use of ceph config with fuse client
> 
> Signed-off-by: Alwin Antreich <a.antreich at proxmox.com>
> ---
>  PVE/API2/Storage/Config.pm  |   2 +-
>  PVE/Storage.pm              |   2 +
>  PVE/Storage/CephFSPlugin.pm | 225 ++++++++++++++++++++++++++++++++++++++++++++
>  PVE/Storage/Makefile        |   2 +-
>  PVE/Storage/Plugin.pm       |   1 +
>  debian/control              |   1 +
>  6 files changed, 231 insertions(+), 2 deletions(-)
>  create mode 100644 PVE/Storage/CephFSPlugin.pm
> 
> diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm
> index 3b38304..368a5c9 100755
> --- a/PVE/API2/Storage/Config.pm
> +++ b/PVE/API2/Storage/Config.pm
> @@ -171,7 +171,7 @@ __PACKAGE__->register_method ({
>  		    PVE::Storage::activate_storage($cfg, $baseid);
>  
>  		    PVE::Storage::LVMPlugin::lvm_create_volume_group($path, $opts->{vgname}, $opts->{shared});
> -		} elsif ($type eq 'rbd' && !defined($opts->{monhost})) {
> +		} elsif (($type eq 'rbd' || $type eq 'cephfs') && !defined($opts->{monhost})) {
>  		    my $ceph_admin_keyring = '/etc/pve/priv/ceph.client.admin.keyring';
>  		    my $ceph_storage_keyring = "/etc/pve/priv/ceph/${storeid}.keyring";
>  
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index d733380..f9732fe 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -28,6 +28,7 @@ use PVE::Storage::NFSPlugin;
>  use PVE::Storage::CIFSPlugin;
>  use PVE::Storage::ISCSIPlugin;
>  use PVE::Storage::RBDPlugin;
> +use PVE::Storage::CephFSPlugin;
>  use PVE::Storage::SheepdogPlugin;
>  use PVE::Storage::ISCSIDirectPlugin;
>  use PVE::Storage::GlusterfsPlugin;
> @@ -46,6 +47,7 @@ PVE::Storage::NFSPlugin->register();
>  PVE::Storage::CIFSPlugin->register();
>  PVE::Storage::ISCSIPlugin->register();
>  PVE::Storage::RBDPlugin->register();
> +PVE::Storage::CephFSPlugin->register();
>  PVE::Storage::SheepdogPlugin->register();
>  PVE::Storage::ISCSIDirectPlugin->register();
>  PVE::Storage::GlusterfsPlugin->register();
> diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
> new file mode 100644
> index 0000000..9882760
> --- /dev/null
> +++ b/PVE/Storage/CephFSPlugin.pm
> @@ -0,0 +1,225 @@
> +package PVE::Storage::CephFSPlugin;
> +
> +use strict;
> +use warnings;
> +use IO::File;
> +use Net::IP;
> +use File::Path;
> +use PVE::Tools qw(run_command);
> +use PVE::ProcFSTools;
> +use PVE::Storage::Plugin;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::CephStorageTools;

You add this module in the next patch (3/5) but use it already here,
which breaks build! Swap 3/5 before 2/5..
I'd also call it PVE::Storage::CephTools and put it there if we
have this here.

> +
> +use base qw(PVE::Storage::Plugin);
> +
> +my $parse_ceph_config = sub {

AFAIS, this is the direct copy of PVE::CephTools::parse_ceph_config ?
Why not move this into PVE::Storage::CephTools (called
PVE::CephStorageTools here) together with the monaddr function and
potential other functions from the existing package PVE::CephTools,
ideally making it obsolete so that only one is left.

> +    my ($filename) = @_;
> +
> +    my $cfg = {};
> +
> +    return $cfg if ! -f $filename;
> +
> +    my $fh = IO::File->new($filename, "r") ||
> +	die "unable to open '$filename' - $!\n";
> +
> +    my $section;
> +
> +    while (defined(my $line = <$fh>)) {
> +	$line =~ s/[;#].*$//;
> +	$line =~ s/^\s+//;
> +	$line =~ s/\s+$//;
> +	next if !$line;
> +
> +	$section = $1 if $line =~ m/^\[(\S+)\]$/;
> +	if (!$section) {
> +	    warn "no section - skip: $line\n";
> +	    next;
> +	}
> +
> +	if ($line =~ m/^(.*?\S)\s*=\s*(\S.*)$/) {
> +	    $cfg->{$section}->{$1} = $2;
> +	}
> +
> +    }
> +
> +    return $cfg;
> +};
> +
> +my $get_monaddr_list = sub {
> +    my ($scfg, $configfile) = @_;
> +
> +    my $server;
> +    my $no_mon = !defined($scfg->{monhost});
> +
> +    if (($no_mon) && defined($configfile)) {
> +	my $config = $parse_ceph_config->($configfile);
> +	$server = join(',', sort { $a cmp $b }

$a cmp $b is default, so not needed?

> +	    map { $config->{$_}->{'mon addr'} } grep {/mon/} %{$config});
> +    }else {

if $no_mon is false and $configfile is also we still get here,
with an undefined $scfg->{monhost}, can this happen? If so, I'd
error out above

> +	$server = PVE::CephStorageTools::hostlist($scfg->{monhost}, ',');
> +    }
> +
> +    return $server;
> +};
> +
> +sub cephfs_is_mounted {
> +    my ($scfg, $storeid, $mountdata) = @_;
> +
> +    my $cmd_option = PVE::CephStorageTools::ceph_connect_option($scfg, $storeid);
> +    my $configfile = $cmd_option->{ceph_conf};
> +    my $server = $get_monaddr_list->($scfg, $configfile);
> +
> +    my $subdir = $scfg->{subdir} ? $scfg->{subdir} : '/';

We normally use the following style for this:

my $subdir = $scfg->{subdir} // '/';

> +    my $mountpoint = $scfg->{path};
> +    my $source = "$server:$subdir";
> +
> +    $mountdata = PVE::ProcFSTools::parse_proc_mounts() if !$mountdata;
> +    return $mountpoint if grep {
> +	$_->[2] =~ m#^ceph|fuse\.ceph-fuse# &&
> +	$_->[0] =~ m#^\Q$source\E|ceph-fuse$# &&
> +	$_->[1] eq $mountpoint
> +    } @$mountdata;
> +
> +    warn "A filesystem is already mounted on $mountpoint\n"
> +	if grep { $_->[1] eq $mountpoint } @$mountdata;
> +
> +    return undef;
> +}
> +
> +sub cephfs_mount {
> +    my ($scfg, $storeid) = @_;
> +
> +    my $cmd;
> +    my $mountpoint = $scfg->{path};
> +    my $subdir = $scfg->{subdir} ? $scfg->{subdir} : '/';
> +
> +    my $cmd_option = PVE::CephStorageTools::ceph_connect_option($scfg, $storeid);
> +    my $configfile = $cmd_option->{ceph_conf};
> +    my $secretfile = $cmd_option->{keyring};
> +    my $server = $get_monaddr_list->($scfg, $configfile);
> +
> +    # fuse -> client-enforced quotas (kernel doesn't), updates w/ ceph-fuse pkg
> +    # kernel -> better performance, less frequent updates
> +    if ($scfg->{fuse}) {
> +	    # FIXME: ceph-fuse client complains about missing ceph.conf or keyring if
> +	    # not provided on its default locations but still connects. Fix upstream??
> +	    $cmd = ['/usr/bin/ceph-fuse', '-n', "client.$cmd_option->{userid}", '-m', $server];
> +	    push @$cmd, '--keyfile', $secretfile if defined($secretfile);
> +	    push @$cmd, '-r', $subdir if !($subdir =~ m|^/$|);
> +	    push @$cmd, $mountpoint;
> +	    push @$cmd, '--conf', $configfile if defined($configfile);
> +    }else {
> +	my $source = "$server:$subdir";
> +	$cmd = ['/bin/mount', '-t', 'ceph', $source, $mountpoint, '-o', "name=$cmd_option->{userid}"];
> +	push @$cmd, '-o', "secretfile=$secretfile" if defined($secretfile);
> +    }
> +
> +    if ($scfg->{options}) {
> +	push @$cmd, '-o', $scfg->{options};
> +    }
> +
> +    run_command($cmd, errmsg => "mount error");
> +}
> +
> +# Configuration
> +
> +sub type {
> +    return 'cephfs';
> +}
> +
> +sub plugindata {
> +    return {
> +	content => [ { vztmpl => 1, iso => 1, backup => 1},
> +		     { backup => 1 }],
> +    };
> +}
> +
> +sub properties {
> +    return {
> +	fuse => {
> +	    description => "Mount CephFS through FUSE.",
> +	    type => 'boolean',
> +	},
> +	subdir => {
> +	    description => "Subdir to mount.",
> +	    type => 'string', format => 'pve-storage-path',
> +	},
> +    };
> +}
> +
> +sub options {
> +    return {
> +	path => { fixed => 1 },
> +	monhost => { optional => 1},
> +	nodes => { optional => 1 },
> +	subdir => { optional => 1 },
> +	disable => { optional => 1 },
> +	options => { optional => 1 },
> +	username => { optional => 1 },

username? for what? gets nowhere used...
maybe a copy-is-my-hobby error from the CIFS plugin? :-)

> +	content => { optional => 1 },
> +	format => { optional => 1 },
> +	mkdir => { optional => 1 },
> +	fuse => { optional => 1 },
> +	bwlimit => { optional => 1 },
> +    };
> +}
> +
> +sub check_config {
> +    my ($class, $sectionId, $config, $create, $skipSchemaCheck) = @_;
> +
> +    $config->{path} = "/mnt/pve/$sectionId" if $create && !$config->{path};
> +
> +    return $class->SUPER::check_config($sectionId, $config, $create, $skipSchemaCheck);
> +}
> +
> +# Storage implementation
> +
> +sub status {
> +    my ($class, $storeid, $scfg, $cache) = @_;
> +
> +    $cache->{mountdata} = PVE::ProcFSTools::parse_proc_mounts()
> +	if !$cache->{mountdata};
> +
> +    return undef if !cephfs_is_mounted($scfg, $storeid, $cache->{mountdata});
> +
> +    return $class->SUPER::status($storeid, $scfg, $cache);
> +}
> +
> +sub activate_storage {
> +    my ($class, $storeid, $scfg, $cache) = @_;
> +
> +    $cache->{mountdata} = PVE::ProcFSTools::parse_proc_mounts()
> +	if !$cache->{mountdata};
> +
> +    my $path = $scfg->{path};
> +
> +    if (!cephfs_is_mounted($scfg, $storeid, $cache->{mountdata})) {
> +
> +	# NOTE: only call mkpath when not mounted (avoid hang
> +	# when cephfs is offline
> +
> +	mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir});

what? I do not understand this, who set's mkdir and why should it
tell us if cephfs is not mounted? 
I'd just omit above comment, it's more confusing than helpful, IMO.

> +
> +	die "unable to activate storage '$storeid' - " .
> +	    "directory '$path' does not exist\n" if ! -d $path;
> +
> +	cephfs_mount($scfg, $storeid);
> +    }
> +
> +    $class->SUPER::activate_storage($storeid, $scfg, $cache);
> +}
> +
> +sub deactivate_storage {
> +    my ($class, $storeid, $scfg, $cache) = @_;
> +
> +    $cache->{mountdata} = PVE::ProcFSTools::parse_proc_mounts()
> +	if !$cache->{mountdata};
> +
> +    my $path = $scfg->{path};
> +
> +    if (cephfs_is_mounted($scfg, $storeid, $cache->{mountdata})) {
> +	my $cmd = ['/bin/umount', $path];
> +	run_command($cmd, errmsg => 'umount error'); 

trailing whitespace

> +    }
> +}
> diff --git a/PVE/Storage/Makefile b/PVE/Storage/Makefile
> index 7b168fa..ad69532 100644
> --- a/PVE/Storage/Makefile
> +++ b/PVE/Storage/Makefile
> @@ -1,4 +1,4 @@
> -SOURCES=Plugin.pm DirPlugin.pm LVMPlugin.pm NFSPlugin.pm CIFSPlugin.pm ISCSIPlugin.pm RBDPlugin.pm SheepdogPlugin.pm ISCSIDirectPlugin.pm GlusterfsPlugin.pm ZFSPoolPlugin.pm ZFSPlugin.pm DRBDPlugin.pm LvmThinPlugin.pm
> +SOURCES=Plugin.pm DirPlugin.pm LVMPlugin.pm NFSPlugin.pm CIFSPlugin.pm ISCSIPlugin.pm CephFSPlugin.pm RBDPlugin.pm SheepdogPlugin.pm ISCSIDirectPlugin.pm GlusterfsPlugin.pm ZFSPoolPlugin.pm ZFSPlugin.pm DRBDPlugin.pm LvmThinPlugin.pm
>  
>  .PHONY: install
>  install:
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 163871d..3769e01 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -24,6 +24,7 @@ our @SHARED_STORAGE = (
>      'nfs',
>      'cifs',
>      'rbd',
> +    'cephfs',
>      'sheepdog',
>      'iscsidirect',
>      'glusterfs',
> diff --git a/debian/control b/debian/control
> index 908dd24..9923350 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -28,6 +28,7 @@ Depends: cstream,
>           udev,
>           smbclient,
>           ceph-common,
> +         ceph-fuse,
>           cifs-utils,
>           ${perl:Depends},
>  Description: Proxmox VE storage management library
> 




More information about the pve-devel mailing list