[pve-devel] [PATCH v4 storage 18/27] Introduce allow_rename parameter for pvesm import and storage_migrate
Fabian Ebner
f.ebner at proxmox.com
Thu Mar 26 09:09:48 CET 2020
and also return the ID of the allocated volume. This option
allows plugins to choose a new name if there is a collision.
In storage_migrate, the API version for the receiving side is checked.
In Storage.pm's volume_import, when a plugin returns 'undef',
it can be assumed that the import with the requested volid was
successful (it should've died otherwise) and so volid is returned.
This is done for backwards compatibility with foreign plugins.
Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
---
NOTE that the APIVER/AGE bump conflicts with the APIVER/AGE bump
for my other patch-set for container templates [0]
[0]: https://pve.proxmox.com/pipermail/pve-devel/2020-March/042539.html
PVE/CLI/pvesm.pm | 22 +++++++++++----
PVE/Storage.pm | 55 +++++++++++++++++++++++++++---------
PVE/Storage/LVMPlugin.pm | 17 +++++++----
PVE/Storage/Plugin.pm | 16 +++++++----
PVE/Storage/ZFSPoolPlugin.pm | 13 +++++----
5 files changed, 88 insertions(+), 35 deletions(-)
diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 0d1d816..30bdcf6 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -321,9 +321,16 @@ __PACKAGE__->register_method ({
maxLength => 80,
optional => 1,
},
+ 'allow-rename' => {
+ description => "Choose a new volume ID if the requested " .
+ "volume ID already exists, instead of throwing an error.",
+ type => 'boolean',
+ optional => 1,
+ default => 0,
+ },
},
},
- returns => { type => 'null' },
+ returns => { type => 'string' },
code => sub {
my ($param) = @_;
@@ -376,11 +383,11 @@ __PACKAGE__->register_method ({
my $cfg = PVE::Storage::config();
my $volume = $param->{volume};
my $delete = $param->{'delete-snapshot'};
- PVE::Storage::volume_import($cfg, $infh, $volume, $param->{format},
- $param->{base}, $param->{'with-snapshots'});
- PVE::Storage::volume_snapshot_delete($cfg, $volume, $delete)
+ my $imported_volid = PVE::Storage::volume_import($cfg, $infh, $volume, $param->{format},
+ $param->{base}, $param->{'with-snapshots'}, $param->{'allow-rename'});
+ PVE::Storage::volume_snapshot_delete($cfg, $imported_volid, $delete)
if defined($delete);
- return;
+ return $imported_volid;
}
});
@@ -801,7 +808,10 @@ our $cmddef = {
path => [ __PACKAGE__, 'path', ['volume']],
extractconfig => [__PACKAGE__, 'extractconfig', ['volume']],
export => [ __PACKAGE__, 'export', ['volume', 'format', 'filename']],
- import => [ __PACKAGE__, 'import', ['volume', 'format', 'filename']],
+ import => [ __PACKAGE__, 'import', ['volume', 'format', 'filename'], {}, sub {
+ my $volid = shift;
+ print PVE::Storage::volume_imported_message($volid);
+ }],
apiinfo => [ __PACKAGE__, 'apiinfo', [], {}, sub {
my $res = shift;
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index dc415e2..92a71b7 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -39,11 +39,11 @@ use PVE::Storage::DRBDPlugin;
use PVE::Storage::PBSPlugin;
# Storage API version. Icrement it on changes in storage API interface.
-use constant APIVER => 3;
+use constant APIVER => 4;
# 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 => 2;
+use constant APIAGE => 3;
# load standard plugins
PVE::Storage::DirPlugin->register();
@@ -578,7 +578,7 @@ sub storage_migrate {
my $scfg = storage_config($cfg, $storeid);
# no need to migrate shared content
- return if $storeid eq $target_storeid && $scfg->{shared};
+ return $volid if $storeid eq $target_storeid && $scfg->{shared};
my $tcfg = storage_config($cfg, $target_storeid);
@@ -616,6 +616,11 @@ sub storage_migrate {
$import_fn = "tcp://$net";
}
+ my $target_apiver = 1; # if there is no apiinfo call, assume 1
+ my $get_api_version = [@$ssh, 'pvesm', 'apiinfo'];
+ my $match_api_version = sub { $target_apiver = $1 if $_[0] =~ m!^APIVER (\d+)$!; };
+ eval { run_command($get_api_version, logfunc => $match_api_version); };
+
my $send = ['pvesm', 'export', $volid, $format, '-', '-with-snapshots', $with_snapshots];
my $recv = [@$ssh, '--', 'pvesm', 'import', $target_volid, $format, $import_fn, '-with-snapshots', $with_snapshots];
if (defined($snapshot)) {
@@ -624,6 +629,7 @@ sub storage_migrate {
if ($migration_snapshot) {
push @$recv, '-delete-snapshot', $snapshot;
}
+ push @$recv, '-allow-rename', $allow_rename if $target_apiver >= 4;
if (defined($base_snapshot)) {
# Check if the snapshot exists on the remote side:
@@ -631,6 +637,19 @@ sub storage_migrate {
push @$recv, '-base', $base_snapshot;
}
+ my $new_volid;
+ my $pattern = volume_imported_message(undef, 1);
+ my $match_volid_and_log = sub {
+ my $line = shift;
+
+ $new_volid = $1 if ($line =~ $pattern);
+
+ if ($logfunc) {
+ chomp($line);
+ $logfunc->($line);
+ }
+ };
+
volume_snapshot($cfg, $volid, $snapshot) if $migration_snapshot;
eval {
if ($insecure) {
@@ -648,13 +667,8 @@ sub storage_migrate {
shutdown($socket, 1);
# wait for the remote process to finish
- if ($logfunc) {
- while (my $line = <$info>) {
- chomp($line);
- $logfunc->("[$target_sshinfo->{name}] $line");
- }
- } else {
- 1 while <$info>;
+ while (my $line = <$info>) {
+ $match_volid_and_log->("[$target_sshinfo->{name}] $line");
}
# now close the socket
@@ -664,8 +678,11 @@ sub storage_migrate {
die "import failed: exit code ".($?>>8)."\n";
}
} else {
- run_command([$send, @cstream, $recv], logfunc => $logfunc);
+ run_command([$send, @cstream, $recv], logfunc => $match_volid_and_log);
}
+
+ die "unable to get ID of the migrated volume\n"
+ if !defined($new_volid) && $target_apiver >= 4;
};
my $err = $@;
warn "send/receive failed, cleaning up snapshot(s)..\n" if $err;
@@ -674,6 +691,8 @@ sub storage_migrate {
warn "could not remove source snapshot: $@\n" if $@;
}
die $err if $err;
+
+ return $new_volid // $target_volid;
}
sub vdisk_clone {
@@ -1430,14 +1449,14 @@ sub volume_export {
}
sub volume_import {
- my ($cfg, $fh, $volid, $format, $base_snapshot, $with_snapshots) = @_;
+ my ($cfg, $fh, $volid, $format, $base_snapshot, $with_snapshots, $allow_rename) = @_;
my ($storeid, $volname) = parse_volume_id($volid, 1);
die "cannot import into volume '$volid'\n" if !$storeid;
my $scfg = storage_config($cfg, $storeid);
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
return $plugin->volume_import($scfg, $storeid, $fh, $volname, $format,
- $base_snapshot, $with_snapshots);
+ $base_snapshot, $with_snapshots, $allow_rename) // $volid;
}
sub volume_export_formats {
@@ -1472,6 +1491,16 @@ sub volume_transfer_formats {
return @common;
}
+sub volume_imported_message {
+ my ($volid, $want_pattern) = @_;
+
+ if ($want_pattern) {
+ return qr/successfully imported '([^']*)'$/;
+ } else {
+ return "successfully imported '$volid'\n";
+ }
+}
+
# bash completion helper
sub complete_storage {
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index c9fc191..c0740d4 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -631,7 +631,7 @@ sub volume_import_formats {
}
sub volume_import {
- my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots) = @_;
+ my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots, $allow_rename) = @_;
die "volume import format $format not available for $class\n"
if $format ne 'raw+size';
die "cannot import volumes together with their snapshots in $class\n"
@@ -645,17 +645,20 @@ sub volume_import {
my $vg = $scfg->{vgname};
my $lvs = lvm_list_volumes($vg);
- die "volume $vg/$volname already exists\n"
- if $lvs->{$vg}->{$volname};
+ if ($lvs->{$vg}->{$volname}) {
+ die "volume $vg/$volname already exists\n" if !$allow_rename;
+ warn "volume $vg/$volname already exists - importing with a different name\n";
+ $name = undef;
+ }
my ($size) = PVE::Storage::Plugin::read_common_header($fh);
$size = int($size/1024);
eval {
my $allocname = $class->alloc_image($storeid, $scfg, $vmid, 'raw', $name, $size);
- if ($allocname ne $volname) {
- my $oldname = $volname;
- $volname = $allocname; # Let the cleanup code know what to free
+ my $oldname = $volname;
+ $volname = $allocname;
+ if (defined($name) && $allocname ne $oldname) {
die "internal error: unexpected allocated name: '$allocname' != '$oldname'\n";
}
my $file = $class->path($scfg, $volname, $storeid)
@@ -668,6 +671,8 @@ sub volume_import {
warn $@ if $@;
die $err;
}
+
+ return "$storeid:$volname";
}
sub volume_import_write {
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 2232261..4299eb5 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1222,7 +1222,7 @@ sub volume_export_formats {
# Import data from a stream, creating a new or replacing or adding to an existing volume.
sub volume_import {
- my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots) = @_;
+ my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots, $allow_rename) = @_;
die "volume import format '$format' not available for $class\n"
if $format !~ /^(raw|tar|qcow2|vmdk)\+size$/;
@@ -1244,16 +1244,20 @@ sub volume_import {
# Check for an existing file first since interrupting alloc_image doesn't
# free it.
my $file = $class->path($scfg, $volname, $storeid);
- die "file '$file' already exists\n" if -e $file;
+ if (-e $file) {
+ die "file '$file' already exists\n" if !$allow_rename;
+ warn "file '$file' already exists - importing with a different name\n";
+ $name = undef;
+ }
my ($size) = read_common_header($fh);
$size = int($size/1024);
eval {
my $allocname = $class->alloc_image($storeid, $scfg, $vmid, $file_format, $name, $size);
- if ($allocname ne $volname) {
- my $oldname = $volname;
- $volname = $allocname; # Let the cleanup code know what to free
+ my $oldname = $volname;
+ $volname = $allocname;
+ if (defined($name) && $allocname ne $oldname) {
die "internal error: unexpected allocated name: '$allocname' != '$oldname'\n";
}
my $file = $class->path($scfg, $volname, $storeid)
@@ -1273,6 +1277,8 @@ sub volume_import {
warn $@ if $@;
die $err;
}
+
+ return "$storeid:$volname";
}
sub volume_import_formats {
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index b538e3b..b4fd65f 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -743,7 +743,7 @@ sub volume_export_formats {
}
sub volume_import {
- my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots) = @_;
+ my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots, $allow_rename) = @_;
die "unsupported import stream format for $class: $format\n"
if $format ne 'zfs';
@@ -752,15 +752,18 @@ sub volume_import {
die "internal error: invalid file handle for volume_import\n"
if !defined($fd);
- my $dataset = ($class->parse_volname($volname))[1];
+ my (undef, $dataset, $vmid) = $class->parse_volname($volname);
my $zfspath = "$scfg->{pool}/$dataset";
my $suffix = defined($base_snapshot) ? "\@$base_snapshot" : '';
my $exists = 0 == run_command(['zfs', 'get', '-H', 'name', $zfspath.$suffix],
noerr => 1, errfunc => sub {});
if (defined($base_snapshot)) {
die "base snapshot '$zfspath\@$base_snapshot' doesn't exist\n" if !$exists;
- } else {
- die "volume '$zfspath' already exists\n" if $exists;
+ } elsif ($exists) {
+ die "volume '$zfspath' already exists\n" if !$allow_rename;
+ warn "volume '$zfspath' already exists - importing with a different name\n";
+ $dataset = $class->find_free_diskname($storeid, $scfg, $vmid, $format);
+ $zfspath = "$scfg->{pool}/$dataset";
}
eval { run_command(['zfs', 'recv', '-F', '--', $zfspath], input => "<&$fd") };
@@ -773,7 +776,7 @@ sub volume_import {
die $err;
}
- return;
+ return "$storeid:$dataset";
}
sub volume_import_formats {
--
2.20.1
More information about the pve-devel
mailing list