public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Filip Schauer <f.schauer@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH storage] add API method to move a volume between storages
Date: Tue, 25 Jun 2024 16:55:12 +0200	[thread overview]
Message-ID: <ad0c51cf-a617-438c-b71a-aff7b172f745@proxmox.com> (raw)
In-Reply-To: <8464a86e-0006-4e07-90f0-e59008d7aac4@proxmox.com>

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


      reply	other threads:[~2024-06-25 14:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-12 14:45 Filip Schauer
2024-06-12 14:47 ` Filip Schauer
2024-06-14 17:29 ` Thomas Lamprecht
2024-06-25 14:55   ` Filip Schauer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ad0c51cf-a617-438c-b71a-aff7b172f745@proxmox.com \
    --to=f.schauer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal