* [pve-devel] [PATCH storage] add API method to move a volume between storages
@ 2024-06-12 14:45 Filip Schauer
2024-06-12 14:47 ` Filip Schauer
2024-06-14 17:29 ` Thomas Lamprecht
0 siblings, 2 replies; 4+ messages in thread
From: Filip Schauer @ 2024-06-12 14:45 UTC (permalink / raw)
To: pve-devel
Add the ability to move a backup, ISO, container template or snippet
between storages of a node via an API method. Moving a VMA backup to a
Proxmox Backup Server requires the proxmox-vma-to-pbs package to be
installed. Currently only VMA backups can be moved to a Proxmox Backup
Server and moving backups from a Proxmox Backup Server is not yet
supported.
The method can be called from the PVE shell with `pvesm move`:
# pvesm move <source volume> <target storage>
pvesm move local:backup/vzdump-lxc-102-2024_05_29-17_05_27.tar.zst pbs
Or use curl to call the API method:
curl https://$APINODE:8006/api2/json/nodes/$TARGETNODE/storage/$TARGETSTORAGE/move \
--insecure --cookie "$(<cookie)" -H "$(<csrftoken)" -X POST \
--data-raw "source-volume=$SOURCEVOLUME"
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
This patch depends on
[PATCH backup-qemu/vma-to-pbs 0/2] add support for notes and logs
https://lists.proxmox.com/pipermail/pbs-devel/2024-May/009445.html
src/PVE/API2/Storage/Makefile | 2 +-
src/PVE/API2/Storage/MoveVolume.pm | 61 +++++++++
src/PVE/API2/Storage/Status.pm | 7 ++
src/PVE/CLI/pvesm.pm | 33 +++++
src/PVE/Storage.pm | 195 +++++++++++++++++++++++++----
5 files changed, 271 insertions(+), 27 deletions(-)
create mode 100644 src/PVE/API2/Storage/MoveVolume.pm
diff --git a/src/PVE/API2/Storage/Makefile b/src/PVE/API2/Storage/Makefile
index 1705080..11f3c95 100644
--- a/src/PVE/API2/Storage/Makefile
+++ b/src/PVE/API2/Storage/Makefile
@@ -1,5 +1,5 @@
-SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm Scan.pm FileRestore.pm
+SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm Scan.pm FileRestore.pm MoveVolume.pm
.PHONY: install
install:
diff --git a/src/PVE/API2/Storage/MoveVolume.pm b/src/PVE/API2/Storage/MoveVolume.pm
new file mode 100644
index 0000000..52447a4
--- /dev/null
+++ b/src/PVE/API2/Storage/MoveVolume.pm
@@ -0,0 +1,61 @@
+package PVE::API2::Storage::MoveVolume;
+
+use strict;
+use warnings;
+
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTHandler;
+use PVE::RPCEnvironment;
+use PVE::Storage;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+ name => 'move',
+ path => '',
+ method => 'POST',
+ description => "Move a volume from one storage to another",
+ permissions => {
+ description => "You need the 'Datastore.Allocate' privilege on the storages.",
+ user => 'all',
+ },
+ protected => 1,
+ proxyto => 'node',
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ 'source-volume' => {
+ description => "Source volume",
+ type => 'string',
+ },
+ 'storage' => get_standard_option('pve-storage-id', {
+ completion => \&PVE::Storage::complete_storage_enabled,
+ description => 'Target storage',
+ }),
+ },
+ },
+ returns => { type => 'string' },
+ code => sub {
+ my ($param) = @_;
+
+ my $cfg = PVE::Storage::config();
+ my $source_volume = $param->{'source-volume'};
+ my $target_storeid = $param->{'storage'};
+
+ my ($source_storeid, undef) = PVE::Storage::parse_volume_id($source_volume, 0);
+
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $authuser = $rpcenv->get_user();
+
+ $rpcenv->check($authuser, "/storage/$source_storeid", ["Datastore.Allocate"]);
+ $rpcenv->check($authuser, "/storage/$target_storeid", ["Datastore.Allocate"]);
+
+ my $worker = sub {
+ PVE::Storage::volume_move($cfg, $source_volume, $target_storeid);
+ };
+
+ return $rpcenv->fork_worker('move_volume', '', $authuser, $worker);
+ }});
+
+1;
diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index dc6cc69..6c816b7 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -18,6 +18,7 @@ use PVE::Tools qw(run_command);
use PVE::API2::Storage::Content;
use PVE::API2::Storage::FileRestore;
+use PVE::API2::Storage::MoveVolume;
use PVE::API2::Storage::PruneBackups;
use PVE::Storage;
@@ -28,6 +29,11 @@ __PACKAGE__->register_method ({
path => '{storage}/prunebackups',
});
+__PACKAGE__->register_method ({
+ subclass => "PVE::API2::Storage::MoveVolume",
+ path => '{storage}/move',
+});
+
__PACKAGE__->register_method ({
subclass => "PVE::API2::Storage::Content",
# set fragment delimiter (no subdirs) - we need that, because volume
@@ -233,6 +239,7 @@ __PACKAGE__->register_method ({
{ subdir => 'rrddata' },
{ subdir => 'status' },
{ subdir => 'upload' },
+ { subdir => 'move' },
];
return $res;
diff --git a/src/PVE/CLI/pvesm.pm b/src/PVE/CLI/pvesm.pm
index 9b9676b..4c042aa 100755
--- a/src/PVE/CLI/pvesm.pm
+++ b/src/PVE/CLI/pvesm.pm
@@ -20,6 +20,7 @@ use PVE::Storage;
use PVE::Tools qw(extract_param);
use PVE::API2::Storage::Config;
use PVE::API2::Storage::Content;
+use PVE::API2::Storage::MoveVolume;
use PVE::API2::Storage::PruneBackups;
use PVE::API2::Storage::Scan;
use PVE::API2::Storage::Status;
@@ -480,6 +481,37 @@ __PACKAGE__->register_method ({
}
});
+__PACKAGE__->register_method ({
+ name => 'move',
+ path => 'move',
+ method => 'POST',
+ description => "Move a volume from one storage to another",
+ protected => 1,
+ proxyto => 'node',
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ 'source-volume' => {
+ description => "Source volume",
+ type => 'string',
+ },
+ 'storage' => get_standard_option('pve-storage-id', {
+ completion => \&PVE::Storage::complete_storage_enabled,
+ description => 'Target storage',
+ }),
+ },
+ },
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+
+ PVE::API2::Storage::MoveVolume->move($param);
+
+ return;
+ },
+});
+
__PACKAGE__->register_method ({
name => 'prunebackups',
path => 'prunebackups',
@@ -690,6 +722,7 @@ our $cmddef = {
print "APIVER $res->{apiver}\n";
print "APIAGE $res->{apiage}\n";
}],
+ 'move' => [ __PACKAGE__, 'move', ['source-volume', 'storage'], { node => $nodename } ],
'prune-backups' => [ __PACKAGE__, 'prunebackups', ['storage'], { node => $nodename }, sub {
my $res = shift;
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index f19a115..fe8a5e0 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -9,6 +9,7 @@ use IO::File;
use IO::Socket::IP;
use IPC::Open3;
use File::Basename;
+use File::Copy qw(move);
use File::Path;
use Cwd 'abs_path';
use Socket;
@@ -1620,14 +1621,20 @@ sub archive_info {
}
sub archive_remove {
- my ($archive_path) = @_;
+ my ($archive_path, $ignore_protected) = @_;
+
+ my $protection_path = protection_file_path($archive_path);
die "cannot remove protected archive '$archive_path'\n"
- if -e protection_file_path($archive_path);
+ if !$ignore_protected && -e $protection_path;
unlink $archive_path or $! == ENOENT or die "removing archive $archive_path failed: $!\n";
archive_auxiliaries_remove($archive_path);
+
+ if (-e $protection_path) {
+ unlink $protection_path or $! == ENOENT or log_warn("Removing protection file failed: $!");
+ }
}
sub archive_auxiliaries_remove {
@@ -1680,6 +1687,45 @@ sub extract_vzdump_config_tar {
return wantarray ? ($raw, $file) : $raw;
}
+sub decompress_archive_into_pipe {
+ my ($archive, $cmd, $outfunc) = @_;
+
+ my $info = archive_info($archive);
+ die "archive is not compressed\n" if !$info->{compression};
+ my $decompressor = $info->{decompressor};
+ my $full_cmd = [ [@$decompressor, $archive], $cmd ];
+
+ # lzop/zcat exits with 1 when the pipe is closed early,
+ # detect this and ignore the exit code later
+ my $broken_pipe;
+ my $errstring;
+ my $err = sub {
+ my $output = shift;
+ if (
+ $output =~ m/lzop: Broken pipe: <stdout>/
+ || $output =~ m/gzip: stdout: Broken pipe/
+ || $output =~ m/zstd: error 70 : Write error.*Broken pipe/
+ ) {
+ $broken_pipe = 1;
+ } elsif (!defined ($errstring) && $output !~ m/^\s*$/) {
+ $errstring = "failed to decompress archive: $output\n";
+ }
+ };
+
+ my $rc = eval { run_command($full_cmd, outfunc => $outfunc, errfunc => $err, noerr => 1) };
+ my $rerr = $@;
+
+ $broken_pipe ||= $rc == 141; # broken pipe from cmd POV
+
+ if (!$errstring && !$broken_pipe && $rc != 0) {
+ die "$rerr\n" if $rerr;
+ die "archive decompression failed with exit code $rc\n";
+ }
+ die "$errstring\n" if $errstring;
+
+ return;
+}
+
sub extract_vzdump_config_vma {
my ($archive, $comp) = @_;
@@ -1691,30 +1737,7 @@ sub extract_vzdump_config_vma {
my $decompressor = $info->{decompressor};
if ($comp) {
- my $cmd = [ [@$decompressor, $archive], ["vma", "config", "-"] ];
-
- # lzop/zcat exits with 1 when the pipe is closed early by vma, detect this and ignore the exit code later
- my $broken_pipe;
- my $errstring;
- my $err = sub {
- my $output = shift;
- if ($output =~ m/lzop: Broken pipe: <stdout>/ || $output =~ m/gzip: stdout: Broken pipe/ || $output =~ m/zstd: error 70 : Write error.*Broken pipe/) {
- $broken_pipe = 1;
- } elsif (!defined ($errstring) && $output !~ m/^\s*$/) {
- $errstring = "Failed to extract config from VMA archive: $output\n";
- }
- };
-
- my $rc = eval { run_command($cmd, outfunc => $out, errfunc => $err, noerr => 1) };
- my $rerr = $@;
-
- $broken_pipe ||= $rc == 141; # broken pipe from vma POV
-
- if (!$errstring && !$broken_pipe && $rc != 0) {
- die "$rerr\n" if $rerr;
- die "config extraction failed with exit code $rc\n";
- }
- die "$errstring\n" if $errstring;
+ decompress_archive_into_pipe($archive, ["vma", "config", "-"], $out);
} else {
run_command(["vma", "config", $archive], outfunc => $out);
}
@@ -1753,6 +1776,126 @@ sub extract_vzdump_config {
}
}
+sub volume_move {
+ my ($cfg, $source_volid, $target_storeid) = @_;
+
+ my ($source_storeid, $source_volname) = parse_volume_id($source_volid, 0);
+
+ die "source and target storage cannot be the same\n" if ($source_storeid eq $target_storeid);
+
+ activate_storage($cfg, $source_storeid);
+ my $source_scfg = storage_config($cfg, $source_storeid);
+ my $source_plugin = PVE::Storage::Plugin->lookup($source_scfg->{type});
+ my ($vtype) = $source_plugin->parse_volname($source_volname);
+
+ die "source storage '$source_storeid' does not support volumes of type '$vtype'\n"
+ if !$source_scfg->{content}->{$vtype};
+
+ activate_storage($cfg, $target_storeid);
+ my $target_scfg = storage_config($cfg, $target_storeid);
+ die "target storage '$target_storeid' does not support volumes of type '$vtype'\n"
+ if !$target_scfg->{content}->{$vtype};
+
+ if ($vtype eq 'backup' || $vtype eq 'iso' || $vtype eq 'vztmpl' || $vtype eq 'snippets') {
+ my $target_plugin = PVE::Storage::Plugin->lookup($target_scfg->{type});
+
+ die "moving a backup from a Proxmox Backup Server is not yet supported\n"
+ if ($vtype eq 'backup' && $source_scfg->{type} eq 'pbs');
+
+ my $source_path = $source_plugin->filesystem_path($source_scfg, $source_volname);
+ die "$source_path does not exist" if (!-e $source_path);
+ my $source_dirname = dirname($source_path);
+
+ return if $vtype ne 'backup';
+
+ if ($target_scfg->{type} eq 'pbs') {
+ my $info = archive_info($source_path);
+ die "moving non-VMA backups to a Proxmox Backup Server is not yet supported\n"
+ if ($info->{format} ne 'vma');
+
+ my $repo = PVE::PBSClient::get_repository($target_scfg);
+ my $vmid = ($source_plugin->parse_volname($source_volname))[2];
+ my $fingerprint = $target_scfg->{fingerprint};
+ my $password = PVE::Storage::PBSPlugin::pbs_password_file_name(
+ $target_scfg, $target_storeid);
+ my $namespace = $target_scfg->{namespace};
+ my $keyfile = PVE::Storage::PBSPlugin::pbs_encryption_key_file_name(
+ $target_scfg, $target_storeid);
+ my $master_keyfile = PVE::Storage::PBSPlugin::pbs_master_pubkey_file_name(
+ $target_scfg, $target_storeid);
+
+ my $comp = $info->{compression};
+ my $backup_time = $info->{ctime};
+ my $log_file_path = "$source_dirname/$info->{logfilename}";
+ my $notes_file_path = "$source_dirname/$info->{notesfilename}";
+
+ my $vma_to_pbs_cmd = [
+ "vma-to-pbs",
+ "--repository", $repo,
+ "--vmid", $vmid,
+ "--fingerprint", $fingerprint,
+ "--password-file", $password,
+ "--backup-time", $backup_time,
+ "--compress",
+ ];
+
+ push @$vma_to_pbs_cmd, "--ns", $namespace if $namespace;
+ push @$vma_to_pbs_cmd, "--log-file", $log_file_path if -e $log_file_path;
+ push @$vma_to_pbs_cmd, "--notes-file", $notes_file_path if -e $notes_file_path;
+ push @$vma_to_pbs_cmd, "--encrypt", "--keyfile", $keyfile if -e $keyfile;
+ push @$vma_to_pbs_cmd, "--master-keyfile", $master_keyfile if -e $master_keyfile;
+
+ if ($comp) {
+ decompress_archive_into_pipe($source_path, $vma_to_pbs_cmd);
+ } else {
+ push @$vma_to_pbs_cmd, $source_path;
+ run_command($vma_to_pbs_cmd);
+ }
+
+ my $protection_source_path = protection_file_path($source_path);
+
+ if (-e $protection_source_path) {
+ my $target_volid = PVE::Storage::PBSPlugin::print_volid(
+ $target_storeid, 'vm', $vmid, $backup_time);
+ my (undef, $target_volname) = parse_volume_id($target_volid, 0);
+ $target_plugin->update_volume_attribute(
+ $target_scfg, $target_storeid, $target_volname, 'protected', 1);
+ }
+
+ archive_remove($source_path, 1);
+ } else {
+ my $target_path = $target_plugin->filesystem_path($target_scfg, $source_volname);
+
+ move($source_path, $target_path) or die "failed to move $vtype: $!";
+
+ my $target_dirname = dirname($target_path);
+ my $info = archive_info($source_path);
+
+ for my $type (qw(log notes)) {
+ my $filename = $info->{"${type}filename"} or next;
+ $source_path = "$source_dirname/$filename";
+ $target_path = "$target_dirname/$filename";
+ move($source_path, $target_path) or die "moving backup $type file failed: $!"
+ if -e $source_path;
+ }
+
+ my $protection_source_path = protection_file_path($source_path);
+
+ if (-e $protection_source_path) {
+ my $protection_target_path = protection_file_path($target_path);
+ move($protection_source_path, $protection_target_path)
+ or die "moving backup protection file failed: $!";
+ }
+ }
+ } elsif ($vtype eq 'images') {
+ die "use pct move-volume or qm disk move\n";
+ } elsif ($vtype eq 'rootdir') {
+ die "cannot move OpenVZ rootdir\n";
+ }
+
+ return;
+}
+
sub prune_backups {
my ($cfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc) = @_;
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH storage] add API method to move a volume between storages
2024-06-12 14:45 [pve-devel] [PATCH storage] add API method to move a volume between storages Filip Schauer
@ 2024-06-12 14:47 ` Filip Schauer
2024-06-14 17:29 ` Thomas Lamprecht
1 sibling, 0 replies; 4+ messages in thread
From: Filip Schauer @ 2024-06-12 14:47 UTC (permalink / raw)
To: pve-devel
I forgot to mention that this fixes #5191
On 12/06/2024 16:45, Filip Schauer wrote:
> Add the ability to move a backup, ISO, container template or snippet
> between storages of a node via an API method. Moving a VMA backup to a
> Proxmox Backup Server requires the proxmox-vma-to-pbs package to be
> installed. Currently only VMA backups can be moved to a Proxmox Backup
> Server and moving backups from a Proxmox Backup Server is not yet
> supported.
>
> The method can be called from the PVE shell with `pvesm move`:
>
> # pvesm move <source volume> <target storage>
> pvesm move local:backup/vzdump-lxc-102-2024_05_29-17_05_27.tar.zst pbs
>
> Or use curl to call the API method:
>
> curl https://$APINODE:8006/api2/json/nodes/$TARGETNODE/storage/$TARGETSTORAGE/move \
> --insecure --cookie "$(<cookie)" -H "$(<csrftoken)" -X POST \
> --data-raw "source-volume=$SOURCEVOLUME"
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
> This patch depends on
> [PATCH backup-qemu/vma-to-pbs 0/2] add support for notes and logs
> https://lists.proxmox.com/pipermail/pbs-devel/2024-May/009445.html
>
> src/PVE/API2/Storage/Makefile | 2 +-
> src/PVE/API2/Storage/MoveVolume.pm | 61 +++++++++
> src/PVE/API2/Storage/Status.pm | 7 ++
> src/PVE/CLI/pvesm.pm | 33 +++++
> src/PVE/Storage.pm | 195 +++++++++++++++++++++++++----
> 5 files changed, 271 insertions(+), 27 deletions(-)
> create mode 100644 src/PVE/API2/Storage/MoveVolume.pm
>
> diff --git a/src/PVE/API2/Storage/Makefile b/src/PVE/API2/Storage/Makefile
> index 1705080..11f3c95 100644
> --- a/src/PVE/API2/Storage/Makefile
> +++ b/src/PVE/API2/Storage/Makefile
> @@ -1,5 +1,5 @@
>
> -SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm Scan.pm FileRestore.pm
> +SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm Scan.pm FileRestore.pm MoveVolume.pm
>
> .PHONY: install
> install:
> diff --git a/src/PVE/API2/Storage/MoveVolume.pm b/src/PVE/API2/Storage/MoveVolume.pm
> new file mode 100644
> index 0000000..52447a4
> --- /dev/null
> +++ b/src/PVE/API2/Storage/MoveVolume.pm
> @@ -0,0 +1,61 @@
> +package PVE::API2::Storage::MoveVolume;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::RESTHandler;
> +use PVE::RPCEnvironment;
> +use PVE::Storage;
> +
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> + name => 'move',
> + path => '',
> + method => 'POST',
> + description => "Move a volume from one storage to another",
> + permissions => {
> + description => "You need the 'Datastore.Allocate' privilege on the storages.",
> + user => 'all',
> + },
> + protected => 1,
> + proxyto => 'node',
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + 'source-volume' => {
> + description => "Source volume",
> + type => 'string',
> + },
> + 'storage' => get_standard_option('pve-storage-id', {
> + completion => \&PVE::Storage::complete_storage_enabled,
> + description => 'Target storage',
> + }),
> + },
> + },
> + returns => { type => 'string' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $cfg = PVE::Storage::config();
> + my $source_volume = $param->{'source-volume'};
> + my $target_storeid = $param->{'storage'};
> +
> + my ($source_storeid, undef) = PVE::Storage::parse_volume_id($source_volume, 0);
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $authuser = $rpcenv->get_user();
> +
> + $rpcenv->check($authuser, "/storage/$source_storeid", ["Datastore.Allocate"]);
> + $rpcenv->check($authuser, "/storage/$target_storeid", ["Datastore.Allocate"]);
> +
> + my $worker = sub {
> + PVE::Storage::volume_move($cfg, $source_volume, $target_storeid);
> + };
> +
> + return $rpcenv->fork_worker('move_volume', '', $authuser, $worker);
> + }});
> +
> +1;
> diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
> index dc6cc69..6c816b7 100644
> --- a/src/PVE/API2/Storage/Status.pm
> +++ b/src/PVE/API2/Storage/Status.pm
> @@ -18,6 +18,7 @@ use PVE::Tools qw(run_command);
>
> use PVE::API2::Storage::Content;
> use PVE::API2::Storage::FileRestore;
> +use PVE::API2::Storage::MoveVolume;
> use PVE::API2::Storage::PruneBackups;
> use PVE::Storage;
>
> @@ -28,6 +29,11 @@ __PACKAGE__->register_method ({
> path => '{storage}/prunebackups',
> });
>
> +__PACKAGE__->register_method ({
> + subclass => "PVE::API2::Storage::MoveVolume",
> + path => '{storage}/move',
> +});
> +
> __PACKAGE__->register_method ({
> subclass => "PVE::API2::Storage::Content",
> # set fragment delimiter (no subdirs) - we need that, because volume
> @@ -233,6 +239,7 @@ __PACKAGE__->register_method ({
> { subdir => 'rrddata' },
> { subdir => 'status' },
> { subdir => 'upload' },
> + { subdir => 'move' },
> ];
>
> return $res;
> diff --git a/src/PVE/CLI/pvesm.pm b/src/PVE/CLI/pvesm.pm
> index 9b9676b..4c042aa 100755
> --- a/src/PVE/CLI/pvesm.pm
> +++ b/src/PVE/CLI/pvesm.pm
> @@ -20,6 +20,7 @@ use PVE::Storage;
> use PVE::Tools qw(extract_param);
> use PVE::API2::Storage::Config;
> use PVE::API2::Storage::Content;
> +use PVE::API2::Storage::MoveVolume;
> use PVE::API2::Storage::PruneBackups;
> use PVE::API2::Storage::Scan;
> use PVE::API2::Storage::Status;
> @@ -480,6 +481,37 @@ __PACKAGE__->register_method ({
> }
> });
>
> +__PACKAGE__->register_method ({
> + name => 'move',
> + path => 'move',
> + method => 'POST',
> + description => "Move a volume from one storage to another",
> + protected => 1,
> + proxyto => 'node',
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + 'source-volume' => {
> + description => "Source volume",
> + type => 'string',
> + },
> + 'storage' => get_standard_option('pve-storage-id', {
> + completion => \&PVE::Storage::complete_storage_enabled,
> + description => 'Target storage',
> + }),
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::API2::Storage::MoveVolume->move($param);
> +
> + return;
> + },
> +});
> +
> __PACKAGE__->register_method ({
> name => 'prunebackups',
> path => 'prunebackups',
> @@ -690,6 +722,7 @@ our $cmddef = {
> print "APIVER $res->{apiver}\n";
> print "APIAGE $res->{apiage}\n";
> }],
> + 'move' => [ __PACKAGE__, 'move', ['source-volume', 'storage'], { node => $nodename } ],
> 'prune-backups' => [ __PACKAGE__, 'prunebackups', ['storage'], { node => $nodename }, sub {
> my $res = shift;
>
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index f19a115..fe8a5e0 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -9,6 +9,7 @@ use IO::File;
> use IO::Socket::IP;
> use IPC::Open3;
> use File::Basename;
> +use File::Copy qw(move);
> use File::Path;
> use Cwd 'abs_path';
> use Socket;
> @@ -1620,14 +1621,20 @@ sub archive_info {
> }
>
> sub archive_remove {
> - my ($archive_path) = @_;
> + my ($archive_path, $ignore_protected) = @_;
> +
> + my $protection_path = protection_file_path($archive_path);
>
> die "cannot remove protected archive '$archive_path'\n"
> - if -e protection_file_path($archive_path);
> + if !$ignore_protected && -e $protection_path;
>
> unlink $archive_path or $! == ENOENT or die "removing archive $archive_path failed: $!\n";
>
> archive_auxiliaries_remove($archive_path);
> +
> + if (-e $protection_path) {
> + unlink $protection_path or $! == ENOENT or log_warn("Removing protection file failed: $!");
> + }
> }
>
> sub archive_auxiliaries_remove {
> @@ -1680,6 +1687,45 @@ sub extract_vzdump_config_tar {
> return wantarray ? ($raw, $file) : $raw;
> }
>
> +sub decompress_archive_into_pipe {
> + my ($archive, $cmd, $outfunc) = @_;
> +
> + my $info = archive_info($archive);
> + die "archive is not compressed\n" if !$info->{compression};
> + my $decompressor = $info->{decompressor};
> + my $full_cmd = [ [@$decompressor, $archive], $cmd ];
> +
> + # lzop/zcat exits with 1 when the pipe is closed early,
> + # detect this and ignore the exit code later
> + my $broken_pipe;
> + my $errstring;
> + my $err = sub {
> + my $output = shift;
> + if (
> + $output =~ m/lzop: Broken pipe: <stdout>/
> + || $output =~ m/gzip: stdout: Broken pipe/
> + || $output =~ m/zstd: error 70 : Write error.*Broken pipe/
> + ) {
> + $broken_pipe = 1;
> + } elsif (!defined ($errstring) && $output !~ m/^\s*$/) {
> + $errstring = "failed to decompress archive: $output\n";
> + }
> + };
> +
> + my $rc = eval { run_command($full_cmd, outfunc => $outfunc, errfunc => $err, noerr => 1) };
> + my $rerr = $@;
> +
> + $broken_pipe ||= $rc == 141; # broken pipe from cmd POV
> +
> + if (!$errstring && !$broken_pipe && $rc != 0) {
> + die "$rerr\n" if $rerr;
> + die "archive decompression failed with exit code $rc\n";
> + }
> + die "$errstring\n" if $errstring;
> +
> + return;
> +}
> +
> sub extract_vzdump_config_vma {
> my ($archive, $comp) = @_;
>
> @@ -1691,30 +1737,7 @@ sub extract_vzdump_config_vma {
> my $decompressor = $info->{decompressor};
>
> if ($comp) {
> - my $cmd = [ [@$decompressor, $archive], ["vma", "config", "-"] ];
> -
> - # lzop/zcat exits with 1 when the pipe is closed early by vma, detect this and ignore the exit code later
> - my $broken_pipe;
> - my $errstring;
> - my $err = sub {
> - my $output = shift;
> - if ($output =~ m/lzop: Broken pipe: <stdout>/ || $output =~ m/gzip: stdout: Broken pipe/ || $output =~ m/zstd: error 70 : Write error.*Broken pipe/) {
> - $broken_pipe = 1;
> - } elsif (!defined ($errstring) && $output !~ m/^\s*$/) {
> - $errstring = "Failed to extract config from VMA archive: $output\n";
> - }
> - };
> -
> - my $rc = eval { run_command($cmd, outfunc => $out, errfunc => $err, noerr => 1) };
> - my $rerr = $@;
> -
> - $broken_pipe ||= $rc == 141; # broken pipe from vma POV
> -
> - if (!$errstring && !$broken_pipe && $rc != 0) {
> - die "$rerr\n" if $rerr;
> - die "config extraction failed with exit code $rc\n";
> - }
> - die "$errstring\n" if $errstring;
> + decompress_archive_into_pipe($archive, ["vma", "config", "-"], $out);
> } else {
> run_command(["vma", "config", $archive], outfunc => $out);
> }
> @@ -1753,6 +1776,126 @@ sub extract_vzdump_config {
> }
> }
>
> +sub volume_move {
> + my ($cfg, $source_volid, $target_storeid) = @_;
> +
> + my ($source_storeid, $source_volname) = parse_volume_id($source_volid, 0);
> +
> + die "source and target storage cannot be the same\n" if ($source_storeid eq $target_storeid);
> +
> + activate_storage($cfg, $source_storeid);
> + my $source_scfg = storage_config($cfg, $source_storeid);
> + my $source_plugin = PVE::Storage::Plugin->lookup($source_scfg->{type});
> + my ($vtype) = $source_plugin->parse_volname($source_volname);
> +
> + die "source storage '$source_storeid' does not support volumes of type '$vtype'\n"
> + if !$source_scfg->{content}->{$vtype};
> +
> + activate_storage($cfg, $target_storeid);
> + my $target_scfg = storage_config($cfg, $target_storeid);
> + die "target storage '$target_storeid' does not support volumes of type '$vtype'\n"
> + if !$target_scfg->{content}->{$vtype};
> +
> + if ($vtype eq 'backup' || $vtype eq 'iso' || $vtype eq 'vztmpl' || $vtype eq 'snippets') {
> + my $target_plugin = PVE::Storage::Plugin->lookup($target_scfg->{type});
> +
> + die "moving a backup from a Proxmox Backup Server is not yet supported\n"
> + if ($vtype eq 'backup' && $source_scfg->{type} eq 'pbs');
> +
> + my $source_path = $source_plugin->filesystem_path($source_scfg, $source_volname);
> + die "$source_path does not exist" if (!-e $source_path);
> + my $source_dirname = dirname($source_path);
> +
> + return if $vtype ne 'backup';
> +
> + if ($target_scfg->{type} eq 'pbs') {
> + my $info = archive_info($source_path);
> + die "moving non-VMA backups to a Proxmox Backup Server is not yet supported\n"
> + if ($info->{format} ne 'vma');
> +
> + my $repo = PVE::PBSClient::get_repository($target_scfg);
> + my $vmid = ($source_plugin->parse_volname($source_volname))[2];
> + my $fingerprint = $target_scfg->{fingerprint};
> + my $password = PVE::Storage::PBSPlugin::pbs_password_file_name(
> + $target_scfg, $target_storeid);
> + my $namespace = $target_scfg->{namespace};
> + my $keyfile = PVE::Storage::PBSPlugin::pbs_encryption_key_file_name(
> + $target_scfg, $target_storeid);
> + my $master_keyfile = PVE::Storage::PBSPlugin::pbs_master_pubkey_file_name(
> + $target_scfg, $target_storeid);
> +
> + my $comp = $info->{compression};
> + my $backup_time = $info->{ctime};
> + my $log_file_path = "$source_dirname/$info->{logfilename}";
> + my $notes_file_path = "$source_dirname/$info->{notesfilename}";
> +
> + my $vma_to_pbs_cmd = [
> + "vma-to-pbs",
> + "--repository", $repo,
> + "--vmid", $vmid,
> + "--fingerprint", $fingerprint,
> + "--password-file", $password,
> + "--backup-time", $backup_time,
> + "--compress",
> + ];
> +
> + push @$vma_to_pbs_cmd, "--ns", $namespace if $namespace;
> + push @$vma_to_pbs_cmd, "--log-file", $log_file_path if -e $log_file_path;
> + push @$vma_to_pbs_cmd, "--notes-file", $notes_file_path if -e $notes_file_path;
> + push @$vma_to_pbs_cmd, "--encrypt", "--keyfile", $keyfile if -e $keyfile;
> + push @$vma_to_pbs_cmd, "--master-keyfile", $master_keyfile if -e $master_keyfile;
> +
> + if ($comp) {
> + decompress_archive_into_pipe($source_path, $vma_to_pbs_cmd);
> + } else {
> + push @$vma_to_pbs_cmd, $source_path;
> + run_command($vma_to_pbs_cmd);
> + }
> +
> + my $protection_source_path = protection_file_path($source_path);
> +
> + if (-e $protection_source_path) {
> + my $target_volid = PVE::Storage::PBSPlugin::print_volid(
> + $target_storeid, 'vm', $vmid, $backup_time);
> + my (undef, $target_volname) = parse_volume_id($target_volid, 0);
> + $target_plugin->update_volume_attribute(
> + $target_scfg, $target_storeid, $target_volname, 'protected', 1);
> + }
> +
> + archive_remove($source_path, 1);
> + } else {
> + my $target_path = $target_plugin->filesystem_path($target_scfg, $source_volname);
> +
> + move($source_path, $target_path) or die "failed to move $vtype: $!";
> +
> + my $target_dirname = dirname($target_path);
> + my $info = archive_info($source_path);
> +
> + for my $type (qw(log notes)) {
> + my $filename = $info->{"${type}filename"} or next;
> + $source_path = "$source_dirname/$filename";
> + $target_path = "$target_dirname/$filename";
> + move($source_path, $target_path) or die "moving backup $type file failed: $!"
> + if -e $source_path;
> + }
> +
> + my $protection_source_path = protection_file_path($source_path);
> +
> + if (-e $protection_source_path) {
> + my $protection_target_path = protection_file_path($target_path);
> + move($protection_source_path, $protection_target_path)
> + or die "moving backup protection file failed: $!";
> + }
> + }
> + } elsif ($vtype eq 'images') {
> + die "use pct move-volume or qm disk move\n";
> + } elsif ($vtype eq 'rootdir') {
> + die "cannot move OpenVZ rootdir\n";
> + }
> +
> + return;
> +}
> +
> sub prune_backups {
> my ($cfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc) = @_;
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH storage] add API method to move a volume between storages
2024-06-12 14:45 [pve-devel] [PATCH storage] add API method to move a volume between storages Filip Schauer
2024-06-12 14:47 ` Filip Schauer
@ 2024-06-14 17:29 ` Thomas Lamprecht
2024-06-25 14:55 ` Filip Schauer
1 sibling, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2024-06-14 17:29 UTC (permalink / raw)
To: Proxmox VE development discussion, Filip Schauer
some higher level review:
subject could mention CLI too, e.g.:
api, cli: implement moving a volume between storages
Am 12/06/2024 um 16:45 schrieb Filip Schauer:
> Add the ability to move a backup, ISO, container template or snippet
> between storages of a node via an API method. Moving a VMA backup to a
> Proxmox Backup Server requires the proxmox-vma-to-pbs package to be
> installed. Currently only VMA backups can be moved to a Proxmox Backup
> Server and moving backups from a Proxmox Backup Server is not yet
> supported.
Interesting integration of the vma-to-pbs importer! I'd proably make this
copy by default and do the remove-source-volume-on-success only through an
opt-in parameter, similar to how the similar, but more specialized, "move
guest disk/volume" works.
>
> The method can be called from the PVE shell with `pvesm move`:
>
> # pvesm move <source volume> <target storage>
For the command and the API endpoint path I'd prefer using 'move-volume' as
name. That would clarify what the subject is, and not imply that one could
move a whole storage, a feature which some users also might want as separate
feature.
Speaking about API paths, in theory mounting this below the
`/nodes/{node}/storage/{storage}/content/{volume}/` path would be quite a
bit nicer. But, currently that's not trivially possible as we already use
the GET endpoint of that path for getting the volumes attributes.
So here we have three choices:
1 decide that it's better to keep it as a separate path at the {storage}
top-level (or even if we don't agree on that being better, just close
our eyes and look away).
Can be OK, but as we face this pain every once in a while I'd at least
think about the alternatives below, even though they would extend scope
quite a bit.
2 try to use the existing structure, which might need some adaptions in
the router/schema stuff, as we essentially would need to learn to
pull the list of API subdirectories out of a nested variable inside
the object. As currently API directory indexes use a top-level array
so any API endpoint that returns an object at the top-level cannot
be really extended to also host API endpoints in subdirectories.
If we could specify in the schema that the href array is returned in
a sub-property we could add deeper links much more easily in the
future.
Here the most important thing would be to check potential negative
drawbacks of doing so, and also how much work it would be to add
that functionality (I did not check, gut feeling tells me that it
probably is not _that_ much, as it should be mostly adding a check
about getting the same info from another location).
3 using a new API path, e.g. doing s/content/volume/ where the GET
endpoint is a router index and the "metadata" one gets provided
inside that API path folder. Then we could slowly deprecate the old
one and be done.
What, and if, those options make sense can be debatable. IMO checking
out 2 could be good as we run into this "problem" every once in a while,
and having the possibility of 2 would avoid shuffling API paths around
just to be able to do deeper nesting.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH storage] add API method to move a volume between storages
2024-06-14 17:29 ` Thomas Lamprecht
@ 2024-06-25 14:55 ` Filip Schauer
0 siblings, 0 replies; 4+ messages in thread
From: Filip Schauer @ 2024-06-25 14:55 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
Superseded by:
https://lists.proxmox.com/pipermail/pve-devel/2024-June/064283.html
Ended up moving the API method to the POST endpoint at
/nodes/{node}/storage/{storage}/content/{volume}, replacing the
experimental copy method.
On 14/06/2024 19:29, Thomas Lamprecht wrote:
> some higher level review:
>
> subject could mention CLI too, e.g.:
>
> api, cli: implement moving a volume between storages
>
> Am 12/06/2024 um 16:45 schrieb Filip Schauer:
>> Add the ability to move a backup, ISO, container template or snippet
>> between storages of a node via an API method. Moving a VMA backup to a
>> Proxmox Backup Server requires the proxmox-vma-to-pbs package to be
>> installed. Currently only VMA backups can be moved to a Proxmox Backup
>> Server and moving backups from a Proxmox Backup Server is not yet
>> supported.
> Interesting integration of the vma-to-pbs importer! I'd proably make this
> copy by default and do the remove-source-volume-on-success only through an
> opt-in parameter, similar to how the similar, but more specialized, "move
> guest disk/volume" works.
>
>> The method can be called from the PVE shell with `pvesm move`:
>>
>> # pvesm move <source volume> <target storage>
> For the command and the API endpoint path I'd prefer using 'move-volume' as
> name. That would clarify what the subject is, and not imply that one could
> move a whole storage, a feature which some users also might want as separate
> feature.
>
> Speaking about API paths, in theory mounting this below the
> `/nodes/{node}/storage/{storage}/content/{volume}/` path would be quite a
> bit nicer. But, currently that's not trivially possible as we already use
> the GET endpoint of that path for getting the volumes attributes.
>
> So here we have three choices:
>
> 1 decide that it's better to keep it as a separate path at the {storage}
> top-level (or even if we don't agree on that being better, just close
> our eyes and look away).
> Can be OK, but as we face this pain every once in a while I'd at least
> think about the alternatives below, even though they would extend scope
> quite a bit.
>
> 2 try to use the existing structure, which might need some adaptions in
> the router/schema stuff, as we essentially would need to learn to
> pull the list of API subdirectories out of a nested variable inside
> the object. As currently API directory indexes use a top-level array
> so any API endpoint that returns an object at the top-level cannot
> be really extended to also host API endpoints in subdirectories.
> If we could specify in the schema that the href array is returned in
> a sub-property we could add deeper links much more easily in the
> future.
> Here the most important thing would be to check potential negative
> drawbacks of doing so, and also how much work it would be to add
> that functionality (I did not check, gut feeling tells me that it
> probably is not _that_ much, as it should be mostly adding a check
> about getting the same info from another location).
>
> 3 using a new API path, e.g. doing s/content/volume/ where the GET
> endpoint is a router index and the "metadata" one gets provided
> inside that API path folder. Then we could slowly deprecate the old
> one and be done.
>
> What, and if, those options make sense can be debatable. IMO checking
> out 2 could be good as we run into this "problem" every once in a while,
> and having the possibility of 2 would avoid shuffling API paths around
> just to be able to do deeper nesting.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-06-25 14:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-12 14:45 [pve-devel] [PATCH storage] add API method to move a volume between storages Filip Schauer
2024-06-12 14:47 ` Filip Schauer
2024-06-14 17:29 ` Thomas Lamprecht
2024-06-25 14:55 ` Filip Schauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox