From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 82F9F69217 for ; Mon, 1 Mar 2021 17:13:52 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7511922FC8 for ; Mon, 1 Mar 2021 17:13:22 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id A9C3C22FB8 for ; Mon, 1 Mar 2021 17:13:20 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 684344581B for ; Mon, 1 Mar 2021 17:13:20 +0100 (CET) Message-ID: <3795ea42-81ce-64bc-5fb9-15c474082b38@proxmox.com> Date: Mon, 1 Mar 2021 17:13:18 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:87.0) Gecko/20100101 Thunderbird/87.0 Content-Language: en-US To: Proxmox VE development discussion , Aaron Lauterer References: <20210301151226.9631-1-a.lauterer@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20210301151226.9631-1-a.lauterer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.053 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH RFC storage] rbd: fix #3286 add namespace support X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Mar 2021 16:13:52 -0000 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 > --- > > 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 '//' 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"); > >