Introduce allow_rename parameter for pvesm import and storage_migrate

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 of 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@proxmox.com>

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;
 

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 => 4;
+use constant APIVER => 5;
 # 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 => 3;
+use constant APIAGE => 4;
 
 # 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 >= 5;
 
     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>;
+		$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 >= 5;
     };
     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 {

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
+	$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 {

PVE/Storage/Plugin.pm

@@ -1227,7 +1227,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$/;
@@ -1249,16 +1249,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
+	$volname = $allocname;
+	if (defined($name) && $allocname ne $oldname) {
 	    die "internal error: unexpected allocated name: '$allocname' != '$oldname'\n";
 	}
 	my $file = $class->path($scfg, $volname, $storeid)
@@ -1278,6 +1282,8 @@ sub volume_import {
 	warn $@ if $@;
 	die $err;
     }
+
+    return "$storeid:$volname";
 }
 
 sub volume_import_formats {

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 {