Re: [pve-devel] [PATCH v1 pve-storage 8/8] pluginbase: document import and export methods

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

Am 01.04.25 um 10:40 schrieb Fabian Grünbichler:
> On March 26, 2025 3:20 pm, Max Carrara wrote:
>> +=head3 $plugin->volume_export(\%scfg, $storeid, $fh, $volname, $format [, $snapshot, $base_snapshot, $with_snapshots])
>> +
>> +=head3 $plugin->volume_export(...)
>> +
>> +Exports a volume or a volume's C<$snapshot> into a file handle C<$fh> as a
>> +stream with a desired export C<$format>. See L<FORMATS> for all import/export
>> +formats.
>> +
>> +Optionally, C<$snapshot> (if provided) may have a C<$base_snapshot>, and
>> +C<$with_snapshots> states whether the volume has snapshots overall.
> 
> this is incomplete/wrong
> 
> $with_snapshots means the export should include snapshots, not whether
> the volume has snapshots..
> $snapshot means "this is the snapshot to export" if only exporting the
> snapshot ($with_snapshots == 0), or "this is the *last* snapshot export"
> if exporting a stream of snapshots ($with_snapshots == 1)
> $base_snapshot means "this is the start of the snapshot range to export"
> (i.e., do an incremental export on top of this base)
> 
> this is mostly only relevant for zfs at the moment (other storages can
> either export a volume including its snapshots, or just the volume, but
> no complicated incremental streams of snapshots), but will change once
> we implement replication export/import for other storages..

There are already ideas floating around to change this and add proper
format negotiation. We'll also need to ask the target what it supports
like for remote migration, the sending side cannot really know that. And
as part of that change from the confusing set of snapshot-related
parameters to having an actual "what kind of transport" enum:
1. current data (of an image or a snapshot)
2. full sync with all snapshots
3. incremental stream
No details worked out yet though and not really relevant for documenting
the status quo.

>> +Optionally, C<$snapshot> (if provided) may have a C<$base_snapshot>, and
>> +C<$with_snapshots> states whether the volume has snapshots overall. Renaming an
>> +existing volume may also optionally be allowed via C<$allow_rename>
> 
> see above, but here $snapshot is mainly there to have the same
> arguments for volume_import_formats so a plugin can have a single
> implementation, not because it is used anywhere IIRC..

The LVMPlugin.pm and Plugin.pm do have different implementations of the
volume_{export,import}_formats() methods, precisely because they need to
ignore the $snapshot parameter for the import case. Yes, we do pass the
same arguments, but it can only matter for the "incremental stream"
scenario. Otherwise, the parameter has nothing to do with the import
side. Here it would also be much nicer to have an actual "what kind of
transport" parameter.


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