[pve-devel] [RFC storage 3/16] volume_import: Use a new name when the the name to import with already exists

[Fabian Ebner]

The ID of the new volume is returned and pvesm import prints it. This is
useful for migration, since the storage on the target might already contain
unused/orphaned disks.

Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
---

Breaks the current migration in QEMU/LXC if there is a collision,
since the code doesn't die anymore and the config is not updated
with the new name, i.e. needs patches 1,5,12 and 13.

 PVE/CLI/pvesm.pm             | 13 ++++++++-----
 PVE/Storage/LVMPlugin.pm     | 14 +++++++++-----
 PVE/Storage/Plugin.pm        | 12 ++++++++----
 PVE/Storage/ZFSPoolPlugin.pm | 10 ++++++----
 4 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 510faba..7d8547b 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -299,7 +299,7 @@ __PACKAGE__->register_method ({
 	    },
 	},
     },
-    returns => { type => 'null' },
+    returns => { type => 'string' },
     code => sub {
 	my ($param) = @_;
 
@@ -352,11 +352,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},
+	my $volid = PVE::Storage::volume_import($cfg, $infh, $volume, $param->{format},
 	    $param->{base}, $param->{'with-snapshots'});
-	PVE::Storage::volume_snapshot_delete($cfg, $volume, $delete)
+	PVE::Storage::volume_snapshot_delete($cfg, $volid, $delete)
 	    if defined($delete);
-	return;
+	return $volid;
     }
 });
 
@@ -777,7 +777,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 "successfully imported '$volid'\n";
+    }],
 };
 
 1;
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index f02c110..6c575f2 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -638,17 +638,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}) {
+	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)
@@ -661,6 +664,7 @@ 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 0c39cbd..358a831 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1198,16 +1198,19 @@ 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) {
+	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)
@@ -1227,6 +1230,7 @@ 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 d72ee16..6394692 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -750,15 +750,17 @@ 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) {
+	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") };
@@ -771,7 +773,7 @@ sub volume_import {
 	die $err;
     }
 
-    return;
+    return "$storeid:$dataset";
 }
 
 sub volume_import_formats {
-- 
2.20.1