public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Roland Kammerer via pve-devel <pve-devel@lists.proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: Roland Kammerer <roland.kammerer@linbit.com>
Subject: Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]
Date: Fri, 7 Mar 2025 09:24:25 +0100	[thread overview]
Message-ID: <mailman.876.1741335877.293.pve-devel@lists.proxmox.com> (raw)
In-Reply-To: <D81GW402I9IT.2I6IX7N77DSJQ@proxmox.com>

[-- Attachment #1: Type: message/rfc822, Size: 9437 bytes --]

From: Roland Kammerer <roland.kammerer@linbit.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]
Date: Fri, 7 Mar 2025 09:24:25 +0100
Message-ID: <Z8qtOTycFZpN5bpL@arm64>

Hi Max,

took me a bit longer than expected, but here we go...

On Tue, Feb 25, 2025 at 11:50:31AM +0100, Max Carrara wrote:
> Thanks a lot for the offer! I do actually have a couple questions. It
> would be nice if you could answer them, as it would aid in cleaning all
> this up, but please don't feel like you have to, of course!
> 
> Note that I can't guarantee that everything will be incorporated into
> the code of course, but I still wanted to reach out, as I'm in the
> process of sorting all of this out.
> 
> 1. Which parts of the plugin API (specifically PVE::Storage::Plugin)
>    are hard to grasp / work with?

At the time I took over maintainership resources like [1] did not exist
and I remember that it was hard to know which parts of the plugin/API
are mandatory and which are not. I was lucky enough that the predecessor
was maintained by Proxmox folks and I got mainly away with some cargo
culting existing plugins/code.

What I also remember as a bit strange was the use of the cache some
functions get passed in. Like what is "my" namespace? Is it cached
forever or what is the lifetime? Is it per "run" and what is a "run". Is
it per plugin type?... You get the idea.

What also took me quite some time to get is that there are differences
in offline and online migration and that one might work and the other
fails. For example qemu move disk is very very picky when it comes to
src vs. dst sizes. Might be worth to document what is used behind the
scene in which scenario.

> 
> 2. Since you've been working with the code for a while, do you have any
>    improvement suggestions for the API? If so, which?
>    (Note that by that I don't mean new features and such, but rather
>    improvements to the API as a whole -- the subroutines it consists of
>    etc.)
> 
> 3. Are there any parts of the API that you would change? If so, which?

What always felt a bit strange was that snapshots are mixed into
functions as parameters and then one always has to if-then-else split
based on if it is a snapshot or not. That obviously only is the case if
it is not the other way around and there are dedicated *_snapshot_*
functions well :). That is a bit weird and mixed and "for historical
reasons".

Maybe the cache should be a bit more powerful? I don't know if other
plugins run into that problem but we have a central component (the
LINSTOR controller) that gets queried for status information via http.
The cache as is helps, but if you have a certain amount of nodes it
still gets hammered too much. That is why I had to implement some
persistent cache stored in the file system with a longer lifetime. We
might be special in that regard, don't know, but might be worth
considering.

> 4. Do you think you would benefit from having (API) subroutines
>    documented via docstrings?

Yes, I think so.

What also could be nice is to extensively document one of the shipped
plugins and point to that in the documentation as kind of a show case.
Usually when I'm new to some platform and need to implement a plugin for
our stuff I look into how they did it for LVM/LVM thin. Might be a good
candidate for such an effort.

> 5. Is there any kind of "tooling" that you'd like to have, which would
>    aid you with plugin development? By that I mean things like being
>    able to check if your plugin conforms to the current API version and
>    such.

Some kind of "test suite" would be great. I have a "simple" shell script
where one sets PVE node information, which storage to use,... and then
it creates volumes, deletes them, creates VMs and migrates them (live
and offline), creates snapshots and restores them and so on. While there
will always be plugin specific tests, having something like that
available would be great. Probably it already exists (Proxmox
internally).

> 6. Is there any other things you'd like to mention? Feedback, critique
>    and such are all welcome!

Just that the documentation in general is very good, the "how to submit
patches" page is excellent for example. Also the ML IMO works very well,
it always has been a good and helpful experience.

Best, rck

[1] https://pve.proxmox.com/wiki/Storage_Plugin_Development


[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

  reply	other threads:[~2025-03-07  8:24 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-07  9:02 Roland Kammerer via pve-devel
2025-02-07  9:18 ` Fabian Grünbichler
2025-02-07 12:34   ` Roland Kammerer via pve-devel
2025-02-12 10:57     ` Fabian Grünbichler
     [not found]   ` <Z6X9vOv2lCsvTy3o@arm64>
2025-02-25 10:50     ` Max Carrara
2025-03-07  8:24       ` Roland Kammerer via pve-devel [this message]
     [not found]       ` <Z8qtOTycFZpN5bpL@arm64>
2025-03-07 12:01         ` Roland Kammerer via pve-devel
2025-03-12 11:09         ` Max Carrara
2025-02-07  9:19 ` Fiona Ebner

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=mailman.876.1741335877.293.pve-devel@lists.proxmox.com \
    --to=pve-devel@lists.proxmox.com \
    --cc=roland.kammerer@linbit.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