Re: [pve-devel] [PATCH storage] add API method to move a volume between storages
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
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
Re: [pve-devel] [PATCH storage] add API method to move a volume between storages
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
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
Re: [pve-devel] [PATCH storage] add API method to move a volume between storages
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
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 "$(
---
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 000..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 => \::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-devel] [PATCH storage] add API method to move a volume between storages
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
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 "$(
---
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 000..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 => \::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
