[pve-devel] [PATCH storage v5 09/32] plugin: introduce new_backup_provider() method

Wolfgang Bumiller w.bumiller at proxmox.com
Thu Mar 27 12:03:09 CET 2025


On Tue, Mar 25, 2025 at 01:50:20PM +0100, Fiona Ebner wrote:
> Am 24.03.25 um 16:43 schrieb Wolfgang Bumiller:
> > Just a short high level nit today, will have to look more closely at
> > this and the series the next days:
> > 
> > There's a `new()` which takes an $scfg + $storeid.
> > 
> > But later there are some methods taking `$self` (which usually means the
> > thing returned from `new()`), which also get a `$storeid` as additional
> > parameter (but without any `$scfg`). IMO the `$storeid` should be
> > dropped there.
> 
> Nice catch! Yeah, I think that was an oversight when I restructured in
> an earlier version. In fact, my example implementations of those
> functions even use $self->{storeid} already (or don't require the
> storeid at all). I'll remove those left-overs in v6.

Two more small things:

The `restore_get_{guest,firewall}_config` docs should probably
specifically mention that these are independent queries and called
without any `restore_*_init()` calls.

Thinking about this more, maybe they should be renamed.
It might be nicer to have the `restore_` prefix used only for restore
processes, and rename these to `archive_get_*_config()`?
These are also used to view the config in the UI (or at least the
`get_geust_config` one is also called from `Storage::extract_vzdump_config()`).




More information about the pve-devel mailing list