this is the recommended way upstream to pass this information, and all commands
(other than those not operating on images/snapshots) consume them (rbd(8)):
> image-spec is [pool-name/[namespace-name/]]image-name
> snap-spec is [pool-name/[namespace-name/]]image-name@snap-name
> [..]
> You may specify each name individually, using --pool, --namespace, --image,
> and --snap options, but this is discouraged in favor of the above spec
> syntax.
We already had a few places calling `get_rbd_path` previously (which meant
passing the pool and namespace information twice to Ceph).
It also allows us to replace the custom import and unmap handling with just
custom `ls` handling, reducing the workarounds in rbd_cmd.
Fixes: #6338 , because `rbd import` doesn't handle a separate `--namespace ..`
at all..
Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
---
we could probably switch some more only-technically-public subs over to
take a spec instead of image name or image + snapshot name, if desired..
alternatively, we could also just special case the import call, and/or convince
upstream to implement --namespace handling there as well or send a patch doing
that their way..
src/PVE/Storage/RBDPlugin.pm | 133 ++++++++++++++++++-----------------
1 file changed, 69 insertions(+), 64 deletions(-)
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 8c67a37..3bb5807 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -89,22 +89,17 @@ my $rbd_cmd = sub {
my ($scfg, $storeid, $op, @options) = @_;
my $cmd_option = PVE::CephConfig::ceph_connect_option($scfg, $storeid);
- my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
my $cmd = ['/usr/bin/rbd'];
- if ($op eq 'import') {
- push $cmd->@*, '--dest-pool', $pool;
- } else {
+ # `ls` doesn't take an image-spec, otherwise pool and namespace should be
specified there
+ if ($op eq 'ls') {
+ my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
push $cmd->@*, '-p', $pool;
+ if (defined($scfg->{namespace})) {
+ push @$cmd, '--namespace', $cfg->{namespace};
+ }
}
- if (defined(my $namespace = $scfg->{namespace})) {
- # some subcommands will fail if the --namespace parameter is present
- my $no_namespace_parameter = {
- unmap => 1,
- };
- push @$cmd, '--namespace', "$namespace" if
!$no_namespace_parameter->{$op};
- }
push @$cmd, '-c', $cmd_option->{ceph_conf} if ($cmd_option->{ceph_conf});
push @$cmd, '-m', $cmd_option->{mon_host} if ($cmd_option->{mon_host});
push @$cmd, '--auth_supported', $cmd_option->{auth_supported} if
($cmd_option->{auth_supported});
@@ -144,9 +139,11 @@ my $krbd_feature_update = sub {
my $to_disable = join(',', grep { $active_features->{$_} } @disable);
my $to_enable = join(',', grep { !$active_features->{$_} } @enable );
+ my $image_spec = get_rbd_path($scfg, $name);
+
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);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'feature', 'disable',
$image_spec, $to_disable);
run_rbd_command(
$cmd,
errmsg => "could not disable krbd-incompatible image features
'$to_disable' for rbd image: $name",
@@ -155,7 +152,7 @@ my $krbd_feature_update = sub {
if ($to_enable) {
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);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'feature', 'enable',
$image_spec, $to_enable);
run_rbd_command(
$cmd,
errmsg => "could not enable krbd-compatible image features
'$to_enable' for rbd image: $name",
@@ -234,9 +231,9 @@ sub rbd_ls {
}
sub rbd_ls_snap {
- my ($scfg, $storeid, $name) = @_;
+ my ($scfg, $storeid, $image_spec) = @_;
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'ls', $name, '--format',
'json');
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'ls', $image_spec,
'--format', 'json');
my $raw = '';
run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc =>
sub { $raw .= shift; });
@@ -244,9 +241,9 @@ sub rbd_ls_snap {
my $list;
if ($raw =~ m/^(\[.*\])$/s) { # untaint
$list = eval { JSON::decode_json($1) };
- die "invalid JSON output from 'rbd snap ls $name': $@\n" if $@;
+ die "invalid JSON output from 'rbd snap ls $image_spec': $@\n" if $@;
} else {
- die "got unexpected data from 'rbd snap ls $name': '$raw'\n";
+ die "got unexpected data from 'rbd snap ls $image_spec': '$raw'\n";
}
$list = [] if !defined($list);
@@ -269,11 +266,9 @@ sub rbd_volume_info {
my ($scfg, $storeid, $volname, $snap) = @_;
my $cmd = undef;
+ my $spec = get_rbd_path($scfg, $volname, $snap);
- my @options = ('info', $volname, '--format', 'json');
- if ($snap) {
- push @options, '--snap', $snap;
- }
+ my @options = ('info', $spec, '--format', 'json');
$cmd = $rbd_cmd->($scfg, $storeid, @options);
@@ -300,7 +295,8 @@ sub rbd_volume_info {
sub rbd_volume_du {
my ($scfg, $storeid, $volname) = @_;
- my @options = ('du', $volname, '--format', 'json');
+ my $spec = get_rbd_path($scfg, $volname);
+ my @options = ('du', $spec, '--format', 'json');
my $cmd = $rbd_cmd->($scfg, $storeid, @options);
my $raw = '';
@@ -471,14 +467,13 @@ sub path {
my $cmd_option = PVE::CephConfig::ceph_connect_option($scfg, $storeid);
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
- $name .= '@'.$snapname if $snapname;
if ($scfg->{krbd}) {
- my $rbd_dev_path = get_rbd_dev_path($scfg, $storeid, $name);
+ my $rbd_dev_path = get_rbd_dev_path($scfg, $storeid, $name, $snapname);
return ($rbd_dev_path, $vmid, $vtype);
}
- my $rbd_path = get_rbd_path($scfg, $name);
+ my $rbd_path = get_rbd_path($scfg, $name, $snapname);
my $path = "rbd:${rbd_path}";
$path .= ":conf=$cmd_option->{ceph_conf}" if $cmd_option->{ceph_conf};
@@ -560,8 +555,9 @@ sub create_base {
my (undef, undef, undef, $protected) = rbd_volume_info($scfg, $storeid,
$newname, $snap);
if (!$protected){
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'protect', $newname,
'--snap', $snap);
- run_rbd_command($cmd, errmsg => "rbd protect $newname snap '$snap'
error");
+ my $snap_spec = get_rbd_path($scfg, $newname, $snap);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'protect', $snap_spec);
+ run_rbd_command($cmd, errmsg => "rbd protect '$snap_spec' error");
}
return $newvolname;
@@ -588,8 +584,9 @@ sub clone_image {
my (undef, undef, undef, $protected) = rbd_volume_info($scfg, $storeid,
$volname, $snapname);
if (!$protected) {
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'protect', $volname,
'--snap', $snapname);
- run_rbd_command($cmd, errmsg => "rbd protect $volname snap
$snapname error");
+ my $snap_spec = get_rbd_path($scfg, $volname, $snapname);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'protect',
$snap_spec);
+ run_rbd_command($cmd, errmsg => "rbd protect '$snap_spec' error");
}
}
@@ -597,8 +594,7 @@ sub clone_image {
$newvol = $name if length($snapname);
my @options = (
- get_rbd_path($scfg, $basename),
- '--snap', $snap,
+ get_rbd_path($scfg, $basename, $snap),
);
push @options, ('--data-pool', $scfg->{'data-pool'}) if
$scfg->{'data-pool'};
@@ -623,7 +619,7 @@ sub alloc_image {
);
push @options, ('--data-pool', $scfg->{'data-pool'}) if
$scfg->{'data-pool'};
- my $cmd = $rbd_cmd->($scfg, $storeid, 'create', @options, $name);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'create', @options,
get_rbd_path($scfg, $name));
run_rbd_command($cmd, errmsg => "rbd create '$name' error");
return $name;
@@ -635,22 +631,23 @@ sub free_image {
my ($vtype, $name, $vmid, undef, undef, undef) =
$class->parse_volname($volname);
-
- my $snaps = rbd_ls_snap($scfg, $storeid, $name);
+ my $image_spec = get_rbd_path($scfg, $name);
+ my $snaps = rbd_ls_snap($scfg, $storeid, $image_spec);
foreach my $snap (keys %$snaps) {
if ($snaps->{$snap}->{protected}) {
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'unprotect', $name,
'--snap', $snap);
- run_rbd_command($cmd, errmsg => "rbd unprotect $name snap '$snap'
error");
+ my $snap_spec = get_rbd_path($scfg, $name, $snap);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'unprotect',
$snap_spec);
+ run_rbd_command($cmd, errmsg => "rbd unprotect $snap_spec error");
}
}
$class->deactivate_volume($storeid, $scfg, $volname);
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'purge', $name);
- run_rbd_command($cmd, errmsg => "rbd snap purge '$name' error");
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'purge', $image_spec);
+ run_rbd_command($cmd, errmsg => "rbd snap purge '$image_spec' error");
- $cmd = $rbd_cmd->($scfg, $storeid, 'rm', $name);
- run_rbd_command($cmd, errmsg => "rbd rm '$name' error");
+ $cmd = $rbd_cmd->($scfg, $storeid, 'rm', $image_spec);
+ run_rbd_command($cmd, errmsg => "rbd rm '$image_spec' error");
return undef;
}
@@ -725,20 +722,18 @@ sub deactivate_storage {
sub map_volume {
my ($class, $storeid, $scfg, $volname, $snapname) = @_;
- my ($vtype, $img_name, $vmid) = $class->parse_volname($volname);
+ my ($vtype, $name, $vmid) = $class->parse_volname($volname);
- my $name = $img_name;
- $name .= '@'.$snapname if $snapname;
-
- my $kerneldev = get_rbd_dev_path($scfg, $storeid, $name);
+ my $kerneldev = get_rbd_dev_path($scfg, $storeid, $name, $snapname);
return $kerneldev if -b $kerneldev; # already mapped
# features can only be enabled/disabled for image, not for snapshot!
- $krbd_feature_update->($scfg, $storeid, $img_name);
+ $krbd_feature_update->($scfg, $storeid, $name);
- my $cmd = $rbd_cmd->($scfg, $storeid, 'map', $name);
- run_rbd_command($cmd, errmsg => "can't map rbd volume $name");
+ my $spec = get_rbd_path($scfg, $name, $snapname);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'map', $spec);
+ run_rbd_command($cmd, errmsg => "can't map rbd volume '$spec'");
return $kerneldev;
}
@@ -747,9 +742,8 @@ sub unmap_volume {
my ($class, $storeid, $scfg, $volname, $snapname) = @_;
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
- $name .= '@'.$snapname if $snapname;
- my $kerneldev = get_rbd_dev_path($scfg, $storeid, $name);
+ my $kerneldev = get_rbd_dev_path($scfg, $storeid, $name, $snapname);
if (-b $kerneldev) {
my $cmd = $rbd_cmd->($scfg, $storeid, 'unmap', $kerneldev);
@@ -791,8 +785,10 @@ sub volume_resize {
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
- my $cmd = $rbd_cmd->($scfg, $storeid, 'resize', '--size',
int(ceil($size/1024/1024)), $name);
- run_rbd_command($cmd, errmsg => "rbd resize '$volname' error");
+ my $image_spec = get_rbd_path($scfg, $volname);
+ $size = int(ceil($size/1024/1024));
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'resize', '--size', $size,
$image_spec);
+ run_rbd_command($cmd, errmsg => "rbd resize '$image_spec' error");
return undef;
}
@@ -801,8 +797,9 @@ sub volume_snapshot {
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'create', '--snap', $snap,
$name);
- run_rbd_command($cmd, errmsg => "rbd snapshot '$volname' error");
+ my $snap_spec = get_rbd_path($scfg, $name, $snap);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'create', $snap_spec);
+ run_rbd_command($cmd, errmsg => "rbd snapshot '$snap_spec' error");
return undef;
}
@@ -811,8 +808,9 @@ sub volume_snapshot_rollback {
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'rollback', '--snap', $snap,
$name);
- run_rbd_command($cmd, errmsg => "rbd snapshot $volname to '$snap' error");
+ my $snap_spec = get_rbd_path($scfg, $name, $snap);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'rollback', $snap_spec);
+ run_rbd_command($cmd, errmsg => "rbd snapshot rollback to '$snap_spec'
error");
}
sub volume_snapshot_delete {
@@ -823,14 +821,16 @@ sub volume_snapshot_delete {
my ($vtype, $name, $vmid) = $class->parse_volname($volname);
my (undef, undef, undef, $protected) = rbd_volume_info($scfg, $storeid,
$name, $snap);
+ my $snap_spec = get_rbd_path($scfg, $volname, $snap);
+
if ($protected){
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'unprotect', $name,
'--snap', $snap);
- run_rbd_command($cmd, errmsg => "rbd unprotect $name snap '$snap'
error");
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'unprotect', $snap_spec);
+ run_rbd_command($cmd, errmsg => "rbd unprotect '$snap_spec' error");
}
- my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'rm', '--snap', $snap,
$name);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'rm', $snap_spec);
- run_rbd_command($cmd, errmsg => "rbd snapshot '$volname' error");
+ run_rbd_command($cmd, errmsg => "rbd snapshot delete '$snap_spec' error");
return undef;
}
@@ -890,7 +890,9 @@ sub volume_export {
my ($size) = $class->volume_size_info($scfg, $storeid, $volname);
PVE::Storage::Plugin::write_common_header($fh, $size);
- my $cmd = $rbd_cmd->($scfg, $storeid, 'export', '--export-format', '1',
$volname, '-');
+
+ my $snap_spec = get_rbd_path($scfg, $volname, $snapshot);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'export', '--export-format', '1',
$snap_spec, '-');
run_rbd_command(
$cmd,
errmsg => 'could not export image',
@@ -938,8 +940,9 @@ sub volume_import {
my ($size) = PVE::Storage::Plugin::read_common_header($fh);
$size = PVE::Storage::Common::align_size_up($size, 1024) / 1024;
+ my $image_spec = get_rbd_path($scfg, $volname);
eval {
- my $cmd = $rbd_cmd->($scfg, $storeid, 'import', '--export-format', '1',
'-', $volname);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'import', '--export-format', '1',
'-', $image_spec);
run_rbd_command(
$cmd,
errmsg => 'could not import image',
@@ -976,11 +979,13 @@ sub rename_volume {
die "target volume '${target_volname}' already exists\n"
if rbd_volume_exists($scfg, $storeid, $target_volname);
- my $cmd = $rbd_cmd->($scfg, $storeid, 'rename', $source_image,
$target_volname);
+ my $source_spec = get_rbd_path($scfg, $source_volname);
+ my $target_spec = get_rbd_path($scfg, $target_volname);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'rename', $source_spec,
$target_spec);
run_rbd_command(
$cmd,
- errmsg => "could not rename image '${source_image}' to
'${target_volname}'",
+ errmsg => "could not rename image '${source_spec}' to '${target_spec}'",
);
eval { $class->unmap_volume($storeid, $scfg, $source_volname); };
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
