* [pve-devel] [PATCH RFC storage] rbd: fix #3286 add namespace support
@ 2021-03-01 15:12 Aaron Lauterer
2021-03-01 16:13 ` Thomas Lamprecht
2021-03-03 10:10 ` aderumier
0 siblings, 2 replies; 4+ messages in thread
From: Aaron Lauterer @ 2021-03-01 15:12 UTC (permalink / raw)
To: pve-devel
This RFC introduces support for Cephs RBD namespaces.
A new storage config parameter 'namespace' defines the namespace to be
used for the RBD storage.
The namespace must already exist in the Ceph cluster as it is not
automatically created.
The main intention is to use this for external Ceph clusters. With
namespaces, each PVE cluster can get its own namespace and will not
conflict with other PVE clusters.
Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
There are two ways to address namespaces. One is the '--namespace'
parameter which is not supported by all rbd subcommands though!
The other is the path style '<pool>/<namespace>/<image>' way.
Where possible I inject the '--namespace' parameter to the @$cmd but
sometimes it is necessary to use the path style. There might be a nicer
way to inject the namespace in these cases than what I have now.
It would be good to have some tests but this cannot be done at build
time since it requires a ceph cluster with namespaces configured.
Therefore having a test script that can be called manually in a fitting
test environment is what I still have planned (thanks @Dominik for the
hint).
Should I just place that in the 'test' directory of the repo without
adding it to the Makefile?
PVE/Storage/RBDPlugin.pm | 57 +++++++++++++++++++++++++++++++++-------
1 file changed, 48 insertions(+), 9 deletions(-)
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index fab6d57..8a6329f 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -27,7 +27,9 @@ my $add_pool_to_disk = sub {
my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
- return "$pool/$disk";
+ my $namespace = $scfg->{namespace} ? "/$scfg->{namespace}" : "";
+
+ return "$pool$namespace/$disk";
};
my $build_cmd = sub {
@@ -77,6 +79,8 @@ my $librados_connect = sub {
my $krbd_feature_update = sub {
my ($scfg, $storeid, $name) = @_;
+ my $namespace = $scfg->{namespace};
+
my (@disable, @enable);
my ($kmajor, $kminor) = PVE::ProcFSTools::kernel_version();
@@ -102,6 +106,7 @@ my $krbd_feature_update = sub {
if ($to_disable) {
print "disable RBD image features this kernel RBD drivers is not compatible with: $to_disable\n";
my $cmd = $rbd_cmd->($scfg, $storeid, 'feature', 'disable', $name, $to_disable);
+ push @$cmd, '--namespace', $namespace if $namespace;
run_rbd_command(
$cmd,
errmsg => "could not disable krbd-incompatible image features '$to_disable' for rbd image: $name",
@@ -111,6 +116,7 @@ my $krbd_feature_update = sub {
print "enable RBD image features this kernel RBD drivers supports: $to_enable\n";
eval {
my $cmd = $rbd_cmd->($scfg, $storeid, 'feature', 'enable', $name, $to_enable);
+ push @$cmd, '--namespace', $namespace if $namespace;
run_rbd_command(
$cmd,
errmsg => "could not enable krbd-compatible image features '$to_enable' for rbd image: $name",
@@ -153,7 +159,10 @@ sub rbd_ls {
my ($scfg, $storeid) = @_;
my $cmd = &$rbd_cmd($scfg, $storeid, 'ls', '-l', '--format', 'json');
+ my $namespace = $scfg->{namespace};
+ push @$cmd, '--namespace', $namespace if $namespace;
my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
+ $pool .= "/${namespace}" if $namespace;
my $raw = '';
my $parser = sub { $raw .= shift };
@@ -199,6 +208,7 @@ sub rbd_ls_snap {
my ($scfg, $storeid, $name) = @_;
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'ls', $name, '--format', 'json');
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
my $raw = '';
run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => sub { $raw .= shift; });
@@ -238,6 +248,7 @@ sub rbd_volume_info {
}
$cmd = &$rbd_cmd($scfg, $storeid, @options);
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
my $raw = '';
my $parser = sub { $raw .= shift };
@@ -281,6 +292,10 @@ sub properties {
description => "Pool.",
type => 'string',
},
+ namespace=> {
+ description => "RBD Namespace.",
+ type => 'string',
+ },
username => {
description => "RBD Id.",
type => 'string',
@@ -302,6 +317,7 @@ sub options {
disable => { optional => 1 },
monhost => { optional => 1},
pool => { optional => 1 },
+ namespace => { optional => 1 },
username => { optional => 1 },
content => { optional => 1 },
krbd => { optional => 1 },
@@ -349,9 +365,10 @@ sub path {
$name .= '@'.$snapname if $snapname;
my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
- return ("/dev/rbd/$pool/$name", $vmid, $vtype) if $scfg->{krbd};
+ my $namespace = $scfg->{namespace} ? "/$scfg->{namespace}" : "";
+ return ("/dev/rbd/${pool}${namespace}/${name}", $vmid, $vtype) if $scfg->{krbd};
- my $path = "rbd:$pool/$name";
+ my $path = "rbd:${pool}${namespace}/${name}";
$path .= ":conf=$cmd_option->{ceph_conf}" if $cmd_option->{ceph_conf};
if (defined($scfg->{monhost})) {
@@ -370,6 +387,8 @@ sub find_free_diskname {
my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
my $cmd = &$rbd_cmd($scfg, $storeid, 'ls');
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
+
my $disk_list = [];
my $parser = sub {
@@ -423,6 +442,7 @@ sub create_base {
if (!$protected){
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'protect', $newname, '--snap', $snap);
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
run_rbd_command($cmd, errmsg => "rbd protect $newname snap '$snap' error");
}
@@ -451,6 +471,7 @@ sub clone_image {
if (!$protected) {
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'protect', $volname, '--snap', $snapname);
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
run_rbd_command($cmd, errmsg => "rbd protect $volname snap $snapname error");
}
}
@@ -476,6 +497,7 @@ sub alloc_image {
$name = $class->find_free_diskname($storeid, $scfg, $vmid) if !$name;
my $cmd = &$rbd_cmd($scfg, $storeid, 'create', '--image-format' , 2, '--size', int(($size+1023)/1024), $name);
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
run_rbd_command($cmd, errmsg => "rbd create $name' error");
return $name;
@@ -487,10 +509,13 @@ sub free_image {
my ($vtype, $name, $vmid, undef, undef, undef) =
$class->parse_volname($volname);
+ my $namespace = $scfg->{namespace};
+
my $snaps = rbd_ls_snap($scfg, $storeid, $name);
foreach my $snap (keys %$snaps) {
if ($snaps->{$snap}->{protected}) {
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'unprotect', $name, '--snap', $snap);
+ push @$cmd, '--namespace', $namespace if $namespace;
run_rbd_command($cmd, errmsg => "rbd unprotect $name snap '$snap' error");
}
}
@@ -498,9 +523,11 @@ sub free_image {
$class->deactivate_volume($storeid, $scfg, $volname);
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'purge', $name);
+ push @$cmd, '--namespace', $namespace if $namespace;
run_rbd_command($cmd, errmsg => "rbd snap purge '$volname' error");
$cmd = &$rbd_cmd($scfg, $storeid, 'rm', $name);
+ push @$cmd, '--namespace', $namespace if $namespace;
run_rbd_command($cmd, errmsg => "rbd rm '$volname' error");
return undef;
@@ -511,6 +538,8 @@ sub list_images {
$cache->{rbd} = rbd_ls($scfg, $storeid) if !$cache->{rbd};
my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
+ my $namespace = $scfg->{namespace};
+ $pool .= "/${namespace}" if $namespace;
my $res = [];
@@ -575,9 +604,11 @@ sub deactivate_storage {
}
my $get_kernel_device_name = sub {
- my ($pool, $name) = @_;
+ my ($pool, $name, $namespace) = @_;
+
+ return "/dev/rbd/${pool}/${namespace}/${name}" if $namespace;
- return "/dev/rbd/$pool/$name";
+ return "/dev/rbd/${pool}/${name}";
};
sub map_volume {
@@ -585,12 +616,13 @@ sub map_volume {
my ($vtype, $img_name, $vmid) = $class->parse_volname($volname);
- my $name = $img_name;
+ my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
+ my $name = "${img_name}";
$name .= '@'.$snapname if $snapname;
- my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
+ my $namespace = $scfg->{namespace};
- my $kerneldev = $get_kernel_device_name->($pool, $name);
+ my $kerneldev = $get_kernel_device_name->($pool, $name, $namespace);
return $kerneldev if -b $kerneldev; # already mapped
@@ -598,6 +630,7 @@ sub map_volume {
$krbd_feature_update->($scfg, $storeid, $img_name);
my $cmd = &$rbd_cmd($scfg, $storeid, 'map', $name);
+ push @$cmd, '--namespace', $namespace if $namespace;
run_rbd_command($cmd, errmsg => "can't map rbd volume $name");
return $kerneldev;
@@ -611,7 +644,7 @@ sub unmap_volume {
my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
- my $kerneldev = $get_kernel_device_name->($pool, $name);
+ my $kerneldev = $get_kernel_device_name->($pool, $name, $scfg->{namespace});
if (-b $kerneldev) {
my $cmd = &$rbd_cmd($scfg, $storeid, 'unmap', $kerneldev);
@@ -653,6 +686,7 @@ sub volume_resize {
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
my $cmd = &$rbd_cmd($scfg, $storeid, 'resize', '--allow-shrink', '--size', ($size/1024/1024), $name);
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
run_rbd_command($cmd, errmsg => "rbd resize '$volname' error");
return undef;
}
@@ -663,6 +697,7 @@ sub volume_snapshot {
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'create', '--snap', $snap, $name);
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
run_rbd_command($cmd, errmsg => "rbd snapshot '$volname' error");
return undef;
}
@@ -673,6 +708,7 @@ sub volume_snapshot_rollback {
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'rollback', '--snap', $snap, $name);
+ push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
run_rbd_command($cmd, errmsg => "rbd snapshot $volname to '$snap' error");
}
@@ -684,14 +720,17 @@ sub volume_snapshot_delete {
$class->deactivate_volume($storeid, $scfg, $volname, $snap, {});
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
+ my $namespace = $scfg->{namespace};
my (undef, undef, undef, $protected) = rbd_volume_info($scfg, $storeid, $name, $snap);
if ($protected){
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'unprotect', $name, '--snap', $snap);
+ push @$cmd, '--namespace', $namespace if $namespace;
run_rbd_command($cmd, errmsg => "rbd unprotect $name snap '$snap' error");
}
my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'rm', '--snap', $snap, $name);
+ push @$cmd, '--namespace', $namespace if $namespace;
run_rbd_command($cmd, errmsg => "rbd snapshot '$volname' error");
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH RFC storage] rbd: fix #3286 add namespace support
2021-03-01 15:12 [pve-devel] [PATCH RFC storage] rbd: fix #3286 add namespace support Aaron Lauterer
@ 2021-03-01 16:13 ` Thomas Lamprecht
2021-03-03 10:10 ` aderumier
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2021-03-01 16:13 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
On 01.03.21 16:12, Aaron Lauterer wrote:
> This RFC introduces support for Cephs RBD namespaces.
>
> A new storage config parameter 'namespace' defines the namespace to be
> used for the RBD storage.
>
> The namespace must already exist in the Ceph cluster as it is not
> automatically created.
>
> The main intention is to use this for external Ceph clusters. With
> namespaces, each PVE cluster can get its own namespace and will not
> conflict with other PVE clusters.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>
> There are two ways to address namespaces. One is the '--namespace'
> parameter which is not supported by all rbd subcommands though!
> The other is the path style '<pool>/<namespace>/<image>' way.
So if the latter is supported by all why not use only that one?
Could be done in a small get path helper handling krdb/librbd differences
and returning just the path?
Note, I did not looked into this that close, so it may be that I'm
overlooking something, but they way you state this here and the fact that
having a single way to do the same thing is normally easier/nicer made me
wonder..
>
> Where possible I inject the '--namespace' parameter to the @$cmd but
> sometimes it is necessary to use the path style. There might be a nicer
> way to inject the namespace in these cases than what I have now.
>
> It would be good to have some tests but this cannot be done at build
> time since it requires a ceph cluster with namespaces configured.
> Therefore having a test script that can be called manually in a fitting
> test environment is what I still have planned (thanks @Dominik for the
> hint).
>
> Should I just place that in the 'test' directory of the repo without
> adding it to the Makefile?
yes please
>
> PVE/Storage/RBDPlugin.pm | 57 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 48 insertions(+), 9 deletions(-)
>
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
> index fab6d57..8a6329f 100644
> --- a/PVE/Storage/RBDPlugin.pm
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -27,7 +27,9 @@ my $add_pool_to_disk = sub {
>
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
>
> - return "$pool/$disk";
> + my $namespace = $scfg->{namespace} ? "/$scfg->{namespace}" : "";
> +
> + return "$pool$namespace/$disk";
> };
>
> my $build_cmd = sub {
> @@ -77,6 +79,8 @@ my $librados_connect = sub {
> my $krbd_feature_update = sub {
> my ($scfg, $storeid, $name) = @_;
>
> + my $namespace = $scfg->{namespace};
> +
> my (@disable, @enable);
> my ($kmajor, $kminor) = PVE::ProcFSTools::kernel_version();
>
> @@ -102,6 +106,7 @@ my $krbd_feature_update = sub {
> if ($to_disable) {
> print "disable RBD image features this kernel RBD drivers is not compatible with: $to_disable\n";
> my $cmd = $rbd_cmd->($scfg, $storeid, 'feature', 'disable', $name, $to_disable);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command(
> $cmd,
> errmsg => "could not disable krbd-incompatible image features '$to_disable' for rbd image: $name",
> @@ -111,6 +116,7 @@ my $krbd_feature_update = sub {
> print "enable RBD image features this kernel RBD drivers supports: $to_enable\n";
> eval {
> my $cmd = $rbd_cmd->($scfg, $storeid, 'feature', 'enable', $name, $to_enable);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command(
> $cmd,
> errmsg => "could not enable krbd-compatible image features '$to_enable' for rbd image: $name",
> @@ -153,7 +159,10 @@ sub rbd_ls {
> my ($scfg, $storeid) = @_;
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'ls', '-l', '--format', 'json');
> + my $namespace = $scfg->{namespace};
> + push @$cmd, '--namespace', $namespace if $namespace;
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> + $pool .= "/${namespace}" if $namespace;
>
> my $raw = '';
> my $parser = sub { $raw .= shift };
> @@ -199,6 +208,7 @@ sub rbd_ls_snap {
> my ($scfg, $storeid, $name) = @_;
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'ls', $name, '--format', 'json');
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
>
> my $raw = '';
> run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => sub { $raw .= shift; });
> @@ -238,6 +248,7 @@ sub rbd_volume_info {
> }
>
> $cmd = &$rbd_cmd($scfg, $storeid, @options);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
>
> my $raw = '';
> my $parser = sub { $raw .= shift };
> @@ -281,6 +292,10 @@ sub properties {
> description => "Pool.",
> type => 'string',
> },
> + namespace=> {
> + description => "RBD Namespace.",
> + type => 'string',
> + },
> username => {
> description => "RBD Id.",
> type => 'string',
> @@ -302,6 +317,7 @@ sub options {
> disable => { optional => 1 },
> monhost => { optional => 1},
> pool => { optional => 1 },
> + namespace => { optional => 1 },
> username => { optional => 1 },
> content => { optional => 1 },
> krbd => { optional => 1 },
> @@ -349,9 +365,10 @@ sub path {
> $name .= '@'.$snapname if $snapname;
>
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> - return ("/dev/rbd/$pool/$name", $vmid, $vtype) if $scfg->{krbd};
> + my $namespace = $scfg->{namespace} ? "/$scfg->{namespace}" : "";
> + return ("/dev/rbd/${pool}${namespace}/${name}", $vmid, $vtype) if $scfg->{krbd};
>
> - my $path = "rbd:$pool/$name";
> + my $path = "rbd:${pool}${namespace}/${name}";
>
> $path .= ":conf=$cmd_option->{ceph_conf}" if $cmd_option->{ceph_conf};
> if (defined($scfg->{monhost})) {
> @@ -370,6 +387,8 @@ sub find_free_diskname {
> my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'ls');
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
> +
> my $disk_list = [];
>
> my $parser = sub {
> @@ -423,6 +442,7 @@ sub create_base {
>
> if (!$protected){
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'protect', $newname, '--snap', $snap);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
> run_rbd_command($cmd, errmsg => "rbd protect $newname snap '$snap' error");
> }
>
> @@ -451,6 +471,7 @@ sub clone_image {
>
> if (!$protected) {
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'protect', $volname, '--snap', $snapname);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
> run_rbd_command($cmd, errmsg => "rbd protect $volname snap $snapname error");
> }
> }
> @@ -476,6 +497,7 @@ sub alloc_image {
> $name = $class->find_free_diskname($storeid, $scfg, $vmid) if !$name;
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'create', '--image-format' , 2, '--size', int(($size+1023)/1024), $name);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
> run_rbd_command($cmd, errmsg => "rbd create $name' error");
>
> return $name;
> @@ -487,10 +509,13 @@ sub free_image {
> my ($vtype, $name, $vmid, undef, undef, undef) =
> $class->parse_volname($volname);
>
> + my $namespace = $scfg->{namespace};
> +
> my $snaps = rbd_ls_snap($scfg, $storeid, $name);
> foreach my $snap (keys %$snaps) {
> if ($snaps->{$snap}->{protected}) {
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'unprotect', $name, '--snap', $snap);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "rbd unprotect $name snap '$snap' error");
> }
> }
> @@ -498,9 +523,11 @@ sub free_image {
> $class->deactivate_volume($storeid, $scfg, $volname);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'purge', $name);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "rbd snap purge '$volname' error");
>
> $cmd = &$rbd_cmd($scfg, $storeid, 'rm', $name);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "rbd rm '$volname' error");
>
> return undef;
> @@ -511,6 +538,8 @@ sub list_images {
>
> $cache->{rbd} = rbd_ls($scfg, $storeid) if !$cache->{rbd};
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> + my $namespace = $scfg->{namespace};
> + $pool .= "/${namespace}" if $namespace;
>
> my $res = [];
>
> @@ -575,9 +604,11 @@ sub deactivate_storage {
> }
>
> my $get_kernel_device_name = sub {
> - my ($pool, $name) = @_;
> + my ($pool, $name, $namespace) = @_;
> +
> + return "/dev/rbd/${pool}/${namespace}/${name}" if $namespace;
>
> - return "/dev/rbd/$pool/$name";
> + return "/dev/rbd/${pool}/${name}";
> };
>
> sub map_volume {
> @@ -585,12 +616,13 @@ sub map_volume {
>
> my ($vtype, $img_name, $vmid) = $class->parse_volname($volname);
>
> - my $name = $img_name;
> + my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> + my $name = "${img_name}";
> $name .= '@'.$snapname if $snapname;
>
> - my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> + my $namespace = $scfg->{namespace};
>
> - my $kerneldev = $get_kernel_device_name->($pool, $name);
> + my $kerneldev = $get_kernel_device_name->($pool, $name, $namespace);
>
> return $kerneldev if -b $kerneldev; # already mapped
>
> @@ -598,6 +630,7 @@ sub map_volume {
> $krbd_feature_update->($scfg, $storeid, $img_name);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'map', $name);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "can't map rbd volume $name");
>
> return $kerneldev;
> @@ -611,7 +644,7 @@ sub unmap_volume {
>
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
>
> - my $kerneldev = $get_kernel_device_name->($pool, $name);
> + my $kerneldev = $get_kernel_device_name->($pool, $name, $scfg->{namespace});
>
> if (-b $kerneldev) {
> my $cmd = &$rbd_cmd($scfg, $storeid, 'unmap', $kerneldev);
> @@ -653,6 +686,7 @@ sub volume_resize {
> my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'resize', '--allow-shrink', '--size', ($size/1024/1024), $name);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
> run_rbd_command($cmd, errmsg => "rbd resize '$volname' error");
> return undef;
> }
> @@ -663,6 +697,7 @@ sub volume_snapshot {
> my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'create', '--snap', $snap, $name);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
> run_rbd_command($cmd, errmsg => "rbd snapshot '$volname' error");
> return undef;
> }
> @@ -673,6 +708,7 @@ sub volume_snapshot_rollback {
> my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'rollback', '--snap', $snap, $name);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg->{namespace};
> run_rbd_command($cmd, errmsg => "rbd snapshot $volname to '$snap' error");
> }
>
> @@ -684,14 +720,17 @@ sub volume_snapshot_delete {
> $class->deactivate_volume($storeid, $scfg, $volname, $snap, {});
>
> my ($vtype, $name, $vmid) = $class->parse_volname($volname);
> + my $namespace = $scfg->{namespace};
>
> my (undef, undef, undef, $protected) = rbd_volume_info($scfg, $storeid, $name, $snap);
> if ($protected){
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'unprotect', $name, '--snap', $snap);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "rbd unprotect $name snap '$snap' error");
> }
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'rm', '--snap', $snap, $name);
> + push @$cmd, '--namespace', $namespace if $namespace;
>
> run_rbd_command($cmd, errmsg => "rbd snapshot '$volname' error");
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH RFC storage] rbd: fix #3286 add namespace support
2021-03-01 15:12 [pve-devel] [PATCH RFC storage] rbd: fix #3286 add namespace support Aaron Lauterer
2021-03-01 16:13 ` Thomas Lamprecht
@ 2021-03-03 10:10 ` aderumier
2021-03-03 12:17 ` Fabian Grünbichler
1 sibling, 1 reply; 4+ messages in thread
From: aderumier @ 2021-03-03 10:10 UTC (permalink / raw)
To: Proxmox VE development discussion
Hi,
I wasn't aware about namespace support in ceph rbd, that's great :)
Is they any plan on the roadmap to generalize namespace, but at vm
level ?
I'm still looking for easy cross-cluster vm migration with shared
storage.
I was thinking about something simple like
/etc/pve/<node>/qemu-server/<namespace>/<vmid.conf>
with new disk volumes including the namespace in their path like:
"scsi0: <storage>:<namespace>/vm-100-disk-0"
Le lundi 01 mars 2021 à 16:12 +0100, Aaron Lauterer a écrit :
> This RFC introduces support for Cephs RBD namespaces.
>
> A new storage config parameter 'namespace' defines the namespace to
> be
> used for the RBD storage.
>
> The namespace must already exist in the Ceph cluster as it is not
> automatically created.
>
> The main intention is to use this for external Ceph clusters. With
> namespaces, each PVE cluster can get its own namespace and will not
> conflict with other PVE clusters.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>
> There are two ways to address namespaces. One is the '--namespace'
> parameter which is not supported by all rbd subcommands though!
> The other is the path style '<pool>/<namespace>/<image>' way.
>
> Where possible I inject the '--namespace' parameter to the @$cmd but
> sometimes it is necessary to use the path style. There might be a
> nicer
> way to inject the namespace in these cases than what I have now.
>
> It would be good to have some tests but this cannot be done at build
> time since it requires a ceph cluster with namespaces configured.
> Therefore having a test script that can be called manually in a
> fitting
> test environment is what I still have planned (thanks @Dominik for
> the
> hint).
>
> Should I just place that in the 'test' directory of the repo without
> adding it to the Makefile?
>
> PVE/Storage/RBDPlugin.pm | 57 +++++++++++++++++++++++++++++++++-----
> --
> 1 file changed, 48 insertions(+), 9 deletions(-)
>
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
> index fab6d57..8a6329f 100644
> --- a/PVE/Storage/RBDPlugin.pm
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -27,7 +27,9 @@ my $add_pool_to_disk = sub {
>
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
>
> - return "$pool/$disk";
> + my $namespace = $scfg->{namespace} ? "/$scfg->{namespace}" : "";
> +
> + return "$pool$namespace/$disk";
> };
>
> my $build_cmd = sub {
> @@ -77,6 +79,8 @@ my $librados_connect = sub {
> my $krbd_feature_update = sub {
> my ($scfg, $storeid, $name) = @_;
>
> + my $namespace = $scfg->{namespace};
> +
> my (@disable, @enable);
> my ($kmajor, $kminor) = PVE::ProcFSTools::kernel_version();
>
> @@ -102,6 +106,7 @@ my $krbd_feature_update = sub {
> if ($to_disable) {
> print "disable RBD image features this kernel RBD drivers is
> not compatible with: $to_disable\n";
> my $cmd = $rbd_cmd->($scfg, $storeid, 'feature', 'disable',
> $name, $to_disable);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command(
> $cmd,
> errmsg => "could not disable krbd-incompatible image
> features '$to_disable' for rbd image: $name",
> @@ -111,6 +116,7 @@ my $krbd_feature_update = sub {
> print "enable RBD image features this kernel RBD drivers
> supports: $to_enable\n";
> eval {
> my $cmd = $rbd_cmd->($scfg, $storeid, 'feature',
> 'enable', $name, $to_enable);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command(
> $cmd,
> errmsg => "could not enable krbd-compatible image
> features '$to_enable' for rbd image: $name",
> @@ -153,7 +159,10 @@ sub rbd_ls {
> my ($scfg, $storeid) = @_;
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'ls', '-l', '--format',
> 'json');
> + my $namespace = $scfg->{namespace};
> + push @$cmd, '--namespace', $namespace if $namespace;
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> + $pool .= "/${namespace}" if $namespace;
>
> my $raw = '';
> my $parser = sub { $raw .= shift };
> @@ -199,6 +208,7 @@ sub rbd_ls_snap {
> my ($scfg, $storeid, $name) = @_;
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'ls', $name, '--
> format', 'json');
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
>
> my $raw = '';
> run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {},
> outfunc => sub { $raw .= shift; });
> @@ -238,6 +248,7 @@ sub rbd_volume_info {
> }
>
> $cmd = &$rbd_cmd($scfg, $storeid, @options);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
>
> my $raw = '';
> my $parser = sub { $raw .= shift };
> @@ -281,6 +292,10 @@ sub properties {
> description => "Pool.",
> type => 'string',
> },
> + namespace=> {
> + description => "RBD Namespace.",
> + type => 'string',
> + },
> username => {
> description => "RBD Id.",
> type => 'string',
> @@ -302,6 +317,7 @@ sub options {
> disable => { optional => 1 },
> monhost => { optional => 1},
> pool => { optional => 1 },
> + namespace => { optional => 1 },
> username => { optional => 1 },
> content => { optional => 1 },
> krbd => { optional => 1 },
> @@ -349,9 +365,10 @@ sub path {
> $name .= '@'.$snapname if $snapname;
>
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> - return ("/dev/rbd/$pool/$name", $vmid, $vtype) if $scfg->{krbd};
> + my $namespace = $scfg->{namespace} ? "/$scfg->{namespace}" : "";
> + return ("/dev/rbd/${pool}${namespace}/${name}", $vmid, $vtype)
> if $scfg->{krbd};
>
> - my $path = "rbd:$pool/$name";
> + my $path = "rbd:${pool}${namespace}/${name}";
>
> $path .= ":conf=$cmd_option->{ceph_conf}" if $cmd_option-
> >{ceph_conf};
> if (defined($scfg->{monhost})) {
> @@ -370,6 +387,8 @@ sub find_free_diskname {
> my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'ls');
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
> +
> my $disk_list = [];
>
> my $parser = sub {
> @@ -423,6 +442,7 @@ sub create_base {
>
> if (!$protected){
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'protect',
> $newname, '--snap', $snap);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
> run_rbd_command($cmd, errmsg => "rbd protect $newname snap
> '$snap' error");
> }
>
> @@ -451,6 +471,7 @@ sub clone_image {
>
> if (!$protected) {
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'protect',
> $volname, '--snap', $snapname);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
> run_rbd_command($cmd, errmsg => "rbd protect $volname
> snap $snapname error");
> }
> }
> @@ -476,6 +497,7 @@ sub alloc_image {
> $name = $class->find_free_diskname($storeid, $scfg, $vmid) if
> !$name;
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'create', '--image-format'
> , 2, '--size', int(($size+1023)/1024), $name);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
> run_rbd_command($cmd, errmsg => "rbd create $name' error");
>
> return $name;
> @@ -487,10 +509,13 @@ sub free_image {
> my ($vtype, $name, $vmid, undef, undef, undef) =
> $class->parse_volname($volname);
>
> + my $namespace = $scfg->{namespace};
> +
> my $snaps = rbd_ls_snap($scfg, $storeid, $name);
> foreach my $snap (keys %$snaps) {
> if ($snaps->{$snap}->{protected}) {
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'unprotect',
> $name, '--snap', $snap);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "rbd unprotect $name snap
> '$snap' error");
> }
> }
> @@ -498,9 +523,11 @@ sub free_image {
> $class->deactivate_volume($storeid, $scfg, $volname);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'purge', $name);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "rbd snap purge '$volname'
> error");
>
> $cmd = &$rbd_cmd($scfg, $storeid, 'rm', $name);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "rbd rm '$volname' error");
>
> return undef;
> @@ -511,6 +538,8 @@ sub list_images {
>
> $cache->{rbd} = rbd_ls($scfg, $storeid) if !$cache->{rbd};
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> + my $namespace = $scfg->{namespace};
> + $pool .= "/${namespace}" if $namespace;
>
> my $res = [];
>
> @@ -575,9 +604,11 @@ sub deactivate_storage {
> }
>
> my $get_kernel_device_name = sub {
> - my ($pool, $name) = @_;
> + my ($pool, $name, $namespace) = @_;
> +
> + return "/dev/rbd/${pool}/${namespace}/${name}" if $namespace;
>
> - return "/dev/rbd/$pool/$name";
> + return "/dev/rbd/${pool}/${name}";
> };
>
> sub map_volume {
> @@ -585,12 +616,13 @@ sub map_volume {
>
> my ($vtype, $img_name, $vmid) = $class->parse_volname($volname);
>
> - my $name = $img_name;
> + my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> + my $name = "${img_name}";
> $name .= '@'.$snapname if $snapname;
>
> - my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
> + my $namespace = $scfg->{namespace};
>
> - my $kerneldev = $get_kernel_device_name->($pool, $name);
> + my $kerneldev = $get_kernel_device_name->($pool, $name,
> $namespace);
>
> return $kerneldev if -b $kerneldev; # already mapped
>
> @@ -598,6 +630,7 @@ sub map_volume {
> $krbd_feature_update->($scfg, $storeid, $img_name);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'map', $name);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "can't map rbd volume $name");
>
> return $kerneldev;
> @@ -611,7 +644,7 @@ sub unmap_volume {
>
> my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
>
> - my $kerneldev = $get_kernel_device_name->($pool, $name);
> + my $kerneldev = $get_kernel_device_name->($pool, $name, $scfg-
> >{namespace});
>
> if (-b $kerneldev) {
> my $cmd = &$rbd_cmd($scfg, $storeid, 'unmap', $kerneldev);
> @@ -653,6 +686,7 @@ sub volume_resize {
> my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'resize', '--allow-shrink',
> '--size', ($size/1024/1024), $name);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
> run_rbd_command($cmd, errmsg => "rbd resize '$volname' error");
> return undef;
> }
> @@ -663,6 +697,7 @@ sub volume_snapshot {
> my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'create', '--snap',
> $snap, $name);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
> run_rbd_command($cmd, errmsg => "rbd snapshot '$volname'
> error");
> return undef;
> }
> @@ -673,6 +708,7 @@ sub volume_snapshot_rollback {
> my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'rollback', '--
> snap', $snap, $name);
> + push @$cmd, '--namespace', $scfg->{namespace} if $scfg-
> >{namespace};
> run_rbd_command($cmd, errmsg => "rbd snapshot $volname to
> '$snap' error");
> }
>
> @@ -684,14 +720,17 @@ sub volume_snapshot_delete {
> $class->deactivate_volume($storeid, $scfg, $volname, $snap, {});
>
> my ($vtype, $name, $vmid) = $class->parse_volname($volname);
> + my $namespace = $scfg->{namespace};
>
> my (undef, undef, undef, $protected) = rbd_volume_info($scfg,
> $storeid, $name, $snap);
> if ($protected){
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'unprotect',
> $name, '--snap', $snap);
> + push @$cmd, '--namespace', $namespace if $namespace;
> run_rbd_command($cmd, errmsg => "rbd unprotect $name snap
> '$snap' error");
> }
>
> my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'rm', '--snap',
> $snap, $name);
> + push @$cmd, '--namespace', $namespace if $namespace;
>
> run_rbd_command($cmd, errmsg => "rbd snapshot '$volname'
> error");
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH RFC storage] rbd: fix #3286 add namespace support
2021-03-03 10:10 ` aderumier
@ 2021-03-03 12:17 ` Fabian Grünbichler
0 siblings, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2021-03-03 12:17 UTC (permalink / raw)
To: Proxmox VE development discussion
On March 3, 2021 11:10 am, aderumier@odiso.com wrote:
> Is they any plan on the roadmap to generalize namespace, but at vm
> level ?
>
> I'm still looking for easy cross-cluster vm migration with shared
> storage.
I recently picked up the remote migration feature, FWIW ;)
>
> I was thinking about something simple like
> /etc/pve/<node>/qemu-server/<namespace>/<vmid.conf>
> with new disk volumes including the namespace in their path like:
> "scsi0: <storage>:<namespace>/vm-100-disk-0"
I am not sure how that would solve the issue? the problem with sharing a
shared storage between clusters is that VMID 100 on cluster A and VMID
100 on cluster B are not the same entity, so a volume owned by VMID 100
is not attributable to either cluster.
if both clusters are allowed to setup a namespace FOO, then you need to
manually take care not to duplicate VMIDs inside this namespace across
all clusters, just like you have to take care to not duplicate VMIDs
across all clusters right now?
if only one cluster is allowed to use a certain namespace, then shared
migration needs to do a rename (or rather, move the VM and volumes
from one namespace to another). that would mean no live-migration, since
a live-rename of a volume is not possible, unless the namespace is not
actually encoded in the volume name on the storage. if the namespace
is not actually encoded in the volume name, it does not protect against
cross-namespace confusion (since when listing a storage's contents, I
can't tell which namespace volume BAR belongs to), and we'd be back to
square one.
IMHO there are things that might help with the issue:
- a client used to manage all clusters that ensures a VMID is not
assigned to more than one cluster
- better support for custom volids (reduce chance of clashes, does not
solve issue with orphaned/unreferenced volumes)
- allow marking a storage as "don't scan for unreferenced volumes", so
that stray volumes likely belonging to other clusters are not picked
up when migrating/deleting/.. guests (setting this would also need to
disallow deleting any volumes via the storage API instead of the guest
API, as we don't have any safeguards on the storage level then..)
the first point is hard to do atomically, since we don't have a
cross-cluster pmxcfs, but some sort of "assign ranges to clusters,
remember exceptions for VMs which have been migrated away" could work,
if ALL management then happens using this client and not the regular
per-cluster API. this could also be supported in PVE right now
(configure range in datacenter.cfg, have some API call to register "this
VMID is burnt/does not belong to this cluster anymore, ignore it for all
intents and purposes) - although obviously this would not yet guarantuee
no re-use across clusters, but just enable integration/management tools
to have some support on the PVE side for enforcing those ranges.
just some quick thoughts, might not be 100% thought-through in all
directions :-P
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-03 12:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 15:12 [pve-devel] [PATCH RFC storage] rbd: fix #3286 add namespace support Aaron Lauterer
2021-03-01 16:13 ` Thomas Lamprecht
2021-03-03 10:10 ` aderumier
2021-03-03 12:17 ` Fabian Grünbichler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox