[pve-devel] [PATCH storage] add API method to move a volume between storages

Thomas Lamprecht t.lamprecht at proxmox.com
Fri Jun 14 19:29:03 CEST 2024


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.




More information about the pve-devel mailing list