Am 19.07.21 um 16:52 schrieb Aaron Lauterer:
Functionality has been added for the following storage types:
* directory ones, based on the default implementation:
* directory
* NFS
* CIFS
* gluster
* ZFS
* (thin) LVM
* Ceph
A new feature `rename` has been introduced to mark which storage
plugin supports the feature.
Version API and AGE have been bumped.
The storage gets locked and each plugin checks if the target volume
already exists prior renaming.
This is done because there could be a race condition from the time the
external caller requests a new free disk name to the time the volume is
actually renamed.
Signed-off-by: Aaron Lauterer <a.laute...@proxmox.com>
---
rfc -> v1:
* reduced number of parameters to minimum needed, plugins infer needed
information themselves
* added storage locking and checking if volume already exists
* parse target_volname prior to renaming to check if valid
old dedicated API endpoint -> rfc:
only do rename now but the rename function handles templates and returns
the new volid as this can be differently handled on some storages.
PVE/Storage.pm | 20 +++++++++++--
PVE/Storage/LVMPlugin.pm | 27 +++++++++++++++++
PVE/Storage/LvmThinPlugin.pm | 1 +
PVE/Storage/Plugin.pm | 58 ++++++++++++++++++++++++++++++++++++
PVE/Storage/RBDPlugin.pm | 29 ++++++++++++++++++
PVE/Storage/ZFSPoolPlugin.pm | 24 +++++++++++++++
6 files changed, 157 insertions(+), 2 deletions(-)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index afeb2e3..f6d86e1 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -41,11 +41,11 @@ use PVE::Storage::PBSPlugin;
use PVE::Storage::BTRFSPlugin;
# Storage API version. Increment it on changes in storage API interface.
-use constant APIVER => 9;
+use constant APIVER => 10;
# Age is the number of versions we're backward compatible with.
# This is like having 'current=APIVER' and age='APIAGE' in libtool,
# see
https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
-use constant APIAGE => 0;
+use constant APIAGE => 1;
# load standard plugins
PVE::Storage::DirPlugin->register();
@@ -360,6 +360,7 @@ sub volume_snapshot_needs_fsfreeze {
# snapshot - taking a snapshot is possible
# sparseinit - volume is sparsely initialized
# template - conversion to base image is possible
+# rename - renaming volumes is possible
# $snap - check if the feature is supported for a given snapshot
# $running - if the guest owning the volume is running
# $opts - hash with further options:
@@ -1868,6 +1869,21 @@ sub complete_volume {
return $res;
}
+sub rename_volume {
+ my ($cfg, $source_volid, $target_volname) = @_;
+
+ my ($storeid) = parse_volume_id($source_volid);
+
+ activate_storage($cfg, $storeid);
+
+ my $scfg = storage_config($cfg, $storeid);
+ my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+ return $plugin->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub
{
+ return $plugin->rename_volume($scfg, $storeid, $source_volid,
$target_volname);
I know I suggested these parameters last time, but I hadn't worked it
out in detail, and using $source_volname instead of $source_volid seems
to be possible and nicer (would avoid some calls in the plugins AFAICS).
+ });
+}
+
# Various io-heavy operations require io/bandwidth limits which can be
# configured on multiple levels: The global defaults in datacenter.cfg, and
# per-storage overrides. When we want to do a restore from storage A to
storage
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index 3e5b6c8..7a13a96 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -344,6 +344,16 @@ sub lvcreate {
run_command($cmd, errmsg => "lvcreate '$vg/$name' error");
}
+sub lvrename {
+ my ($vg, $oldname, $newname) = @_;
+
+ my $cmd = ['/sbin/lvrename', $vg, $oldname, $newname];
$cmd is not used
+ run_command(
+ ['/sbin/lvrename', $vg, $oldname, $newname],
+ errmsg => "lvrename '${vg}/${oldname}' to '${newname}' error"
+ );
+}
+
sub alloc_image {
my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
@@ -589,6 +599,7 @@ sub volume_has_feature {
my $features = {
copy => { base => 1, current => 1},
+ rename => {current => 1},
};
my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
@@ -697,4 +708,20 @@ sub volume_import_write {
input => '<&'.fileno($input_fh));
}
+sub rename_volume {
+ my ($class, $scfg, $storeid, $source_volid, $target_volname) = @_;
+
+ $class->parse_volname($target_volname);
+
+ my (undef, $source_volname) =
PVE::Storage::Plugin::parse_volume_id($source_volid); > +
+ my $vg = $scfg->{vgname};
+ my $lvs = lvm_list_volumes($vg);
+ die "target volume '${target_volname}' already exists\n"
+ if ($lvs->{$vg}->{$target_volname});
+
+ lvrename($scfg->{vgname}, $source_volname, $target_volname);
Nit: $vg can be re-used
+ return "${storeid}:${target_volname}";
+}
+
1;
diff --git a/PVE/Storage/LvmThinPlugin.pm b/PVE/Storage/LvmThinPlugin.pm
index 4ba6f90..c24af22 100644
--- a/PVE/Storage/LvmThinPlugin.pm
+++ b/PVE/Storage/LvmThinPlugin.pm
@@ -355,6 +355,7 @@ sub volume_has_feature {
template => { current => 1},
copy => { base => 1, current => 1, snap => 1},
sparseinit => { base => 1, current => 1},
+ rename => {current => 1},
};
my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 5c6c659..3f38a94 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -967,6 +967,7 @@ sub volume_has_feature {
snap => {qcow2 => 1} },
sparseinit => { base => {qcow2 => 1, raw => 1, vmdk => 1},
current => {qcow2 => 1, raw => 1, vmdk => 1} },
+ rename => { current =>{qcow2 => 1, raw => 1, vmdk => 1} },
Style nit: missing space
};
# clone_image creates a qcow2 volume
@@ -974,6 +975,14 @@ sub volume_has_feature {
defined($opts->{valid_target_formats}) &&
!(grep { $_ eq 'qcow2' } @{$opts->{valid_target_formats}});
+ if (
+ $feature eq 'rename'
+ && $class->can('api')
+ && $class->api() < 9
Should be < 10 now like below.
+ ) {
+ return 0;
+ }
+
my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) =
$class->parse_volname($volname);
@@ -1491,4 +1500,53 @@ sub volume_import_formats {
return ();
}
+sub rename_volume {
+ my ($class, $scfg, $storeid, $source_volid, $target_volname) = @_;
+ die "not implemented in storage plugin '$class'\n" if $class->can('api') &&
$class->api() < 10;
Could also die if there is no $scfg->{path} (so that PBS, ISCSI, etc.
die if the method is ever called for them).
+
+ my $target_vmid;
+
+ if ($target_volname =~ m/^(base|vm)?-(\d+)-\S+$/) {
+ $target_vmid = $2;
+ } else {
+ die "could not parse target vmid\n";
+ }
+
+ $class->parse_volname("${target_vmid}/${target_volname}");
So the $target_volname that's passed in here is not an actual volname by
our "definition", i.e. something that can be parsed by parse_volname.
And passing in an actual volname like '123/vm-123-disk-0.raw' doesn't work.
+
+ my (undef, $source_volname) =
PVE::Storage::Plugin::parse_volume_id($source_volid);
+ my (
+ undef,
+ $source_name,
+ $source_vmid,
+ $base_name,
+ $base_vmid,
+ undef,
+ $format
+ ) = $class->parse_volname($source_volname);
+
+ my $basedir = $class->get_subdir($scfg, 'images');
+ # $source_volname includes the '<vmid>/' prefix
+ my $sourcedir = "${basedir}/${source_vmid}";
+ my $targetdir = "${basedir}/${target_vmid}";
+
+ mkpath $targetdir;
+
+ if ($target_volname !~ m/(vm|subsvol)-\d+-\S+\.(raw|qcow2|vmdk)$/) {
+ $target_volname = "${target_volname}.${format}";
+ }
parse_volname for the target didn't fail, so no need for this check.
+
+ my $old_path = "${sourcedir}/${source_name}";
+ my $new_path = "${targetdir}/${target_volname}";
+
+ die "target volume '${target_volname}' already exists\n" if -e $new_path;
+
+ my $base = $base_name ? "${base_vmid}/${base_name}/" : '';
+
+ rename($old_path, $new_path) ||
+ die "rename '$old_path' to '$new_path' failed - $!\n";
+
+ return "${storeid}:${base}${target_vmid}/${target_volname}";
+}
+
1;
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 86ea45a..6ae846a 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -733,6 +733,7 @@ sub volume_has_feature {
template => { current => 1},
copy => { base => 1, current => 1, snap => 1},
sparseinit => { base => 1, current => 1},
+ rename => {current => 1},
};
my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) = $class->parse_volname($volname);
@@ -748,4 +749,32 @@ sub volume_has_feature {
return undef;
}
+sub rename_volume {
+ my ($class, $scfg, $storeid, $source_volid, $target_volname) = @_;
+
+ $class->parse_volname($target_volname);
+
+ my (undef, $source_volname) =
PVE::Storage::Plugin::parse_volume_id($source_volid);
+ my (undef, undef, $source_vmid, $base_name) =
$class->parse_volname($source_volname);
+
+ eval {
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'info', $target_volname);
+ run_rbd_command($cmd, errmsg => "exist check", quiet => 1, noerr => 1);
+ };
+ my $err = $@;
+ die "target volume '${target_volname}' already exists\n"
+ if !$err;
+
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'rename', $source_volname,
$target_volname);
+
+ run_rbd_command(
+ $cmd,
+ errmsg => "could not rename image '${source_volname}' to
'${target_volname}'",
+ );
+
+ $base_name = $base_name ? "${base_name}/" : '';
+
+ return "${storeid}:${base_name}${target_volname}";
+}
+
1;
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 85e2211..ac375e8 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -692,6 +692,7 @@ sub volume_has_feature {
copy => { base => 1, current => 1},
sparseinit => { base => 1, current => 1},
replicate => { base => 1, current => 1},
+ rename => {current => 1},
};
my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
@@ -794,4 +795,27 @@ sub volume_import_formats {
return $class->volume_export_formats($scfg, $storeid, $volname,
$snapshot, $base_snapshot, $with_snapshots);
}
+sub rename_volume {
+ my ($class, $scfg, $storeid, $source_volid, $target_volname) = @_;
+
+ $class->parse_volname($target_volname);
+
+ my (undef, $source_volname) =
PVE::Storage::Plugin::parse_volume_id($source_volid);
+ my (undef, undef, $source_vmid, $base_name) =
$class->parse_volname($source_volname);
+
+ my $pool = $scfg->{pool};
+ my $source_zfspath = "${pool}/${source_volname}";
+ my $target_zfspath = "${pool}/${target_volname}";
+
+ my $exists = 0 == run_command(['zfs', 'get', '-H', 'name',
$target_zfspath],
+ noerr => 1, quiet => 1);
+ die "target volume '${target_volname}' already exists\n" if $exists;
+
+ $class->zfs_request($scfg, 5, 'rename', ${source_zfspath},
${target_zfspath});
+
+ $base_name = $base_name ? "${base_name}/" : '';
+
+ return "${storeid}:${base_name}${target_volname}";
+}
+
1;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
