Re: [pve-devel] [PATCH v1 pve-storage 1/8] pluginbase: introduce PVE::Storage::PluginBase with doc scaffold

["Max Carrara" <m.carrara@proxmox.com>]

On Thu Apr 3, 2025 at 9:12 AM CEST, Fabian Grünbichler wrote:
> On April 2, 2025 6:31 pm, Max Carrara wrote:
> > On Mon Mar 31, 2025 at 5:13 PM CEST, Fabian Grünbichler wrote:
> >> On March 26, 2025 3:20 pm, Max Carrara wrote:
> >> > +=head2 HOOKS
> >> > +
> >> > +=cut
> >> > +
> >> > +# called during addition of storage (before the new storage config got written)
> >>
> >> called when adding a storage config entry, before the new config gets written
> >>
> >> > +# die to abort addition if there are (grave) problems
> >> > +# NOTE: runs in a storage config *locked* context
> >> > +sub on_add_hook {
> >> > +    my ($class, $storeid, $scfg, %param) = @_;
> >> > +    return undef;
> >> > +}
> >> > +
> >> > +# called during storage configuration update (before the updated storage config got written)
> >>
> >> called when updating a storage config entry, before the updated config
> >> gets written
> >>
> >> > +# die to abort the update if there are (grave) problems
> >> > +# NOTE: runs in a storage config *locked* context
> >> > +sub on_update_hook {
> >> > +    my ($class, $storeid, $scfg, %param) = @_;
> >> > +    return undef;
> >> > +}
> >> > +
> >> > +# called during deletion of storage (before the new storage config got written)
> >> > +# and if the activate check on addition fails, to cleanup all storage traces
> >> > +# which on_add_hook may have created.
> >>
> >> called when deleting a storage config entry, before the new storage
> >> config gets written.
> >>
> >> also called as part of error handling when undoing the addition of a new
> >> storage config entry.
> > 
> > Regarding your three responses above: The comments here were preserved
> > from `::Plugin` for context's sake. But tbh, on second thought, they can
> > probably just be removed, as they'll be replaced by POD anyways.
> > 
>
> yes, but those parts mor or less made it to the POD as well slightly
> rephrased, hence I called them out here already :-P

Fair :P

>
> >>
> >> > +# die to abort deletion if there are (very grave) problems
> >> > +# NOTE: runs in a storage config *locked* context
> >> > +sub on_delete_hook {
> >> > +    my ($class, $storeid, $scfg) = @_;
> >> > +    return undef;
> >> > +}
> >> > +
> >> > +=head2 IMAGE OPERATIONS
> >> > +
> >>
> >> should this describe what IMAGES are in the context of PVE? else as a
> >> newcomer the difference between IMAGE here and VOLUME below might not
> >> be clear..
> > 
> > Yeah, that's a good idea. Maximiliano and I were also thinking about
> > maybe adding a GLOSSARY section at the bottom of the file where certain
> > terms could be explained / defined in more detail in general.
> > What do you think?
> > 
> > Alternatively, we could also have the top-level description define the
> > most basic of terms, but I don't want to load the docs here with too
> > much information up front.
>
> I think it depends - things that are not only interesting for plugin
> devs should probably go into docs proper and just be referenced here. I
> don't think the distinction between volume and image is very important
> outside of development, so that might be explained here?
>
> the POD documentation here is targeted at us and plugin devs, so that
> should be the guideline when deciding what goes where - and what goes in
> in the first place - I think a focus on "what do I need to know/be aware
> of when writing a plugin" (or adapting the interface) should be the
> guiding principle for that.
>
> e.g. for that knowing that this parameter is optional doesn't buy me
> much. knowing what it means if it is set does (or that I can ignore it
> if my storage can't do X, or should die if set in that case)

Yeah that's basically what we've tried to follow as much as possible
here; just wasn't sure *how much* information is okay. For now I'll keep
it slim and just exlpain the differences between images and volumes and
*perhaps* any other terms that might come up, but only if they're of
immediate relevancy to the plugin API.

Though, I'll add the glossary idea to my backlog; might be something
that could be useful for the wiki / docs in general.

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



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