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

Fri Jun 1 08:09:26 CEST 2012

Sorry for the trailing whitespace,I'll correct that as soon as possible :(

>>Do we really need that 'rbd_prefix' here? 
I put rbd_ prefix because I see that if 2 storages plugins have the same properties defined, it's conflicting.
Maybe in the future other plugins need id,key,pool properties with differents syntax/parser ?
But it's ok for me to remove it if you want.

>>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? 
Sure, it's ok for me.  (We can have 10 monhost max)

>>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'. 

must be only ip for the moment, as qemu-rbd complain if dns name is use

> rbd_id admin 
> rbd_key AQAmOcZPwNY7GRAAuvJjVAKIm1r3JKqLCa4LGQ== 
> rbd_authsupported cephx;none 
>Do we really want to store such sensitive data here? 

Sure, I have do it to be easy to implement.
the key can be stored in a external file, /etc/ceph/client.rbd_id.keyring.
But we need to replicate the file between the proxmox hosts. (I don't know how to do this ;)

>>(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? 

same here, we can use external file (with only root rights)

file sample
------------
#cat /etc/ceph/client.admin.keyring

[client.admin]
        key = AQAmOcZPwNY7GRAAuvJjVAKIm1r3JKqLCa4LGQ==

----- Mail original ----- 

De: "Dietmar Maurer" <dietmar at proxmox.com> 
À: "Alexandre Derumier" <aderumier at odiso.com>, pve-devel at pve.proxmox.com 
Envoyé: Vendredi 1 Juin 2012 07:49:31 
Objet: RE: [pve-devel] [PATCH] Add rados block plugin storage 

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 

-- 

-- 

	Alexandre D erumier 
Ingénieur Système 
Fixe : 03 20 68 88 90 
Fax : 03 20 68 90 81 
45 Bvd du Général Leclerc 59100 Roubaix - France 
12 rue Marivaux 75002 Paris - France 