[pve-devel] [PATCH storage v9 1/6] api: content: implement copying volumes between storages

[Filip Schauer <f.schauer@proxmox.com>]

Add the ability to copy an `iso`, `snippet`, `vztmpl` or `import` volume
between storages and nodes.

This adapts the experimental storage content copy API method and adds a
few parameters. This also includes a breaking change by renaming
'target_node' to 'target-node'. Since the API method was experimental,
this breaking change is justified by the consistent naming convention of
parameters.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/PVE/API2/Storage/Content.pm | 135 +++++++++++++++++++++-----------
 1 file changed, 89 insertions(+), 46 deletions(-)

diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
index 1fe7303..201d839 100644
--- a/src/PVE/API2/Storage/Content.pm
+++ b/src/PVE/API2/Storage/Content.pm
@@ -435,6 +435,21 @@ __PACKAGE__->register_method({
     },
 });
 
+my $volume_remove = sub {
+    my ($cfg, $volid) = @_;
+
+    my ($path, undef, $vtype) = PVE::Storage::path($cfg, $volid);
+
+    PVE::Storage::vdisk_free($cfg, $volid);
+    if (
+        $vtype eq 'backup'
+        && $path =~ /(.*\/vzdump-\w+-\d+-\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})[^\/]+$/
+    ) {
+        # Remove log file #318 and notes file #3972 if they still exist
+        PVE::Storage::archive_auxiliaries_remove($path);
+    }
+};
+
 __PACKAGE__->register_method({
     name => 'delete',
     path => '{volume}',
@@ -493,15 +508,8 @@ __PACKAGE__->register_method({
         }
 
         my $worker = sub {
-            PVE::Storage::vdisk_free($cfg, $volid);
+            &$volume_remove($cfg, $volid);
             print "Removed volume '$volid'\n";
-            if (
-                $vtype eq 'backup'
-                && $path =~ /(.*\/vzdump-\w+-\d+-\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})[^\/]+$/
-            ) {
-                # Remove log file #318 and notes file #3972 if they still exist
-                PVE::Storage::archive_auxiliaries_remove($path);
-            }
         };
 
         my $id = (defined $ownervm ? "$ownervm@" : '') . $storeid;
@@ -532,29 +540,50 @@ __PACKAGE__->register_method({
     name => 'copy',
     path => '{volume}',
     method => 'POST',
-    description => "Copy a volume. This is experimental code - do not use.",
+    description => "Copy a volume.",
+    permissions => {
+        description => "If the --delete option is used, the 'Datastore.Allocate' privilege is"
+            . " required on the source storage. 'Datastore.AllocateSpace' is required on the target"
+            . " storage.",
+        user => 'all',
+    },
     protected => 1,
     proxyto => 'node',
     parameters => {
         additionalProperties => 0,
         properties => {
             node => get_standard_option('pve-node'),
-            storage => get_standard_option('pve-storage-id', { optional => 1 }),
+            storage => get_standard_option(
+                'pve-storage-id',
+                {
+                    optional => 1,
+                    completion => \&PVE::Storage::complete_storage_enabled,
+                },
+            ),
             volume => {
                 description => "Source volume identifier",
                 type => 'string',
+                completion => \&PVE::Storage::complete_volume,
             },
             target => {
-                description => "Target volume identifier",
+                description => "Target storage or volume identifier.",
                 type => 'string',
+                completion => \&PVE::Storage::complete_volume,
             },
-            target_node => get_standard_option(
+            'target-node' => get_standard_option(
                 'pve-node',
                 {
                     description => "Target node. Default is local node.",
                     optional => 1,
                 },
             ),
+            delete => {
+                type => 'boolean',
+                description => "Delete the original volume after a successful copy."
+                    . " By default the original is kept.",
+                optional => 1,
+                default => 0,
+            },
         },
     },
     returns => {
@@ -563,49 +592,63 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        my $rpcenv = PVE::RPCEnvironment::get();
-
-        my $user = $rpcenv->get_user();
-
-        my $target_node = $param->{target_node} || PVE::INotify::nodename();
-        # pvesh examples
-        # cd /nodes/localhost/storage/local/content
-        # pve:/> create local:103/vm-103-disk-1.raw -target local:103/vm-103-disk-2.raw
-        # pve:/> create 103/vm-103-disk-1.raw -target 103/vm-103-disk-3.raw
-
-        my $src_volid = &$real_volume_id($param->{storage}, $param->{volume});
-        my $dst_volid = &$real_volume_id($param->{storage}, $param->{target});
-
-        print "DEBUG: COPY $src_volid TO $dst_volid\n";
-
+        my $src_volid = $real_volume_id->($param->{storage}, $param->{volume});
+        my ($src_storeid, $src_volname) = PVE::Storage::parse_volume_id($src_volid);
         my $cfg = PVE::Storage::config();
+        my ($src_path, $ownervm, $vtype) = PVE::Storage::path($cfg, $src_volid);
+
+        die "use pct move-volume or qm disk move\n"
+            if $vtype eq 'images' || $vtype eq 'rootdir';
+        die "moving volume of type '$vtype' not implemented\n"
+            if !grep { $vtype eq $_ } qw(import iso snippets vztmpl);
+
+        my ($dst_storeid, $dst_volname);
+        if ($param->{target} =~ m/:/) {
+            ($dst_storeid, $dst_volname) = PVE::Storage::parse_volume_id($param->{target});
+            my $dst_vtype = (PVE::Storage::parse_volname($cfg, $param->{target}))[0];
+            die "source and target must have the same volume type\n" if $vtype ne $dst_vtype;
+        } else {
+            $dst_storeid = $param->{target};
+            $dst_volname = $src_volname;
+        }
+        my $dst_volid = "$dst_storeid:$dst_volname";
 
-        # do all parameter checks first
+        my $src_node = PVE::INotify::nodename();
+        my $dst_node = $param->{'target-node'} || $src_node;
+        my $delete = $param->{delete};
 
-        # then do all short running task (to raise errors before we go to background)
+        die "source and target cannot be the same\n"
+            if $src_node eq $dst_node && $src_storeid eq $dst_storeid;
 
-        # then start the worker task
-        my $worker = sub {
-            my $upid = shift;
+        my $rpcenv = PVE::RPCEnvironment::get();
+        my $user = $rpcenv->get_user();
 
-            print "DEBUG: starting worker $upid\n";
+        PVE::Storage::check_volume_access($rpcenv, $user, $cfg, undef, $src_volid);
 
-            my ($target_sid, $target_volname) = PVE::Storage::parse_volume_id($dst_volid);
-            #my $target_ip = PVE::Cluster::remote_node_ip($target_node);
+        if ($delete) {
+            $rpcenv->check($user, "/storage/$src_storeid", ["Datastore.Allocate"]);
+        }
 
-            # you need to get this working (fails currently, because storage_migrate() uses
-            # ssh to connect to local host (which is not needed
-            my $sshinfo = PVE::SSHInfo::get_ssh_info($target_node);
-            PVE::Storage::storage_migrate(
-                $cfg,
-                $src_volid,
-                $sshinfo,
-                $target_sid,
-                { 'target_volname' => $target_volname },
-            );
+        $rpcenv->check($user, "/storage/$dst_storeid", ["Datastore.AllocateSpace"]);
 
-            print "DEBUG: end worker $upid\n";
+        my $worker = sub {
+            PVE::Storage::storage_check_enabled($cfg, $src_storeid, $src_node);
+            PVE::Storage::storage_check_enabled($cfg, $dst_storeid, $dst_node);
+            my $sshinfo;
+            $sshinfo = PVE::SSHInfo::get_ssh_info($dst_node) if $src_node ne $dst_node;
+            my $opts = { 'target_volname' => $volname };
+            PVE::Storage::storage_migrate($cfg, $src_volid, $sshinfo, $dst_storeid, $opts);
+
+            if ($delete) {
+                &$volume_remove($cfg, $src_volid);
+            }
 
+            if ($src_node eq $dst_node) {
+                print "Copied volume '$src_volid' to '$dst_volid'\n";
+            } else {
+                print "Copied volume '$src_volid' on node '$src_node'"
+                    . " to '$dst_volid' on node '$dst_node'\n";
+            }
         };
 
         return $rpcenv->fork_worker('imgcopy', undef, $user, $worker);
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel