[pve-devel] storage plugins: what is plugindata()->{content}[1]

[pve-devel] storage plugins: what is plugindata()->{content}[1]

[Roland Kammerer via pve-devel]

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

From: Roland Kammerer <roland.kammerer@linbit.com>
To: pve-devel@lists.proxmox.com
Subject: storage plugins: what is plugindata()->{content}[1]
Date: Fri, 7 Feb 2025 10:02:04 +0100
Message-ID: <Z6XMDGbw9qml59Jh@arm64>

Hi all,

rather simple question I guess, but I could not find the answer in
https://pve.proxmox.com/wiki/Storage_Plugin_Development and linked
documents and looks like my grep foo is lacking today, so here we go:
I guess I know what the first hash in the 'content' array is, but what
is the second? Actually, why is there a second one?

Storage/LVMPlugin.pm:
content => [ {images => 1, rootdir => 1}, { images => 1 }]

vs.

Storage/LvmThinPlugin.pm:
content => [ {images => 1, rootdir => 1}, { images => 1, rootdir => 1}]

Best, rck


[-- 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

Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]

[Fabian Grünbichler]

> Roland Kammerer via pve-devel <pve-devel@lists.proxmox.com> hat am 07.02.2025 10:02 CET geschrieben:
> Hi all,
> 
> rather simple question I guess, but I could not find the answer in
> https://pve.proxmox.com/wiki/Storage_Plugin_Development and linked
> documents and looks like my grep foo is lacking today, so here we go:
> I guess I know what the first hash in the 'content' array is, but what
> is the second? Actually, why is there a second one?
> 
> Storage/LVMPlugin.pm:
> content => [ {images => 1, rootdir => 1}, { images => 1 }]
> 
> vs.
> 
> Storage/LvmThinPlugin.pm:
> content => [ {images => 1, rootdir => 1}, { images => 1, rootdir => 1}]
> 
> Best, rck

the first one defines the allowed/valid content types, the second the default one(s):

https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/Storage/Plugin.pm;h=65cf43f7fa32860f709fd8a4568f0ff346dc3d12;hb=HEAD#l544

we are working on improving the docs and cleaning all of this up!

Fabian


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

Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]

[Roland Kammerer via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 6735 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 Feb 2025 13:34:04 +0100
Message-ID: <Z6X9vOv2lCsvTy3o@arm64>

On Fri, Feb 07, 2025 at 10:18:42AM +0100, Fabian Grünbichler wrote:
> 
> > Roland Kammerer via pve-devel <pve-devel@lists.proxmox.com> hat am 07.02.2025 10:02 CET geschrieben:
> > Hi all,
> > 
> > rather simple question I guess, but I could not find the answer in
> > https://pve.proxmox.com/wiki/Storage_Plugin_Development and linked
> > documents and looks like my grep foo is lacking today, so here we go:
> > I guess I know what the first hash in the 'content' array is, but what
> > is the second? Actually, why is there a second one?
> > 
> > Storage/LVMPlugin.pm:
> > content => [ {images => 1, rootdir => 1}, { images => 1 }]
> > 
> > vs.
> > 
> > Storage/LvmThinPlugin.pm:
> > content => [ {images => 1, rootdir => 1}, { images => 1, rootdir => 1}]
> > 
> > Best, rck
> 
> the first one defines the allowed/valid content types, the second the default one(s):

thanks Fabian and Fiona, that explains it.

> we are working on improving the docs and cleaning all of this up!

that is great news, even after maintaining a plugin for some time it
usually takes a deep dive into some core code to actually understand
some of the API. If there is then something to review or give feedback
from an external plugin dev's point of view feel free to explicitly ping
me.

Best, rck


[-- 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

Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]

[Fabian Grünbichler]

> Roland Kammerer via pve-devel <pve-devel@lists.proxmox.com> hat am 07.02.2025 13:34 CET geschrieben:
> On Fri, Feb 07, 2025 at 10:18:42AM +0100, Fabian Grünbichler wrote:
> > 
> > > Roland Kammerer via pve-devel <pve-devel@lists.proxmox.com> hat am 07.02.2025 10:02 CET geschrieben:
> > > Hi all,
> > > 
> > > rather simple question I guess, but I could not find the answer in
> > > https://pve.proxmox.com/wiki/Storage_Plugin_Development and linked
> > > documents and looks like my grep foo is lacking today, so here we go:
> > > I guess I know what the first hash in the 'content' array is, but what
> > > is the second? Actually, why is there a second one?
> > > 
> > > Storage/LVMPlugin.pm:
> > > content => [ {images => 1, rootdir => 1}, { images => 1 }]
> > > 
> > > vs.
> > > 
> > > Storage/LvmThinPlugin.pm:
> > > content => [ {images => 1, rootdir => 1}, { images => 1, rootdir => 1}]
> > > 
> > > Best, rck
> > 
> > the first one defines the allowed/valid content types, the second the default one(s):
> 
> thanks Fabian and Fiona, that explains it.
> 
> > we are working on improving the docs and cleaning all of this up!
> 
> that is great news, even after maintaining a plugin for some time it
> usually takes a deep dive into some core code to actually understand
> some of the API. If there is then something to review or give feedback
> from an external plugin dev's point of view feel free to explicitly ping
> me.

thank you for that offer, it is appreciated!
Fabian


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

Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]

[Max Carrara]

On Fri Feb 7, 2025 at 1:34 PM CET, Roland Kammerer wrote:
> On Fri, Feb 07, 2025 at 10:18:42AM +0100, Fabian Grünbichler wrote:
> > 
> > > Roland Kammerer via pve-devel <pve-devel@lists.proxmox.com> hat am 07.02.2025 10:02 CET geschrieben:
> > > Hi all,
> > > 
> > > rather simple question I guess, but I could not find the answer in
> > > https://pve.proxmox.com/wiki/Storage_Plugin_Development and linked
> > > documents and looks like my grep foo is lacking today, so here we go:
> > > I guess I know what the first hash in the 'content' array is, but what
> > > is the second? Actually, why is there a second one?
> > > 
> > > Storage/LVMPlugin.pm:
> > > content => [ {images => 1, rootdir => 1}, { images => 1 }]
> > > 
> > > vs.
> > > 
> > > Storage/LvmThinPlugin.pm:
> > > content => [ {images => 1, rootdir => 1}, { images => 1, rootdir => 1}]
> > > 
> > > Best, rck
> > 
> > the first one defines the allowed/valid content types, the second the default one(s):
>
> thanks Fabian and Fiona, that explains it.
>
> > we are working on improving the docs and cleaning all of this up!
>
> that is great news, even after maintaining a plugin for some time it
> usually takes a deep dive into some core code to actually understand
> some of the API. If there is then something to review or give feedback
> from an external plugin dev's point of view feel free to explicitly ping
> me.
>
> Best, rck

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?

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?

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

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.

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

Also, should something pop up in the future that you'd like to mention,
please feel free to ping me here on the mailing list and let me know
what it is (: I'm always grateful for any feedback!

Thanks a lot in advance for your time!



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

Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]

[Roland Kammerer via pve-devel]

[-- 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

Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]

[Roland Kammerer via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 5750 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 13:01:07 +0100
Message-ID: <Z8rgA0IjDN1v5_fN@arm64>

On Fri, Mar 07, 2025 at 09:24:25AM +0100, Roland Kammerer wrote:
> On Tue, Feb 25, 2025 at 11:50:31AM +0100, Max Carrara wrote:
> > 6. Is there any other things you'd like to mention? Feedback, critique
> >    and such are all welcome!

something I forgot: the last time I checked proper integration in the
GUI storage section (i.e., adding storage entries to storage.cfg) was
not possible for out of tree plugins. Work in that area would be highly
appreciated.

Regards, rck


[-- 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

Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]

[Max Carrara]

On Fri Mar 7, 2025 at 9:24 AM CET, Roland Kammerer wrote:
> Hi Max,
>
> took me a bit longer than expected, but here we go...
>

Thanks a lot for your detailed feedback, it's highly appreciated!

You'll probably be delighted that a couple of the things you mentioned
are already in progress:

- We're in the process of documenting the PVE::Storage::Plugin API; I'll
  also see if I can address some of your points there already. We can't
  change the API right away of course, so I'd personally rather have
  those "historical" things documented. This will also include what's
  mandatory to implement, what's optional, what you shouldn't override
  at all, etc. Also the specific cases in which you'd probably want to
  implement (or not implement something).

- There's also an example storage plugin we're currently writing, which
  basically does as you suggested. What I have in mind there is that
  we'd show you how to build that specific plugin step-by-step while
  also providing the complete source of the final plugin as well as a
  package so you can just install it and see it in action, too.

API changes of course aren't as easy / straightforward to make, and
there's also the question of whether they'd actually be worth it
overall. But, I've nevertheless noted those things down -- right now the
documentation is a much higher priority, though. At least from my side.

The testing suite you mentioned sounds interesting; I'll add that to my
backlog as well and give it some ponderment. If you have any concrete
ideas regarding this, feel free to toss them my way!

Last but not least, the GUI integration is a very good point. I can't
promise anything in that regard yet, but I'm also taking note here.
Definitely something that can be improved.

Thanks again for your detailed feedback, it's really valuable! If there
are ever any other issues you come across, feel free to let me know here
in this thread. I have taken note of everything you said.



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

Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]

[Fiona Ebner]

Am 07.02.25 um 10:02 schrieb Roland Kammerer via pve-devel:
> Hi all,
> 
> rather simple question I guess, but I could not find the answer in
> https://pve.proxmox.com/wiki/Storage_Plugin_Development and linked
> documents and looks like my grep foo is lacking today, so here we go:
> I guess I know what the first hash in the 'content' array is, but what
> is the second? Actually, why is there a second one?
> 
> Storage/LVMPlugin.pm:
> content => [ {images => 1, rootdir => 1}, { images => 1 }]
> 
> vs.
> 
> Storage/LvmThinPlugin.pm:
> content => [ {images => 1, rootdir => 1}, { images => 1, rootdir => 1}]
> 
> Best, rck

Hi,

the first entry is all valid content types that the plugin supports, the
second entry is the fallback for the storage configuration, i.e. if the
storage configuration for a specific storage does not explicitly specify
a 'content' option, those will be used/allowed. See the parse_config()
method in Plugin.pm.

Best Regards,
Fiona


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