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>

diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 0d1d8160a4225f94c882c3e8d10854b053dd1c02..30bdcf64e9f9fce70ffe537e6798b89b7e829475 100755 (executable)
--- 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 7b285da39241f60e3a0a84bfef57e3a010c7cdcb..bdafdc12dc0ca1c141fdb41ffa110688ec9d3683 100755 (executable)
--- 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 => 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>;
+           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 >= 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 {

diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index c9fc191f78e5cb3764e8dab7a88a438552f9190f..c0740d437f61337a0b53e3f54f1036762ae4c6ec 100644 (file)
--- 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 8c0dae1e09222426e12239e26a6551222465536f..4489a77d034314a0bd384b5703b4e3aa1993368e 100644 (file)
--- a/PVE/Storage/Plugin.pm
+++ b/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
+       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)
@@ -1278,6 +1282,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 b538e3bb207a01f0d8036cf771089a92fbf4fde0..b4fd65f15ccc7e5e405ce6290c3f4c3f68cb4135 100644 (file)
--- 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 {