[pve-devel] [PATCH] Add rados block plugin storage

[Dietmar Maurer]

I always get warning with you patches:

git am your-patch.mbox
Applying: Add rados block plugin storage
/home/dietmar/pve2-devel/pve-storage/.git/rebase-apply/patch:70: trailing whitespace.
	
/home/dietmar/pve2-devel/pve-storage/.git/rebase-apply/patch:90: trailing whitespace.
# Configuration 
/home/dietmar/pve2-devel/pve-storage/.git/rebase-apply/patch:119: trailing whitespace.
	    type => 'string', 
/home/dietmar/pve2-devel/pve-storage/.git/rebase-apply/patch:123: trailing whitespace.
	    type => 'string', 
/home/dietmar/pve2-devel/pve-storage/.git/rebase-apply/patch:183: trailing whitespace.
    die "illegal name '$name' - sould be 'vm-$vmid-*'\n" 
warning: squelched 1 whitespace error
warning: 6 lines add whitespace errors.

Anyway, I committed it as it is.

Inline comments follow:

> /etc/pve/storage.cfg sample config
> 
> rbd: rbdtest
>         rbd_monhost 10.3.94.27:6789;10.3.94.28:6789;10.3.94.29:6789	

Do we really need that 'rbd_prefix' here?

Also, it is usually easier to use separate properties:

monhost1 10.3.94.27:6789
monhost2 10.3.94.28:6789
monhost3 10.3.94.29:6789

What do you think?

We already have a json format type for 'IP or DNS name with optional port',
we use that in ISCSIPlugin for portal. It is called 'pve-storage-portal-dns'.

>         rbd_pool pool2

Again, is that prefix necessary? Maybe 'pool' is good enough?

>         rbd_id admin
>         rbd_key AQAmOcZPwNY7GRAAuvJjVAKIm1r3JKqLCa4LGQ==
>         rbd_authsupported cephx;none

Do we really want to store such sensitive data here?

# ls -l /etc/pve/storage.cfg
-rw-r----- 1 root www-data 430 May 29 10:33 /etc/pve/storage.cfg

(it is readable by group 'www-data').

You even pass that key (path()) on the command line to kvm (all system users can see that key)!

Isn't that key security sensitive?

>         content images
> 
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> ---
>  PVE/Storage.pm           |    3 +
>  PVE/Storage/Makefile     |    2 +-
>  PVE/Storage/RBDPlugin.pm |  247
> ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 251 insertions(+), 1 deletions(-)  create mode 100644
> PVE/Storage/RBDPlugin.pm
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm index 99f09da..ffe2456
> 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -21,12 +21,15 @@ use PVE::Storage::DirPlugin;  use
> PVE::Storage::LVMPlugin;  use PVE::Storage::NFSPlugin;  use
> PVE::Storage::ISCSIPlugin;
> +use PVE::Storage::RBDPlugin;
> 
>  # load and initialize all plugins
>  PVE::Storage::DirPlugin->register();
>  PVE::Storage::LVMPlugin->register();
>  PVE::Storage::NFSPlugin->register();
>  PVE::Storage::ISCSIPlugin->register();
> +PVE::Storage::RBDPlugin->register();
> +
>  PVE::Storage::Plugin->init();
> 
>  my $UDEVADM = '/sbin/udevadm';
> diff --git a/PVE/Storage/Makefile b/PVE/Storage/Makefile index
> 1d2322f..0f9950a 100644
> --- a/PVE/Storage/Makefile
> +++ b/PVE/Storage/Makefile
> @@ -1,4 +1,4 @@
> -SOURCES=Plugin.pm DirPlugin.pm LVMPlugin.pm NFSPlugin.pm
> ISCSIPlugin.pm
> +SOURCES=Plugin.pm DirPlugin.pm LVMPlugin.pm NFSPlugin.pm
> ISCSIPlugin.pm
> +RBDPlugin.pm
> 
>  .PHONY: install
>  install:
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm new file
> mode 100644 index 0000000..7edbbeb
> --- /dev/null
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -0,0 +1,247 @@
> +package PVE::Storage::RBDPlugin;
> +
> +use strict;
> +use warnings;
> +use IO::File;
> +use PVE::Tools qw(run_command trim);
> +use PVE::Storage::Plugin;
> +use PVE::JSONSchema qw(get_standard_option);
> +
> +use base qw(PVE::Storage::Plugin);
> +
> +
> +sub rbd_ls{
> + my ($scfg) = @_;
> +
> +    my $rbdpool = $scfg->{rbd_pool};
> +    my $monhost = $scfg->{rbd_monhost};
> +    $monhost =~ s/;/,/g;
> +
> +    my $cmd = ['/usr/bin/rbd', '-p', $rbdpool, '-m', $monhost, '-n',
> "client.".$scfg->{rbd_id} ,'--key',$scfg->{rbd_key} ,'ls' ];
> +    my $list = {};
> +    run_command($cmd, errfunc => sub {},outfunc => sub {
> +        my $line = shift;
> +
> +        $line = trim($line);
> +        my ($image) = $line;
> +
> +        $list->{$rbdpool}->{$image} = {
> +            name => $image,
> +            size => "",
> +        };
> +
> +    });
> +
> +
> +    return $list;
> +
> +}
> +
> +sub addslashes {
> +    my $text = shift;
> +    $text =~ s/;/\\;/g;
> +    $text =~ s/:/\\:/g;
> +    return $text;
> +}
> +
> +# Configuration
> +
> +PVE::JSONSchema::register_format('pve-storage-rbd-mon',
> +\&parse_rbd_mon); sub parse_rbd_mon {
> +    my ($name, $noerr) = @_;
> +
> +    if ($name !~ m/^[a-z][a-z0-9\-\_\.]*[a-z0-9]$/i) {
> +	return undef if $noerr;
> +	die "lvm name '$name' contains illegal characters\n";

lvm?

> +    }
> +
> +    return $name;
> +}

As mentioned above, this is similar to 'pve-storage-portal-dns'?

> +
> +
> +sub type {
> +    return 'rbd';
> +}
> +
> +sub plugindata {
> +    return {
> +	content => [ {images => 1}, { images => 1 }],
> +    };
> +}
> +
> +sub properties {
> +    return {
> +	rbd_monhost => {
> +	    description => "Monitors daemon ips.",

with optional port?

> +	    type => 'string',

you need to specify the format too:

format => 'pve-storage-rbd-mon-list'

> +	},
> +	rbd_pool => {
> +	    description => "RBD Pool.",
> +	    type => 'string',
> +	},


no format or pattern?


Many thanks for the patch,

- Dietmar