[pve-devel] [PATCH storage 4/5] lvmthin: import/export: implement snapshot exporting
Dietmar Maurer
dietmar at proxmox.com
Thu Jun 22 07:19:32 CEST 2017
question inline:
> On June 21, 2017 at 2:59 PM Wolfgang Bumiller <w.bumiller at proxmox.com> wrote:
>
>
> ---
> PVE/Storage/LVMPlugin.pm | 11 ++++++++---
> PVE/Storage/LvmThinPlugin.pm | 15 +++++++++++++++
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
> index 9633e4c..ed8afa1 100644
> --- a/PVE/Storage/LVMPlugin.pm
> +++ b/PVE/Storage/LVMPlugin.pm
> @@ -509,15 +509,14 @@ sub volume_export_formats {
> return volume_import_formats($class, $scfg, $storeid, $volname,
> $base_snapshot, $with_snapshots);
> }
>
> -sub volume_export {
> +sub __volume_export {
> my ($class, $scfg, $storeid, $fh, $volname, $format, $snapshot,
> $base_snapshot, $with_snapshots) = @_;
> die "volume export format $format not available for $class\n"
> if $format ne 'raw+size';
> die "cannot export volumes together with their snapshots in $class\n"
> if $with_snapshots;
> - die "cannot export a snapshot in $class\n" if defined($snapshot);
> die "cannot export an incremental stream in $class\n" if
> defined($base_snapshot);
> - my $file = $class->path($scfg, $volname, $storeid);
> + my $file = $class->path($scfg, $volname, $storeid, $snapshot);
> my $size;
> # should be faster than querying LVM, also checks for the device file's
> availability
> run_command(['/sbin/blockdev', '--getsize64', $file], outfunc => sub {
> @@ -529,6 +528,12 @@ sub volume_export {
> run_command(['dd', "if=$file", "bs=64k"], output => '>&'.fileno($fh));
> }
When do we need this functionality (exporting a single snapshot)?
> +sub volume_export {
> + my ($class, $scfg, $storeid, $fh, $volname, $format, $snapshot,
> $base_snapshot, $with_snapshots) = @_;
> + die "cannot export a snapshot in $class\n" if defined($snapshot);
> + __volume_export($class, $scfg, $storeid, $fh, $volname, $format,
> $snapshot, $base_snapshot, $with_snapshots);
> +}
> +
> sub volume_import_formats {
> my ($class, $scfg, $storeid, $volname, $base_snapshot, $with_snapshots) =
> @_;
> return () if $with_snapshots; # not supported
> diff --git a/PVE/Storage/LvmThinPlugin.pm b/PVE/Storage/LvmThinPlugin.pm
> index ccf5b7b..3c55db4 100644
> --- a/PVE/Storage/LvmThinPlugin.pm
> +++ b/PVE/Storage/LvmThinPlugin.pm
> @@ -368,4 +368,19 @@ sub volume_has_feature {
> return undef;
> }
>
> +sub volume_export_formats {
> + my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot,
> $with_snapshots) = @_;
> + # The difference to LVM-non-thin is that we *can* export snapshots
> + return $class->SUPER($class, $scfg, $storeid, $volname, $base_snapshot,
> $with_snapshots);
> +}
> +
> +sub volume_export {
> + my ($class, $scfg, $storeid, $fh, $volname, $format, $snapshot,
> $base_snapshot, $with_snapshots) = @_;
> + # LVMPlugin checks the $snapshot parameter here, we don't.
> + # But we need to activate them:
> + $class->activate_volume($storeid, $scfg, $volname, $snapshot) if
> defined($snapshot);
I do not like this, but OK if required.
> + PVE::Storage::LVMPlugin::__volume_export($class, $scfg, $storeid, $fh,
> $volname, $format, $snapshot, $base_snapshot, $with_snapshots);
> + $class->deactivate_volume($storeid, $scfg, $volname, $snapshot) if
> defined($snapshot);
But IMHO deactivate is wrong - what if someone else wants to access the
snapshot?
More information about the pve-devel
mailing list