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

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Apr 01, 2025 at 11:40:14AM +0200, Fiona Ebner wrote:
> 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.

Actually, 'btrfs' format for btrfs-send/recv uses the snapshot name in
both import & export cases:
- We receive into a temporary directory
- BTRFS does not allow moving read-only snapshots into different
  directories, but we need the final one to also be the current writable
  state, so:
  - We remove the read-only flag.
  - We exchange it with the current sate.
  - We take a "snapshot" to reproduce the that last `$snapshot`.
  - We clean up.

It's a bit awkward. But that's just BTRFS...


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