Re: [pve-devel] [RFC storage v2 10/25] plugin: introduce new_backup_provider() method

[Fiona Ebner <f.ebner@proxmox.com>]

Am 12.09.24 um 14:43 schrieb Fabian Grünbichler:
> On August 13, 2024 3:28 pm, Fiona Ebner wrote:
>>
>> Container mechanism 'directory':
>>
>> The backup provider gives the path to a directory with the full
>> filesystem structure of the container.
>>
>> Container mechanism 'directory':
>>
>> The backup provider gives the path to a (potentially compressed) tar
>> archive with the full filesystem structure of the container.
> 
> same as in the cover letter ;) base on the code here I assume the second
> one should be tar. I wonder whether just tar wouldn't be enough (easier
> to not get ACLs/xattrs/ownership/.. right)?
> 

Yes, should be tar, will fix!

I'd guess it is more convenient for many providers to expose (a FUSE
mount of) a directory. Using tar can mean more work. E.g. with Borg,
mounting the archive seems much cheaper than creating the tar. It's also
that the archive looks like:
guest.config
firewall.config
filesystem/
and while borg has "export-tar" where one can specify specific paths,
the tar will still contain the "filesystem/" prefix. Not sure if there
is an easy way to get rid of that.

>> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
>> index 6444390..d5b76ae 100644
>> --- a/src/PVE/Storage/Plugin.pm
>> +++ b/src/PVE/Storage/Plugin.pm
>> @@ -1755,6 +1755,21 @@ sub rename_volume {
>>      return "${storeid}:${base}${target_vmid}/${target_volname}";
>>  }
>>  
>> +# Used by storage plugins for external backup providers. See PVE::BackupProvider::Plugin for the API
>> +# the provider needs to implement.
>> +#
>> +# $scfg - the storage configuration
>> +# $storeid - the storage ID
>> +# $log_function($log_level, $message) - this log function can be used to write to the backup task
>> +#   log in Proxmox VE. $log_level is 'info', 'warn' or 'err', $message is the message to be printed.
>> +#
>> +# Returns a blessed reference to the backup provider class.
>> +sub new_backup_provider {
>> +    my ($class, $scfg, $storeid, $log_function) = @_;
>> +
>> +    return;
>> +}
> 
> would it maybe make sense to make this a "die implement me" and make the
> opt-in via the storage plugin features? it would be more in line with
> what we do in other parts and less subtle..
> 

We don't have a method for storage plugin features yet, only
volume_has_feature() and the stand-alone storage_can_replicate(). We
could generalize (and deprecate) the latter though.


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